From ad09c54f294b945fd4d037a93f5fa8eced6c1797 Mon Sep 17 00:00:00 2001 From: kiramt Date: Wed, 27 Sep 2017 11:28:24 +0100 Subject: [PATCH] JAL-2674 Removing most calls to getHiddenColumnsCopy --- .../org/jalview/HiddenColsIteratorsBenchmark.java | 37 ----- .../java/org/jalview/HiddenColumnsBenchmark.java | 8 - src/jalview/datamodel/HiddenColumns.java | 37 +++-- src/jalview/util/MappingUtils.java | 6 +- test/jalview/datamodel/ColumnSelectionTest.java | 37 ++--- test/jalview/datamodel/HiddenColumnsTest.java | 168 ++++++++++---------- test/jalview/gui/AlignFrameTest.java | 35 ++-- test/jalview/gui/AnnotationColumnChooserTest.java | 28 ++-- test/jalview/io/JSONFileTest.java | 14 +- test/jalview/util/MappingUtilsTest.java | 29 ++-- 10 files changed, 193 insertions(+), 206 deletions(-) diff --git a/benchmarking/src/main/java/org/jalview/HiddenColsIteratorsBenchmark.java b/benchmarking/src/main/java/org/jalview/HiddenColsIteratorsBenchmark.java index 7836367..e97baba 100644 --- a/benchmarking/src/main/java/org/jalview/HiddenColsIteratorsBenchmark.java +++ b/benchmarking/src/main/java/org/jalview/HiddenColsIteratorsBenchmark.java @@ -138,43 +138,6 @@ public class HiddenColsIteratorsBenchmark { return blockStart; } - - @Benchmark - @BenchmarkMode({Mode.Throughput}) - public int benchHiddenColsCopy(HiddenColsAndStartState tstate) - { - int startx = tstate.visibleColumn; - int blockEnd; - int blockStart = startx; - int screenY = 0; - for (int[] region : tstate.h.getHiddenColumnsCopy()) - { - int hideStart = region[0]; - int hideEnd = region[1]; - - if (hideStart <= blockStart) - { - blockStart += (hideEnd - hideStart) + 1; - continue; - } - - //do something - Blackhole.consumeCPU(5); - blockEnd = Math.min(hideStart - 1, blockStart + 60 - screenY); - - screenY += blockEnd - blockStart + 1; - blockStart = hideEnd + 1; - - if (screenY > 60) - { - //done - break; - } - } - return blockStart; - } - - @Benchmark @BenchmarkMode({Mode.Throughput}) public int benchLoop1(HiddenColsAndStartState tstate) diff --git a/benchmarking/src/main/java/org/jalview/HiddenColumnsBenchmark.java b/benchmarking/src/main/java/org/jalview/HiddenColumnsBenchmark.java index 383a064..aa4ad22 100644 --- a/benchmarking/src/main/java/org/jalview/HiddenColumnsBenchmark.java +++ b/benchmarking/src/main/java/org/jalview/HiddenColumnsBenchmark.java @@ -121,14 +121,6 @@ public class HiddenColumnsBenchmark @Benchmark @BenchmarkMode({Mode.Throughput}) - public ArrayList benchGetHiddenColumnsCopy(HiddenColsAndStartState tstate) - { - return tstate.h.getHiddenColumnsCopy(); - } - - - @Benchmark - @BenchmarkMode({Mode.Throughput}) public int benchGetSize(HiddenColsAndStartState tstate) { return tstate.h.getSize(); diff --git a/src/jalview/datamodel/HiddenColumns.java b/src/jalview/datamodel/HiddenColumns.java index a704b3c..eda8850 100644 --- a/src/jalview/datamodel/HiddenColumns.java +++ b/src/jalview/datamodel/HiddenColumns.java @@ -186,6 +186,28 @@ public class HiddenColumns } } + /** + * Get the number of distinct hidden regions + * + * @return number of regions + */ + public int getNumberOfRegions() + { + try + { + LOCK.readLock().lock(); + int num = 0; + if (hasHiddenColumns()) + { + num = hiddenColumns.size(); + } + return num; + } finally + { + LOCK.readLock().unlock(); + } + } + @Override public boolean equals(Object obj) { @@ -1240,16 +1262,17 @@ public class HiddenColumns // but where each hidden column region is shifted backwards by the number of // preceding visible gaps // update hidden columns at the same time - ArrayList regions = getHiddenColumnsCopy(); + Iterator regions = iterator(); ArrayList newhidden = new ArrayList<>(); int numGapsBefore = 0; int gapPosition = 0; - for (int[] region : regions) + while (regions.hasNext()) { // get region coordinates accounting for gaps // we can rely on gaps not being *in* hidden regions because we already // removed those + int[] region = regions.next(); while (gapPosition < region[0]) { gapPosition++; @@ -1495,7 +1518,8 @@ public class HiddenColumns { if (hiddenColumns != null) { - return new BoundedHiddenColsIterator(0, hiddenColumns.size(), true); + int last = hiddenColumns.get(hiddenColumns.size() - 1)[1]; + return new BoundedHiddenColsIterator(0, last, true); } else { @@ -1550,13 +1574,6 @@ public class HiddenColumns * lower bound to iterate from * @param upperBound * upper bound to iterate to - * @param opt - * Option.OVERLAP: regions which overlap [lowerBound,upperBound] - * are included Option.START: regions which start in - * [lowerBound,upperBound] are included - * @param useAbsolutePos - * have bounds and return values with reference to absolute indices - * (if false, use indices for visible columns) * @param useCopyCols * whether to make a local copy of hiddenColumns for iteration (set * to true if calling from outwith the HiddenColumns class) diff --git a/src/jalview/util/MappingUtils.java b/src/jalview/util/MappingUtils.java index 3682239..09d4b13 100644 --- a/src/jalview/util/MappingUtils.java +++ b/src/jalview/util/MappingUtils.java @@ -542,9 +542,11 @@ public final class MappingUtils toSequences, fromGapChar); } - for (int[] hidden : hiddencols.getHiddenColumnsCopy()) + Iterator regions = hiddencols.iterator(); + while (regions.hasNext()) { - mapHiddenColumns(hidden, codonFrames, newHidden, fromSequences, + mapHiddenColumns(regions.next(), codonFrames, newHidden, + fromSequences, toSequences, fromGapChar); } return; // mappedColumns; diff --git a/test/jalview/datamodel/ColumnSelectionTest.java b/test/jalview/datamodel/ColumnSelectionTest.java index e99e952..8709961 100644 --- a/test/jalview/datamodel/ColumnSelectionTest.java +++ b/test/jalview/datamodel/ColumnSelectionTest.java @@ -32,6 +32,7 @@ import java.util.Arrays; import java.util.BitSet; import java.util.Collections; import java.util.ConcurrentModificationException; +import java.util.Iterator; import java.util.List; import org.testng.annotations.BeforeClass; @@ -132,9 +133,9 @@ public class ColumnSelectionTest // hide column 5 (and adjacent): cs.hideSelectedColumns(5, al.getHiddenColumns()); // 4,5,6 now hidden: - List hidden = al.getHiddenColumns().getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); - assertEquals("[4, 6]", Arrays.toString(hidden.get(0))); + Iterator regions = al.getHiddenColumns().iterator(); + assertEquals(1, al.getHiddenColumns().getNumberOfRegions()); + assertEquals("[4, 6]", Arrays.toString(regions.next())); // none now selected: assertTrue(cs.getSelected().isEmpty()); @@ -145,9 +146,9 @@ public class ColumnSelectionTest cs.addElement(5); cs.addElement(6); cs.hideSelectedColumns(4, al.getHiddenColumns()); - hidden = al.getHiddenColumns().getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); - assertEquals("[4, 6]", Arrays.toString(hidden.get(0))); + regions = al.getHiddenColumns().iterator(); + assertEquals(1, al.getHiddenColumns().getNumberOfRegions()); + assertEquals("[4, 6]", Arrays.toString(regions.next())); assertTrue(cs.getSelected().isEmpty()); // repeat, hiding column (4, 5 and) 6 @@ -157,9 +158,9 @@ public class ColumnSelectionTest cs.addElement(5); cs.addElement(6); cs.hideSelectedColumns(6, al.getHiddenColumns()); - hidden = al.getHiddenColumns().getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); - assertEquals("[4, 6]", Arrays.toString(hidden.get(0))); + regions = al.getHiddenColumns().iterator(); + assertEquals(1, al.getHiddenColumns().getNumberOfRegions()); + assertEquals("[4, 6]", Arrays.toString(regions.next())); assertTrue(cs.getSelected().isEmpty()); // repeat, with _only_ adjacent columns selected @@ -168,9 +169,9 @@ public class ColumnSelectionTest cs.addElement(4); cs.addElement(6); cs.hideSelectedColumns(5, al.getHiddenColumns()); - hidden = al.getHiddenColumns().getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); - assertEquals("[4, 6]", Arrays.toString(hidden.get(0))); + regions = al.getHiddenColumns().iterator(); + assertEquals(1, al.getHiddenColumns().getNumberOfRegions()); + assertEquals("[4, 6]", Arrays.toString(regions.next())); assertTrue(cs.getSelected().isEmpty()); } @@ -196,12 +197,12 @@ public class ColumnSelectionTest cs.hideSelectedColumns(al); assertTrue(cs.getSelected().isEmpty()); - List hidden = cols.getHiddenColumnsCopy(); - assertEquals(4, hidden.size()); - assertEquals("[2, 4]", Arrays.toString(hidden.get(0))); - assertEquals("[7, 9]", Arrays.toString(hidden.get(1))); - assertEquals("[15, 18]", Arrays.toString(hidden.get(2))); - assertEquals("[20, 22]", Arrays.toString(hidden.get(3))); + Iterator regions = cols.iterator(); + assertEquals(4, cols.getNumberOfRegions()); + assertEquals("[2, 4]", Arrays.toString(regions.next())); + assertEquals("[7, 9]", Arrays.toString(regions.next())); + assertEquals("[15, 18]", Arrays.toString(regions.next())); + assertEquals("[20, 22]", Arrays.toString(regions.next())); } /** diff --git a/test/jalview/datamodel/HiddenColumnsTest.java b/test/jalview/datamodel/HiddenColumnsTest.java index 8aee615..19a5671 100644 --- a/test/jalview/datamodel/HiddenColumnsTest.java +++ b/test/jalview/datamodel/HiddenColumnsTest.java @@ -29,7 +29,6 @@ import jalview.analysis.AlignmentGenerator; import jalview.gui.JvOptionPane; import jalview.util.Comparison; -import java.util.ArrayList; import java.util.Arrays; import java.util.BitSet; import java.util.Iterator; @@ -248,17 +247,19 @@ public class HiddenColumnsTest HiddenColumns cs = new HiddenColumns(); cs.hideColumns(10, 11); cs.hideColumns(5, 7); + Iterator regions = cs.iterator(); assertEquals("[5, 7]", - Arrays.toString(cs.getHiddenColumnsCopy().get(0))); + Arrays.toString(regions.next())); HiddenColumns cs2 = new HiddenColumns(cs); + regions = cs2.iterator(); assertTrue(cs2.hasHiddenColumns()); - assertEquals(2, cs2.getHiddenColumnsCopy().size()); + assertEquals(2, cs2.getNumberOfRegions()); // hidden columns are held in column order assertEquals("[5, 7]", - Arrays.toString(cs2.getHiddenColumnsCopy().get(0))); + Arrays.toString(regions.next())); assertEquals("[10, 11]", - Arrays.toString(cs2.getHiddenColumnsCopy().get(1))); + Arrays.toString(regions.next())); } @Test(groups = "Functional") @@ -270,18 +271,20 @@ public class HiddenColumnsTest HiddenColumns cs2 = new HiddenColumns(cs, 3, 9, 1); assertTrue(cs2.hasHiddenColumns()); + Iterator regions = cs2.iterator(); // only [5,7] returned, offset by 1 assertEquals("[4, 6]", - Arrays.toString(cs2.getHiddenColumnsCopy().get(0))); + Arrays.toString(regions.next())); assertEquals(3, cs2.getSize()); cs2 = new HiddenColumns(cs, 8, 15, 4); + regions = cs2.iterator(); assertTrue(cs2.hasHiddenColumns()); // only [10,11] returned, offset by 4 assertEquals("[6, 7]", - Arrays.toString(cs2.getHiddenColumnsCopy().get(0))); + Arrays.toString(regions.next())); assertEquals(2, cs2.getSize()); cs2 = new HiddenColumns(cs, 6, 10, 4); @@ -549,80 +552,79 @@ public class HiddenColumnsTest ColumnSelection colsel = new ColumnSelection(); HiddenColumns cs = al.getHiddenColumns(); colsel.hideSelectedColumns(5, al.getHiddenColumns()); - List hidden = cs.getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); - assertEquals("[5, 5]", Arrays.toString(hidden.get(0))); + Iterator regions = cs.iterator(); + assertEquals(1, cs.getNumberOfRegions()); + assertEquals("[5, 5]", Arrays.toString(regions.next())); colsel.hideSelectedColumns(3, al.getHiddenColumns()); - hidden = cs.getHiddenColumnsCopy(); - assertEquals(2, hidden.size()); + regions = cs.iterator(); + assertEquals(2, cs.getNumberOfRegions()); // two hidden ranges, in order: - assertEquals(hidden.size(), cs.getHiddenColumnsCopy().size()); - assertEquals("[3, 3]", Arrays.toString(hidden.get(0))); - assertEquals("[5, 5]", Arrays.toString(hidden.get(1))); + assertEquals("[3, 3]", Arrays.toString(regions.next())); + assertEquals("[5, 5]", Arrays.toString(regions.next())); // hiding column 4 expands [3, 3] to [3, 4] // and merges to [5, 5] to make [3, 5] colsel.hideSelectedColumns(4, al.getHiddenColumns()); - hidden = cs.getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); - assertEquals("[3, 5]", Arrays.toString(hidden.get(0))); + regions = cs.iterator(); + assertEquals(1, cs.getNumberOfRegions()); + assertEquals("[3, 5]", Arrays.toString(regions.next())); // clear hidden columns (note they are added to selected) cs.revealAllHiddenColumns(colsel); // it is now actually null but getter returns an empty list - assertTrue(cs.getHiddenColumnsCopy().isEmpty()); + assertEquals(0, cs.getNumberOfRegions()); cs.hideColumns(3, 6); - hidden = cs.getHiddenColumnsCopy(); - int[] firstHiddenRange = hidden.get(0); + regions = cs.iterator(); + int[] firstHiddenRange = regions.next(); assertEquals("[3, 6]", Arrays.toString(firstHiddenRange)); // adding a subrange of already hidden should do nothing cs.hideColumns(4, 5); - hidden = cs.getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); + regions = cs.iterator(); + assertEquals(1, cs.getNumberOfRegions()); assertEquals("[3, 6]", - Arrays.toString(cs.getHiddenColumnsCopy().get(0))); + Arrays.toString(regions.next())); cs.hideColumns(3, 5); - hidden = cs.getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); + regions = cs.iterator(); + assertEquals(1, cs.getNumberOfRegions()); assertEquals("[3, 6]", - Arrays.toString(cs.getHiddenColumnsCopy().get(0))); + Arrays.toString(regions.next())); cs.hideColumns(4, 6); - hidden = cs.getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); + regions = cs.iterator(); + assertEquals(1, cs.getNumberOfRegions()); assertEquals("[3, 6]", - Arrays.toString(cs.getHiddenColumnsCopy().get(0))); + Arrays.toString(regions.next())); cs.hideColumns(3, 6); - hidden = cs.getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); + regions = cs.iterator(); + assertEquals(1, cs.getNumberOfRegions()); assertEquals("[3, 6]", - Arrays.toString(cs.getHiddenColumnsCopy().get(0))); + Arrays.toString(regions.next())); cs.revealAllHiddenColumns(colsel); cs.hideColumns(2, 4); - hidden = cs.getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); - assertEquals("[2, 4]", Arrays.toString(hidden.get(0))); + regions = cs.iterator(); + assertEquals(1, cs.getNumberOfRegions()); + assertEquals("[2, 4]", Arrays.toString(regions.next())); // extend contiguous with 2 positions overlap cs.hideColumns(3, 5); - hidden = cs.getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); - assertEquals("[2, 5]", Arrays.toString(hidden.get(0))); + regions = cs.iterator(); + assertEquals(1, cs.getNumberOfRegions()); + assertEquals("[2, 5]", Arrays.toString(regions.next())); // extend contiguous with 1 position overlap cs.hideColumns(5, 6); - hidden = cs.getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); - assertEquals("[2, 6]", Arrays.toString(hidden.get(0))); + regions = cs.iterator(); + assertEquals(1, cs.getNumberOfRegions()); + assertEquals("[2, 6]", Arrays.toString(regions.next())); // extend contiguous with overlap both ends: cs.hideColumns(1, 7); - hidden = cs.getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); - assertEquals("[1, 7]", Arrays.toString(hidden.get(0))); + regions = cs.iterator(); + assertEquals(1, cs.getNumberOfRegions()); + assertEquals("[1, 7]", Arrays.toString(regions.next())); } /** @@ -709,15 +711,15 @@ public class HiddenColumnsTest HiddenColumns cs = new HiddenColumns(); cs.hideColumns(49, 59); cs.hideColumns(69, 79); - List hidden = cs.getHiddenColumnsCopy(); - assertEquals(2, hidden.size()); - assertEquals("[49, 59]", Arrays.toString(hidden.get(0))); - assertEquals("[69, 79]", Arrays.toString(hidden.get(1))); + Iterator regions = cs.iterator(); + assertEquals(2, cs.getNumberOfRegions()); + assertEquals("[49, 59]", Arrays.toString(regions.next())); + assertEquals("[69, 79]", Arrays.toString(regions.next())); cs.hideColumns(48, 80); - hidden = cs.getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); - assertEquals("[48, 80]", Arrays.toString(hidden.get(0))); + regions = cs.iterator(); + assertEquals(1, cs.getNumberOfRegions()); + assertEquals("[48, 80]", Arrays.toString(regions.next())); /* * another...joining hidden ranges @@ -728,9 +730,9 @@ public class HiddenColumnsTest cs.hideColumns(50, 60); // hiding 21-49 should merge to one range cs.hideColumns(21, 49); - hidden = cs.getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); - assertEquals("[10, 60]", Arrays.toString(hidden.get(0))); + regions = cs.iterator(); + assertEquals(1, cs.getNumberOfRegions()); + assertEquals("[10, 60]", Arrays.toString(regions.next())); /* * another...left overlap, subsumption, right overlap, @@ -744,10 +746,10 @@ public class HiddenColumnsTest cs.hideColumns(60, 70); cs.hideColumns(15, 45); - hidden = cs.getHiddenColumnsCopy(); - assertEquals(2, hidden.size()); - assertEquals("[10, 50]", Arrays.toString(hidden.get(0))); - assertEquals("[60, 70]", Arrays.toString(hidden.get(1))); + regions = cs.iterator(); + assertEquals(2, cs.getNumberOfRegions()); + assertEquals("[10, 50]", Arrays.toString(regions.next())); + assertEquals("[60, 70]", Arrays.toString(regions.next())); } @Test(groups = { "Functional" }) @@ -761,23 +763,23 @@ public class HiddenColumnsTest one.set(1); cs = new HiddenColumns(); cs.hideMarkedBits(one); - assertEquals(1, cs.getHiddenColumnsCopy().size()); + assertEquals(1, cs.getNumberOfRegions()); one.set(2); cs = new HiddenColumns(); cs.hideMarkedBits(one); - assertEquals(1, cs.getHiddenColumnsCopy().size()); + assertEquals(1, cs.getNumberOfRegions()); one.set(3); cs = new HiddenColumns(); cs.hideMarkedBits(one); - assertEquals(1, cs.getHiddenColumnsCopy().size()); + assertEquals(1, cs.getNumberOfRegions()); // split one.clear(2); cs = new HiddenColumns(); cs.hideMarkedBits(one); - assertEquals(2, cs.getHiddenColumnsCopy().size()); + assertEquals(2, cs.getNumberOfRegions()); assertEquals(0, cs.adjustForHiddenColumns(0)); assertEquals(2, cs.adjustForHiddenColumns(1)); @@ -788,7 +790,7 @@ public class HiddenColumnsTest cs = new HiddenColumns(); cs.hideMarkedBits(one); - assertEquals(1, cs.getHiddenColumnsCopy().size()); + assertEquals(1, cs.getNumberOfRegions()); assertEquals(0, cs.adjustForHiddenColumns(0)); assertEquals(1, cs.adjustForHiddenColumns(1)); @@ -1144,34 +1146,38 @@ public class HiddenColumnsTest } @Test(groups = "Functional") - public void testGetHiddenColumnsCopy() + public void testIterator() { HiddenColumns h = new HiddenColumns(); - ArrayList result = h.getHiddenColumnsCopy(); - assertTrue(result.isEmpty()); + Iterator result = h.iterator(); + assertFalse(result.hasNext()); h.hideColumns(5, 10); - result = h.getHiddenColumnsCopy(); - assertEquals(1, result.size()); - assertEquals(5, result.get(0)[0]); - assertEquals(10, result.get(0)[1]); + result = h.iterator(); + int[] next = result.next(); + assertEquals(5, next[0]); + assertEquals(10, next[1]); + assertFalse(result.hasNext()); h.hideColumns(22, 23); - result = h.getHiddenColumnsCopy(); - assertEquals(2, result.size()); - assertEquals(5, result.get(0)[0]); - assertEquals(10, result.get(0)[1]); - assertEquals(22, result.get(1)[0]); - assertEquals(23, result.get(1)[1]); + result = h.iterator(); + next = result.next(); + assertEquals(5, next[0]); + assertEquals(10, next[1]); + next = result.next(); + assertEquals(22, next[0]); + assertEquals(23, next[1]); + assertFalse(result.hasNext()); // test for only one hidden region at start of alignment ColumnSelection sel = new ColumnSelection(); h.revealAllHiddenColumns(sel); h.hideColumns(0, 1); - result = h.getHiddenColumnsCopy(); - assertEquals(1, result.size()); - assertEquals(0, result.get(0)[0]); - assertEquals(1, result.get(0)[1]); + result = h.iterator(); + next = result.next(); + assertEquals(0, next[0]); + assertEquals(1, next[1]); + assertFalse(result.hasNext()); } @Test(groups = "Functional") diff --git a/test/jalview/gui/AlignFrameTest.java b/test/jalview/gui/AlignFrameTest.java index af9c045..dd1a4de 100644 --- a/test/jalview/gui/AlignFrameTest.java +++ b/test/jalview/gui/AlignFrameTest.java @@ -46,7 +46,7 @@ import jalview.schemes.TurnColourScheme; import jalview.util.MessageManager; import java.awt.Color; -import java.util.List; +import java.util.Iterator; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeClass; @@ -85,24 +85,20 @@ public class AlignFrameTest */ assertFalse(alignFrame.hideFeatureColumns("exon", true)); assertTrue(alignFrame.getViewport().getColumnSelection().isEmpty()); - assertTrue(alignFrame.getViewport().getAlignment().getHiddenColumns() - .getHiddenColumnsCopy() - .isEmpty()); + assertEquals(alignFrame.getViewport().getAlignment().getHiddenColumns() + .getNumberOfRegions(), 0); assertFalse(alignFrame.hideFeatureColumns("exon", false)); assertTrue(alignFrame.getViewport().getColumnSelection().isEmpty()); - assertTrue(alignFrame.getViewport().getAlignment().getHiddenColumns() - .getHiddenColumnsCopy() - .isEmpty()); + assertEquals(alignFrame.getViewport().getAlignment().getHiddenColumns() + .getNumberOfRegions(), 0); /* * hiding a feature in all columns does nothing */ assertFalse(alignFrame.hideFeatureColumns("Metal", true)); assertTrue(alignFrame.getViewport().getColumnSelection().isEmpty()); - List hidden = alignFrame.getViewport().getAlignment() - .getHiddenColumns() - .getHiddenColumnsCopy(); - assertTrue(hidden.isEmpty()); + assertEquals(alignFrame.getViewport().getAlignment().getHiddenColumns() + .getNumberOfRegions(), 0); /* * hide a feature present in some columns @@ -110,13 +106,16 @@ public class AlignFrameTest * [1-3], [6-8] base zero */ assertTrue(alignFrame.hideFeatureColumns("Turn", true)); - hidden = alignFrame.getViewport().getAlignment().getHiddenColumns() - .getHiddenColumnsCopy(); - assertEquals(hidden.size(), 2); - assertEquals(hidden.get(0)[0], 1); - assertEquals(hidden.get(0)[1], 3); - assertEquals(hidden.get(1)[0], 6); - assertEquals(hidden.get(1)[1], 8); + Iterator regions = alignFrame.getViewport().getAlignment() + .getHiddenColumns().iterator(); + assertEquals(alignFrame.getViewport().getAlignment().getHiddenColumns() + .getNumberOfRegions(), 2); + int[] next = regions.next(); + assertEquals(next[0], 1); + assertEquals(next[1], 3); + next = regions.next(); + assertEquals(next[0], 6); + assertEquals(next[1], 8); } @BeforeClass(alwaysRun = true) diff --git a/test/jalview/gui/AnnotationColumnChooserTest.java b/test/jalview/gui/AnnotationColumnChooserTest.java index 06478d5..912cd27 100644 --- a/test/jalview/gui/AnnotationColumnChooserTest.java +++ b/test/jalview/gui/AnnotationColumnChooserTest.java @@ -35,7 +35,7 @@ import jalview.io.FileFormat; import jalview.io.FormatAdapter; import java.io.IOException; -import java.util.List; +import java.util.Iterator; import org.testng.annotations.BeforeClass; import org.testng.annotations.BeforeMethod; @@ -141,18 +141,21 @@ public class AnnotationColumnChooserTest HiddenColumns currentHidden = af.getViewport().getAlignment() .getHiddenColumns(); - List regions = currentHidden.getHiddenColumnsCopy(); - assertEquals(regions.get(0)[0], 0); - assertEquals(regions.get(0)[1], 3); - assertEquals(regions.get(1)[0], 22); - assertEquals(regions.get(1)[1], 25); + Iterator regions = currentHidden.iterator(); + int[] next = regions.next(); + assertEquals(0, next[0]); + assertEquals(3, next[1]); + next = regions.next(); + assertEquals(22, next[0]); + assertEquals(25, next[1]); // now reset hidden columns acc.reset(); currentHidden = af.getViewport().getAlignment().getHiddenColumns(); - regions = currentHidden.getHiddenColumnsCopy(); - assertEquals(regions.get(0)[0], 10); - assertEquals(regions.get(0)[1], 20); + regions = currentHidden.iterator(); + next = regions.next(); + assertEquals(10, next[0]); + assertEquals(20, next[1]); // check works with empty hidden columns as old columns oldhidden = new HiddenColumns(); @@ -169,8 +172,9 @@ public class AnnotationColumnChooserTest acc.reset(); currentHidden = af.getViewport().getAlignment().getHiddenColumns(); - regions = currentHidden.getHiddenColumnsCopy(); - assertEquals(regions.get(0)[0], 10); - assertEquals(regions.get(0)[1], 20); + regions = currentHidden.iterator(); + next = regions.next(); + assertEquals(10, next[0]); + assertEquals(20, next[1]); } } diff --git a/test/jalview/io/JSONFileTest.java b/test/jalview/io/JSONFileTest.java index 158c901..5e835bf 100644 --- a/test/jalview/io/JSONFileTest.java +++ b/test/jalview/io/JSONFileTest.java @@ -42,6 +42,7 @@ import jalview.schemes.ResidueColourScheme; import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; @@ -205,7 +206,7 @@ public class JSONFileTest TEST_SEQ_HEIGHT = expectedSeqs.size(); TEST_GRP_HEIGHT = expectedGrps.size(); TEST_ANOT_HEIGHT = expectedAnnots.size(); - TEST_CS_HEIGHT = expectedColSel.getHiddenColumnsCopy().size(); + TEST_CS_HEIGHT = expectedColSel.getNumberOfRegions(); exportSettings = new AlignExportSettingI() { @@ -325,11 +326,12 @@ public class JSONFileTest { HiddenColumns cs = testJsonFile.getHiddenColumns(); Assert.assertNotNull(cs); - Assert.assertNotNull(cs.getHiddenColumnsCopy()); - List hiddenCols = cs.getHiddenColumnsCopy(); - Assert.assertEquals(hiddenCols.size(), TEST_CS_HEIGHT); - Assert.assertEquals(hiddenCols.get(0), expectedColSel - .getHiddenColumnsCopy().get(0), + + Iterator it = cs.iterator(); + Iterator colselit = expectedColSel.iterator(); + Assert.assertTrue(it.hasNext()); + Assert.assertEquals(cs.getNumberOfRegions(), TEST_CS_HEIGHT); + Assert.assertEquals(it.next(), colselit.next(), "Mismatched hidden columns!"); } diff --git a/test/jalview/util/MappingUtilsTest.java b/test/jalview/util/MappingUtilsTest.java index d0ec3e8..38b08cd 100644 --- a/test/jalview/util/MappingUtilsTest.java +++ b/test/jalview/util/MappingUtilsTest.java @@ -50,6 +50,7 @@ import java.awt.Color; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Iterator; import java.util.List; import org.testng.annotations.BeforeClass; @@ -913,9 +914,9 @@ public class MappingUtilsTest MappingUtils.mapColumnSelection(proteinSelection, hiddenCols, proteinView, dnaView, dnaSelection, dnaHidden); assertEquals("[]", dnaSelection.getSelected().toString()); - List hidden = dnaHidden.getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); - assertEquals("[0, 4]", Arrays.toString(hidden.get(0))); + Iterator regions = dnaHidden.iterator(); + assertEquals(1, dnaHidden.getNumberOfRegions()); + assertEquals("[0, 4]", Arrays.toString(regions.next())); /* * Column 1 in protein picks up Seq1/K which maps to cols 0-3 in dna @@ -930,9 +931,9 @@ public class MappingUtilsTest proteinSelection.hideSelectedColumns(1, hiddenCols); MappingUtils.mapColumnSelection(proteinSelection, hiddenCols, proteinView, dnaView, dnaSelection, dnaHidden); - hidden = dnaHidden.getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); - assertEquals("[0, 3]", Arrays.toString(hidden.get(0))); + regions = dnaHidden.iterator(); + assertEquals(1, dnaHidden.getNumberOfRegions()); + assertEquals("[0, 3]", Arrays.toString(regions.next())); /* * Column 2 in protein picks up gaps only - no mapping @@ -944,7 +945,7 @@ public class MappingUtilsTest proteinSelection.hideSelectedColumns(2, hiddenCols); MappingUtils.mapColumnSelection(proteinSelection, hiddenCols, proteinView, dnaView, dnaSelection, dnaHidden); - assertTrue(dnaHidden.getHiddenColumnsCopy().isEmpty()); + assertEquals(0, dnaHidden.getNumberOfRegions()); /* * Column 3 in protein picks up Seq1/P, Seq2/Q, Seq3/S which map to columns @@ -959,9 +960,9 @@ public class MappingUtilsTest MappingUtils.mapColumnSelection(proteinSelection, hiddenCols, proteinView, dnaView, dnaSelection, dnaHidden); assertEquals("[0, 1, 2, 3]", dnaSelection.getSelected().toString()); - hidden = dnaHidden.getHiddenColumnsCopy(); - assertEquals(1, hidden.size()); - assertEquals("[5, 10]", Arrays.toString(hidden.get(0))); + regions = dnaHidden.iterator(); + assertEquals(1, dnaHidden.getNumberOfRegions()); + assertEquals("[5, 10]", Arrays.toString(regions.next())); /* * Combine hiding columns 1 and 3 to get discontiguous hidden columns @@ -974,10 +975,10 @@ public class MappingUtilsTest proteinSelection.hideSelectedColumns(3, hiddenCols); MappingUtils.mapColumnSelection(proteinSelection, hiddenCols, proteinView, dnaView, dnaSelection, dnaHidden); - hidden = dnaHidden.getHiddenColumnsCopy(); - assertEquals(2, hidden.size()); - assertEquals("[0, 3]", Arrays.toString(hidden.get(0))); - assertEquals("[5, 10]", Arrays.toString(hidden.get(1))); + regions = dnaHidden.iterator(); + assertEquals(2, dnaHidden.getNumberOfRegions()); + assertEquals("[0, 3]", Arrays.toString(regions.next())); + assertEquals("[5, 10]", Arrays.toString(regions.next())); } @Test(groups = { "Functional" }) -- 1.7.10.2