JAL-2591 Updates following code review

diff --git a/src/jalview/datamodel/ColumnSelection.java b/src/jalview/datamodel/ColumnSelection.java
index eb2d174..4cdd7af 100644 (file)
--- a/src/jalview/datamodel/ColumnSelection.java
+++ b/src/jalview/datamodel/ColumnSelection.java
@@ -482,7 +482,7 @@ public class ColumnSelection
    */
   public void invertColumnSelection(int first, int width, AlignmentI al)
   {
-    boolean hasHidden = al.getHiddenColumns().hasHidden();
+    boolean hasHidden = al.getHiddenColumns().hasHiddenColumns();
     for (int i = first; i < width; i++)
     {
       if (contains(i))
@@ -511,7 +511,7 @@ public class ColumnSelection
     selection = new IntList();
     if (colsel.selection != null && colsel.selection.size() > 0)
     {
-      if (hiddenColumns.hasHidden())
+      if (hiddenColumns.hasHiddenColumns())
       {
         // only select visible columns in this columns selection
         for (Integer col : colsel.getSelected())

diff --git a/src/jalview/datamodel/HiddenColumns.java b/src/jalview/datamodel/HiddenColumns.java
index 65ff9a5..d34b316 100644 (file)
--- a/src/jalview/datamodel/HiddenColumns.java
+++ b/src/jalview/datamodel/HiddenColumns.java
@@ -56,8 +56,7 @@ public class HiddenColumns
   {
     try
     {
-
-      LOCK.readLock().lock();
+      LOCK.writeLock().lock();
       if (copy != null)
       {
         if (copy.hiddenColumns != null)
@@ -67,7 +66,7 @@ public class HiddenColumns
       }
     } finally
     {
-      LOCK.readLock().unlock();
+      LOCK.writeLock().unlock();
     }
   }
 
@@ -128,7 +127,7 @@ public class HiddenColumns
     {
       LOCK.readLock().lock();
       int size = 0;
-      if (hasHidden())
+      if (hasHiddenColumns())
       {
         for (int[] range : hiddenColumns)
         {
@@ -143,24 +142,6 @@ public class HiddenColumns
     }
   }
 
-  /**
-   * Answers if there are any hidden columns
-   * 
-   * @return true if there are hidden columns
-   */
-  public boolean hasHidden()
-  {
-    try
-    {
-      LOCK.readLock().lock();
-      return (hiddenColumns != null) && (!hiddenColumns.isEmpty());
-    } finally
-    {
-      LOCK.readLock().unlock();
-    }
-
-  }
-
   @Override
   public boolean equals(Object obj)
   {
@@ -523,24 +504,19 @@ public class HiddenColumns
    */
   public void hideColumns(int start, int end)
   {
-    hideColumns(start, end, false);
-  }
-
-  /**
-   * Adds the specified column range to the hidden columns
-   * 
-   * @param start
-   * @param end
-   */
-  private void hideColumns(int start, int end, boolean alreadyLocked)
-  {
+    boolean wasAlreadyLocked = false;
     try
     {
-
-      if (!alreadyLocked)
+      // check if the write lock was already locked by this thread,
+      // as this method can be called internally in loops within HiddenColumns
+      if (!LOCK.isWriteLockedByCurrentThread())
       {
         LOCK.writeLock().lock();
       }
+      else
+      {
+        wasAlreadyLocked = true;
+      }
 
       if (hiddenColumns == null)
       {
@@ -610,7 +586,7 @@ public class HiddenColumns
       hiddenColumns.add(new int[] { start, end });
     } finally
     {
-      if (!alreadyLocked)
+      if (!wasAlreadyLocked)
       {
         LOCK.writeLock().unlock();
       }
@@ -1191,7 +1167,7 @@ public class HiddenColumns
       List<int[]> inserts = sr.getInsertions();
       for (int[] r : inserts)
       {
-        hideColumns(r[0], r[1], true);
+        hideColumns(r[0], r[1]);
       }
     } finally
     {
@@ -1620,7 +1596,7 @@ public class HiddenColumns
                       .nextSetBit(lastSet))
       {
         lastSet = inserts.nextClearBit(firstSet);
-        hideColumns(firstSet, lastSet - 1, true);
+        hideColumns(firstSet, lastSet - 1);
       }
     } finally
     {