From e48cf231d302b758416cc9a3b38939853cf6b3bf Mon Sep 17 00:00:00 2001
From: kiramt <k.mourao@dundee.ac.uk>
Date: Mon, 22 Jan 2018 10:06:25 +0000
Subject: [PATCH] JAL-2759 Various minor HiddenColumns updates after review

---
 src/jalview/datamodel/HiddenColumns.java |   45 ++++++++++++++----------------
 1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/src/jalview/datamodel/HiddenColumns.java b/src/jalview/datamodel/HiddenColumns.java
index ab71cbe..b903826 100644
--- a/src/jalview/datamodel/HiddenColumns.java
+++ b/src/jalview/datamodel/HiddenColumns.java
@@ -33,13 +33,13 @@ import java.util.concurrent.locks.ReentrantReadWriteLock;
  * use an iterator obtained from one of:
  * 
  * - getBoundedIterator: iterates over the hidden regions, within some bounds,
- * returning absolute positions
+ * returning *absolute* positions
  * 
  * - getBoundedStartIterator: iterates over the start positions of hidden
- * regions, within some bounds, returning visible positions
+ * regions, within some bounds, returning *visible* positions
  * 
  * - getVisContigsIterator: iterates over visible regions in a range, returning
- * absolute positions
+ * *absolute* positions
  * 
  * - getVisibleColsIterator: iterates over the visible *columns*
  * 
@@ -47,7 +47,20 @@ import java.util.concurrent.locks.ReentrantReadWriteLock;
  * numbering begins at 0 throughout this class.
  * 
  * @author kmourao
- *
+ */
+
+/* Implementation notes:
+ * 
+ * Methods which change the hiddenColumns collection should use a writeLock to
+ * prevent other threads accessing the hiddenColumns collection while changes
+ * are being made. They should also reset the hidden columns cursor, and either
+ * update the hidden columns count, or set it to 0 (so that it will later be
+ * updated when needed).
+ * 
+ * 
+ * Methods which only need read access to the hidden columns collection should
+ * use a readLock to prevent other threads changing the hidden columns
+ * collection while it is in use.
  */
 public class HiddenColumns
 {
@@ -79,14 +92,6 @@ public class HiddenColumns
   {
   }
 
-  /* 
-   * Methods which change the hiddenColumns collection. These methods should 
-   * use a writeLock to prevent other threads accessing the hiddenColumns
-   * collection while changes are being made. They should also reset the hidden
-   * columns cursor, and either update the hidden columns count, or set it to 0 
-   * (so that it will later be updated when needed).
-   */
-
   /**
    * Copy constructor
    * 
@@ -273,9 +278,9 @@ public class HiddenColumns
         }
         region[1] = Math.max(nextRegion[1], end);
 
-        // in theory this is faster than hiddenColumns.remove(i+1)
-        // benchmarking results a bit ambivalent
-        hiddenColumns.subList(i + 1, i + 2).clear();
+        // in theory hiddenColumns.subList(i + 1, i + 2).clear() is faster than
+        // hiddenColumns.remove(i+1) but benchmarking results a bit ambivalent
+        hiddenColumns.remove(i + 1);
       }
       added = true;
     }
@@ -383,15 +388,6 @@ public class HiddenColumns
     }
   }
 
-
-
-
-  /*
-   * Methods which only need read access to the hidden columns collection. 
-   * These methods should use a readLock to prevent other threads changing
-   * the hidden columns collection while it is in use.
-   */
-
   /**
    * Output regions data as a string. String is in the format:
    * reg0[0]<between>reg0[1]<delimiter>reg1[0]<between>reg1[1] ... regn[1]
@@ -824,6 +820,7 @@ public class HiddenColumns
     {
       LOCK.readLock().lock();
       int hashCode = 1;
+
       for (int[] hidden : hiddenColumns)
       {
         hashCode = HASH_MULTIPLIER * hashCode + hidden[0];
-- 
1.7.10.2