From 405e8b16ccb9b1297e1158ace036f0b1634dac67 Mon Sep 17 00:00:00 2001
From: kiramt <k.mourao@dundee.ac.uk>
Date: Wed, 31 Jan 2018 08:17:08 +0000
Subject: [PATCH] JAL-2759 minor refactor to hiding bitsets

---
 src/jalview/datamodel/HiddenColumns.java      |   46 +++++++++++++++++--------
 src/jalview/gui/PopupMenu.java                |    2 +-
 test/jalview/datamodel/HiddenColumnsTest.java |   16 ++++-----
 3 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/src/jalview/datamodel/HiddenColumns.java b/src/jalview/datamodel/HiddenColumns.java
index 17d3fe4..45dcab2 100644
--- a/src/jalview/datamodel/HiddenColumns.java
+++ b/src/jalview/datamodel/HiddenColumns.java
@@ -772,15 +772,39 @@ public class HiddenColumns
    */
   public void hideColumns(BitSet inserts)
   {
+    hideColumns(inserts, 0, inserts.length() - 1);
+  }
+
+  /**
+   * Hide columns corresponding to the marked bits, within the range
+   * [start,end]. Entries in tohide which are outside [start,end] are ignored.
+   * 
+   * @param tohide
+   *          columns mapped to bits starting from zero
+   * @param start
+   *          start of range to hide columns within
+   * @param end
+   *          end of range to hide columns within
+   */
+  private void hideColumns(BitSet tohide, int start, int end)
+  {
     try
     {
       LOCK.writeLock().lock();
-      for (int firstSet = inserts
-              .nextSetBit(0), lastSet = 0; firstSet >= 0; firstSet = inserts
+      for (int firstSet = tohide
+              .nextSetBit(start), lastSet = start; firstSet >= start
+                      && lastSet <= end; firstSet = tohide
                       .nextSetBit(lastSet))
       {
-        lastSet = inserts.nextClearBit(firstSet);
-        hideColumns(firstSet, lastSet - 1);
+        lastSet = tohide.nextClearBit(firstSet);
+        if (lastSet <= end)
+        {
+          hideColumns(firstSet, lastSet - 1);
+        }
+        else if (firstSet <= end)
+        {
+          hideColumns(firstSet, end);
+        }
       }
       cursor = new HiddenColumnsCursor(hiddenColumns);
     } finally
@@ -792,6 +816,7 @@ public class HiddenColumns
   /**
    * Hide columns corresponding to the marked bits, within the range
    * [start,end]. Entries in tohide which are outside [start,end] are ignored.
+   * NB Existing entries in [start,end] are cleared.
    * 
    * @param tohide
    *          columns mapped to bits starting from zero
@@ -800,19 +825,10 @@ public class HiddenColumns
    * @param end
    *          end of range to hide columns within
    */
-  public void hideColumns(BitSet tohide, int start, int end)
+  public void clearAndHideColumns(BitSet tohide, int start, int end)
   {
     clearHiddenColumnsInRange(start, end);
-
-    // make sure only bits between start and end are set
-    if (!tohide.isEmpty())
-    {
-      tohide.clear(0, start);
-      tohide.clear(Math.min(end + 1, tohide.length() + 1),
-              tohide.length() + 1);
-    }
-
-    hideColumns(tohide);
+    hideColumns(tohide, start, end);
   }
 
   /**
diff --git a/src/jalview/gui/PopupMenu.java b/src/jalview/gui/PopupMenu.java
index 9bc2ca1..759c63b 100644
--- a/src/jalview/gui/PopupMenu.java
+++ b/src/jalview/gui/PopupMenu.java
@@ -1473,7 +1473,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
         }
         inserts.and(sq.getInsertionsAsBits());
       }
-      hidden.hideColumns(inserts, ap.av.getSelectionGroup().getStartRes(),
+      hidden.clearAndHideColumns(inserts, ap.av.getSelectionGroup().getStartRes(),
               ap.av.getSelectionGroup().getEndRes());
     }
 
diff --git a/test/jalview/datamodel/HiddenColumnsTest.java b/test/jalview/datamodel/HiddenColumnsTest.java
index 38536c8..1cc9a96 100644
--- a/test/jalview/datamodel/HiddenColumnsTest.java
+++ b/test/jalview/datamodel/HiddenColumnsTest.java
@@ -868,7 +868,7 @@ public class HiddenColumnsTest
     HiddenColumns h = new HiddenColumns();
     HiddenColumns h2 = new HiddenColumns();
 
-    BitSet tohide = new BitSet(21);
+    BitSet tohide = new BitSet(25);
     h.hideColumns(tohide);
     assertTrue(h.equals(h2));
 
@@ -876,7 +876,7 @@ public class HiddenColumnsTest
     tohide.set(3, 6);
     tohide.set(9);
     tohide.set(15, 21);
-    h.hideColumns(tohide, 5, 23);
+    h.clearAndHideColumns(tohide, 5, 23);
 
     h2.hideColumns(5, 5);
     h2.hideColumns(9, 9);
@@ -886,19 +886,19 @@ public class HiddenColumnsTest
 
     tohide.clear();
     tohide.set(41);
-    h.hideColumns(tohide, 23, 30);
+    h.clearAndHideColumns(tohide, 23, 30);
     assertTrue(h.equals(h2));
     assertEquals(h.getSize(), h2.getSize());
 
     tohide.set(41);
-    h.hideColumns(tohide, 30, 45);
+    h.clearAndHideColumns(tohide, 30, 45);
     h2.hideColumns(41, 41);
     assertTrue(h.equals(h2));
     assertEquals(h.getSize(), h2.getSize());
 
     tohide.clear();
     tohide.set(25, 28);
-    h.hideColumns(tohide, 17, 50);
+    h.clearAndHideColumns(tohide, 17, 50);
     h2 = new HiddenColumns();
     h2.hideColumns(5, 5);
     h2.hideColumns(9, 9);
@@ -915,7 +915,7 @@ public class HiddenColumnsTest
     tohide = new BitSet();
 
     // all unhidden if tohide is empty and range covers hidden
-    hc.hideColumns(tohide, 1, 70);
+    hc.clearAndHideColumns(tohide, 1, 70);
     assertTrue(!hc.hasHiddenColumns());
     assertEquals(0, hc.getSize());
 
@@ -925,12 +925,12 @@ public class HiddenColumnsTest
     assertEquals(25, hc.getSize());
 
     // but not if range does not cover hidden
-    hc.hideColumns(tohide, 23, 40);
+    hc.clearAndHideColumns(tohide, 23, 40);
     assertTrue(hc.hasHiddenColumns());
     assertEquals(25, hc.getSize());
 
     // and partial unhide if range partially covers
-    hc.hideColumns(tohide, 1, 17);
+    hc.clearAndHideColumns(tohide, 1, 17);
     Iterator<int[]> it = hc.iterator();
     assertTrue(it.hasNext());
     int[] region = it.next();
-- 
1.7.10.2