From 14aeb3c39e60a604bdeed33949ba05e0c8c8be5d Mon Sep 17 00:00:00 2001 From: kiramt Date: Fri, 29 Sep 2017 11:27:13 +0100 Subject: [PATCH] JAL=2674 Removed getHiddenColumnsCopy --- src/jalview/appletgui/ScalePanel.java | 2 +- src/jalview/appletgui/SeqCanvas.java | 2 +- src/jalview/datamodel/CigarArray.java | 2 +- src/jalview/datamodel/HiddenColumns.java | 170 ++++++++++++++++---- src/jalview/datamodel/VisibleColsCollection.java | 2 +- src/jalview/datamodel/VisibleColsIterator.java | 130 --------------- src/jalview/gui/ScalePanel.java | 2 +- src/jalview/gui/SeqCanvas.java | 2 +- test/jalview/datamodel/HiddenColumnsTest.java | 20 +-- .../jalview/datamodel/VisibleColsIteratorTest.java | 25 ++- 10 files changed, 168 insertions(+), 189 deletions(-) delete mode 100644 src/jalview/datamodel/VisibleColsIterator.java diff --git a/src/jalview/appletgui/ScalePanel.java b/src/jalview/appletgui/ScalePanel.java index bbbb92b..6b3349f 100755 --- a/src/jalview/appletgui/ScalePanel.java +++ b/src/jalview/appletgui/ScalePanel.java @@ -438,7 +438,7 @@ public class ScalePanel extends Panel { int widthx = 1 + endx - startx; Iterator it = hidden.getBoundedStartIterator(startx, - startx + widthx + 1, true); + startx + widthx + 1); while (it.hasNext()) { res = it.next() - startx; diff --git a/src/jalview/appletgui/SeqCanvas.java b/src/jalview/appletgui/SeqCanvas.java index 5d55473..74a9f7f 100755 --- a/src/jalview/appletgui/SeqCanvas.java +++ b/src/jalview/appletgui/SeqCanvas.java @@ -492,7 +492,7 @@ public class SeqCanvas extends Panel implements ViewportListenerI g.setColor(Color.blue); int res; Iterator it = hidden.getBoundedStartIterator(startRes, - endx + 1, true); + endx + 1); while (it.hasNext()) { res = it.next() - startRes; diff --git a/src/jalview/datamodel/CigarArray.java b/src/jalview/datamodel/CigarArray.java index b6708ac..17e9ea6 100644 --- a/src/jalview/datamodel/CigarArray.java +++ b/src/jalview/datamodel/CigarArray.java @@ -165,7 +165,7 @@ public class CigarArray extends CigarBase int hideEnd; int last = start; - Iterator regions = hidden.getBoundedIterator(start, end, true); + Iterator regions = hidden.getBoundedIterator(start, end); while (regions.hasNext()) { region = regions.next(); diff --git a/src/jalview/datamodel/HiddenColumns.java b/src/jalview/datamodel/HiddenColumns.java index b915423..c77fb7b 100644 --- a/src/jalview/datamodel/HiddenColumns.java +++ b/src/jalview/datamodel/HiddenColumns.java @@ -27,6 +27,7 @@ import java.util.BitSet; import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.NoSuchElementException; import java.util.Vector; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -95,7 +96,7 @@ public class HiddenColumns if (copy != null) { hiddenColumns = new ArrayList<>(); - Iterator it = copy.getBoundedIterator(start, end, true); + Iterator it = copy.getBoundedIterator(start, end); while (it.hasNext()) { int[] region = it.next(); @@ -714,26 +715,6 @@ public class HiddenColumns } /** - * Returns a copy of the vector of hidden regions, as an ArrayList. Before - * using this method please consider if you really need access to the hidden - * regions - a new (or existing!) method on HiddenColumns might be more - * appropriate. - * - * @return hidden regions as an ArrayList of [start,end] pairs - */ - public ArrayList getHiddenColumnsCopy() - { - try - { - LOCK.readLock().lock(); - return copyHiddenRegionsToArrayList(0); - } finally - { - LOCK.readLock().unlock(); - } - } - - /** * return all visible segments between the given start and end boundaries * * @param start @@ -1513,16 +1494,19 @@ public class HiddenColumns } } - public Iterator getBoundedIterator(int start, int end, - boolean useCopy) + public Iterator getBoundedIterator(int start, int end) + { + return new BoundedHiddenColsIterator(start, end, true); + } + + public Iterator getBoundedStartIterator(int start, int end) { - return new BoundedHiddenColsIterator(start, end, useCopy); + return new BoundedStartRegionIterator(start, end, true); } - public Iterator getBoundedStartIterator(int start, int end, - boolean useCopy) + public Iterator getVisibleColsIterator(int start, int end) { - return new BoundedStartRegionIterator(start, end, useCopy); + return new VisibleColsIterator(start, end, true); } /** @@ -1531,11 +1515,8 @@ public class HiddenColumns * @author kmourao * */ - - class BoundedHiddenColsIterator implements Iterator { - private int start; // start position to iterate from private int end; // end position to iterate to @@ -1728,4 +1709,133 @@ public class HiddenColumns return result; } } + + public class VisibleColsIterator implements Iterator + { + private int last; + + private int current; + + private int next; + + private List localHidden = new ArrayList<>(); + + private int lasthiddenregion; + + public VisibleColsIterator(int firstcol, int lastcol, boolean useCopy) + { + last = lastcol; + current = firstcol; + next = firstcol; + lasthiddenregion = -1; + + try + { + if (useCopy) + { + // assume that if useCopy is false the calling code has locked + // hiddenColumns + LOCK.readLock().lock(); + } + + if (hiddenColumns != null) + { + int i = 0; + for (i = 0; i < hiddenColumns.size(); ++i) + { + if (current >= hiddenColumns.get(i)[0] + && current <= hiddenColumns.get(i)[1]) + { + // current is hidden, move to right + current = hiddenColumns.get(i)[1] + 1; + next = current; + } + if (current < hiddenColumns.get(i)[0]) + { + break; + } + } + + lasthiddenregion = i - 1; + + for (i = hiddenColumns.size() - 1; i >= 0; --i) + { + if (last >= hiddenColumns.get(i)[0] + && last <= hiddenColumns.get(i)[1]) + { + // last is hidden, move to left + last = hiddenColumns.get(i)[0] - 1; + } + if (last > hiddenColumns.get(i)[1]) + { + break; + } + } + + // make a local copy of the bit we need + i = lasthiddenregion + 1; + while (i < hiddenColumns.size() + && hiddenColumns.get(i)[0] <= last) + { + int[] region = new int[] { hiddenColumns.get(i)[0], + hiddenColumns.get(i)[1] }; + localHidden.add(region); + i++; + } + lasthiddenregion = -1; + } + } finally + { + if (useCopy) + { + LOCK.readLock().unlock(); + } + } + } + + @Override + public boolean hasNext() + { + return next <= last; + } + + @Override + public Integer next() + { + if (next > last) + { + throw new NoSuchElementException(); + } + current = next; + if ((localHidden != null) + && (lasthiddenregion + 1 < localHidden.size())) + { + // still some more hidden regions + if (next + 1 < localHidden.get(lasthiddenregion + 1)[0]) + { + // next+1 is still before the next hidden region + next++; + } + else if ((next + 1 >= localHidden.get(lasthiddenregion + 1)[0]) + && (next + 1 <= localHidden.get(lasthiddenregion + 1)[1])) + { + // next + 1 is in the next hidden region + next = localHidden.get(lasthiddenregion + 1)[1] + 1; + lasthiddenregion++; + } + } + else + { + // finished with hidden regions, just increment normally + next++; + } + return current; + } + + @Override + public void remove() + { + throw new UnsupportedOperationException(); + } + } } diff --git a/src/jalview/datamodel/VisibleColsCollection.java b/src/jalview/datamodel/VisibleColsCollection.java index e9437a7..c3d0ab4 100644 --- a/src/jalview/datamodel/VisibleColsCollection.java +++ b/src/jalview/datamodel/VisibleColsCollection.java @@ -42,7 +42,7 @@ public class VisibleColsCollection implements AlignmentColsCollectionI @Override public Iterator iterator() { - return new VisibleColsIterator(start, end, hidden); + return hidden.getVisibleColsIterator(start, end); } @Override diff --git a/src/jalview/datamodel/VisibleColsIterator.java b/src/jalview/datamodel/VisibleColsIterator.java deleted file mode 100644 index 9de468d..0000000 --- a/src/jalview/datamodel/VisibleColsIterator.java +++ /dev/null @@ -1,130 +0,0 @@ -/* - * Jalview - A Sequence Alignment Editor and Viewer ($$Version-Rel$$) - * Copyright (C) $$Year-Rel$$ The Jalview Authors - * - * This file is part of Jalview. - * - * Jalview is free software: you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * as published by the Free Software Foundation, either version 3 - * of the License, or (at your option) any later version. - * - * Jalview is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty - * of MERCHANTABILITY or FITNESS FOR A PARTICULAR - * PURPOSE. See the GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with Jalview. If not, see . - * The Jalview Authors are detailed in the 'AUTHORS' file. - */ -package jalview.datamodel; - -import java.util.Iterator; -import java.util.List; -import java.util.NoSuchElementException; - -/** - * An iterator which iterates over all visible columns in an alignment - * - * @author kmourao - * - */ -public class VisibleColsIterator implements Iterator -{ - private int last; - - private int current; - - private int next; - - private List hidden; - - private int lasthiddenregion; - - public VisibleColsIterator(int firstcol, int lastcol, - HiddenColumns hiddenCols) - { - last = lastcol; - current = firstcol; - next = firstcol; - hidden = hiddenCols.getHiddenColumnsCopy(); - lasthiddenregion = -1; - - if (hidden != null) - { - int i = 0; - for (i = 0; i < hidden.size(); ++i) - { - if (current >= hidden.get(i)[0] && current <= hidden.get(i)[1]) - { - // current is hidden, move to right - current = hidden.get(i)[1] + 1; - next = current; - } - if (current < hidden.get(i)[0]) - { - break; - } - } - lasthiddenregion = i - 1; - - for (i = hidden.size() - 1; i >= 0; --i) - { - if (last >= hidden.get(i)[0] && last <= hidden.get(i)[1]) - { - // last is hidden, move to left - last = hidden.get(i)[0] - 1; - } - if (last > hidden.get(i)[1]) - { - break; - } - } - } - } - - @Override - public boolean hasNext() - { - return next <= last; - } - - @Override - public Integer next() - { - if (next > last) - { - throw new NoSuchElementException(); - } - current = next; - if ((hidden != null) && (lasthiddenregion + 1 < hidden.size())) - { - // still some more hidden regions - if (next + 1 < hidden.get(lasthiddenregion + 1)[0]) - { - // next+1 is still before the next hidden region - next++; - } - else if ((next + 1 >= hidden.get(lasthiddenregion + 1)[0]) - && (next + 1 <= hidden.get(lasthiddenregion + 1)[1])) - { - // next + 1 is in the next hidden region - next = hidden.get(lasthiddenregion + 1)[1] + 1; - lasthiddenregion++; - } - } - else - { - // finished with hidden regions, just increment normally - next++; - } - return current; - } - - @Override - public void remove() - { - throw new UnsupportedOperationException(); - } -} diff --git a/src/jalview/gui/ScalePanel.java b/src/jalview/gui/ScalePanel.java index bf9ca8e..605cc5c 100755 --- a/src/jalview/gui/ScalePanel.java +++ b/src/jalview/gui/ScalePanel.java @@ -489,7 +489,7 @@ public class ScalePanel extends JPanel if (av.getShowHiddenMarkers()) { Iterator it = hidden.getBoundedStartIterator(startx, - startx + widthx + 1, true); + startx + widthx + 1); while (it.hasNext()) { res = it.next() - startx; diff --git a/src/jalview/gui/SeqCanvas.java b/src/jalview/gui/SeqCanvas.java index b7d74de..f61c7c6 100755 --- a/src/jalview/gui/SeqCanvas.java +++ b/src/jalview/gui/SeqCanvas.java @@ -698,7 +698,7 @@ public class SeqCanvas extends JComponent implements ViewportListenerI HiddenColumns hidden = av.getAlignment().getHiddenColumns(); Iterator it = hidden.getBoundedStartIterator(startRes, - endx + 1, true); + endx + 1); while (it.hasNext()) { res = it.next() - startRes; diff --git a/test/jalview/datamodel/HiddenColumnsTest.java b/test/jalview/datamodel/HiddenColumnsTest.java index 1ad6d85..142762b 100644 --- a/test/jalview/datamodel/HiddenColumnsTest.java +++ b/test/jalview/datamodel/HiddenColumnsTest.java @@ -1337,7 +1337,7 @@ public class HiddenColumnsTest public void testBoundedIterator() { HiddenColumns h = new HiddenColumns(); - Iterator it = h.getBoundedIterator(0, 10, false); + Iterator it = h.getBoundedIterator(0, 10); // no hidden columns = nothing to iterate over assertFalse(it.hasNext()); @@ -1346,7 +1346,7 @@ public class HiddenColumnsTest // all regions are returned h.hideColumns(3, 10); h.hideColumns(14, 16); - it = h.getBoundedIterator(0, 20, false); + it = h.getBoundedIterator(0, 20); assertTrue(it.hasNext()); int[] next = it.next(); assertEquals(3, next[0]); @@ -1358,7 +1358,7 @@ public class HiddenColumnsTest // [start,end] overlaps a region // 1 region returned - it = h.getBoundedIterator(5, 7, false); + it = h.getBoundedIterator(5, 7); assertTrue(it.hasNext()); next = it.next(); assertEquals(3, next[0]); @@ -1367,7 +1367,7 @@ public class HiddenColumnsTest // [start,end] fully contains 1 region and start of last // - 2 regions returned - it = h.getBoundedIterator(3, 15, false); + it = h.getBoundedIterator(3, 15); assertTrue(it.hasNext()); next = it.next(); assertEquals(3, next[0]); @@ -1379,7 +1379,7 @@ public class HiddenColumnsTest // [start,end] contains end of first region and whole of last region // - 2 regions returned - it = h.getBoundedIterator(4, 20, false); + it = h.getBoundedIterator(4, 20); assertTrue(it.hasNext()); next = it.next(); assertEquals(3, next[0]); @@ -1394,7 +1394,7 @@ public class HiddenColumnsTest public void testBoundedStartIterator() { HiddenColumns h = new HiddenColumns(); - Iterator it = h.getBoundedStartIterator(0, 10, false); + Iterator it = h.getBoundedStartIterator(0, 10); // no hidden columns = nothing to iterate over assertFalse(it.hasNext()); @@ -1403,7 +1403,7 @@ public class HiddenColumnsTest // all regions are returned h.hideColumns(3, 10); h.hideColumns(14, 16); - it = h.getBoundedStartIterator(0, 20, false); + it = h.getBoundedStartIterator(0, 20); assertTrue(it.hasNext()); int next = it.next(); assertEquals(3, next); @@ -1413,12 +1413,12 @@ public class HiddenColumnsTest // [start,end] does not contain a start of a region // no regions to iterate over - it = h.getBoundedStartIterator(4, 5, false); + it = h.getBoundedStartIterator(4, 5); assertFalse(it.hasNext()); // [start,end] fully contains 1 region and start of last // - 2 regions returned - it = h.getBoundedStartIterator(3, 7, false); + it = h.getBoundedStartIterator(3, 7); assertTrue(it.hasNext()); next = it.next(); assertEquals(3, next); @@ -1428,7 +1428,7 @@ public class HiddenColumnsTest // [start,end] contains whole of last region // - 1 region returned - it = h.getBoundedStartIterator(4, 20, false); + it = h.getBoundedStartIterator(4, 20); assertTrue(it.hasNext()); next = it.next(); assertEquals(6, next); diff --git a/test/jalview/datamodel/VisibleColsIteratorTest.java b/test/jalview/datamodel/VisibleColsIteratorTest.java index b2d747b..92d39be 100644 --- a/test/jalview/datamodel/VisibleColsIteratorTest.java +++ b/test/jalview/datamodel/VisibleColsIteratorTest.java @@ -22,6 +22,7 @@ package jalview.datamodel; import static org.testng.Assert.assertTrue; +import java.util.Iterator; import java.util.NoSuchElementException; import org.testng.annotations.BeforeClass; @@ -50,7 +51,7 @@ public class VisibleColsIteratorTest @Test(groups = { "Functional" }) public void testHasNextAndNextWithHidden() { - VisibleColsIterator it = new VisibleColsIterator(0, 6, hiddenCols); + Iterator it = hiddenCols.getVisibleColsIterator(0, 6); int count = 0; while (it.hasNext()) { @@ -67,8 +68,8 @@ public class VisibleColsIteratorTest @Test(groups = { "Functional" }) public void testHasNextAndNextNoHidden() { - VisibleColsIterator it2 = new VisibleColsIterator(0, 3, - new HiddenColumns()); + HiddenColumns test = new HiddenColumns(); + Iterator it2 = test.getVisibleColsIterator(0, 3); int count = 0; while (it2.hasNext()) { @@ -85,8 +86,7 @@ public class VisibleColsIteratorTest @Test(groups = { "Functional" }) public void testHasNextAndNextStartHidden() { - VisibleColsIterator it3 = new VisibleColsIterator(0, 6, - hiddenColsAtStart); + Iterator it3 = hiddenColsAtStart.getVisibleColsIterator(0, 6); int count = 0; while (it3.hasNext()) { @@ -103,7 +103,7 @@ public class VisibleColsIteratorTest @Test(groups = { "Functional" }) public void testHasNextAndNextEndHidden() { - VisibleColsIterator it4 = new VisibleColsIterator(0, 4, hiddenCols); + Iterator it4 = hiddenCols.getVisibleColsIterator(0, 4); int count = 0; while (it4.hasNext()) { @@ -123,7 +123,7 @@ public class VisibleColsIteratorTest expectedExceptions = { NoSuchElementException.class }) public void testLastNextWithHidden() throws NoSuchElementException { - VisibleColsIterator it = new VisibleColsIterator(0, 3, hiddenCols); + Iterator it = hiddenCols.getVisibleColsIterator(0, 3); while (it.hasNext()) { it.next(); @@ -140,8 +140,8 @@ public class VisibleColsIteratorTest expectedExceptions = { NoSuchElementException.class }) public void testLastNextNoHidden() throws NoSuchElementException { - VisibleColsIterator it2 = new VisibleColsIterator(0, 3, - new HiddenColumns()); + HiddenColumns test = new HiddenColumns(); + Iterator it2 = test.getVisibleColsIterator(0, 3); while (it2.hasNext()) { it2.next(); @@ -158,8 +158,7 @@ public class VisibleColsIteratorTest expectedExceptions = { NoSuchElementException.class }) public void testLastNextStartHidden() throws NoSuchElementException { - VisibleColsIterator it3 = new VisibleColsIterator(0, 6, - hiddenColsAtStart); + Iterator it3 = hiddenColsAtStart.getVisibleColsIterator(0, 6); while (it3.hasNext()) { it3.next(); @@ -176,7 +175,7 @@ public class VisibleColsIteratorTest expectedExceptions = { NoSuchElementException.class }) public void testLastNextEndHidden() throws NoSuchElementException { - VisibleColsIterator it4 = new VisibleColsIterator(0, 4, hiddenCols); + Iterator it4 = hiddenCols.getVisibleColsIterator(0, 4); while (it4.hasNext()) { it4.next(); @@ -192,7 +191,7 @@ public class VisibleColsIteratorTest expectedExceptions = { UnsupportedOperationException.class }) public void testRemove() throws UnsupportedOperationException { - VisibleColsIterator it = new VisibleColsIterator(0, 3, hiddenCols); + Iterator it = hiddenCols.getVisibleColsIterator(0, 3); it.remove(); } } -- 1.7.10.2