JAL-2591 Further refactoring (still incomplete)
authorkiramt <k.mourao@dundee.ac.uk>
Wed, 7 Jun 2017 17:04:43 +0000 (18:04 +0100)
committerkiramt <k.mourao@dundee.ac.uk>
Wed, 7 Jun 2017 17:04:43 +0000 (18:04 +0100)
src/jalview/appletgui/AlignFrame.java
src/jalview/appletgui/AnnotationColumnChooser.java
src/jalview/appletgui/ScalePanel.java
src/jalview/datamodel/HiddenColumns.java
src/jalview/gui/AlignFrame.java
src/jalview/gui/AnnotationColumnChooser.java
src/jalview/gui/ScalePanel.java
test/jalview/datamodel/HiddenColumnsTest.java

index a75adbb..e77ae54 100644 (file)
@@ -1900,7 +1900,7 @@ public class AlignFrame extends EmbmenuFrame implements ActionListener,
 
   static StringBuffer copiedSequences;
 
-  static Vector copiedHiddenColumns;
+  static Vector<int[]> copiedHiddenColumns;
 
   protected void copy_actionPerformed()
   {
@@ -1924,12 +1924,13 @@ public class AlignFrame extends EmbmenuFrame implements ActionListener,
 
     if (viewport.hasHiddenColumns() && viewport.getSelectionGroup() != null)
     {
-      copiedHiddenColumns = new Vector();
+      copiedHiddenColumns = viewport.getAlignment().getHiddenColumns()
+              .getHiddenColumnsCopy();
       int hiddenOffset = viewport.getSelectionGroup().getStartRes();
-      for (int[] region : viewport.getAlignment().getHiddenColumns())
+      for (int[] region : copiedHiddenColumns)
       {
-        copiedHiddenColumns.addElement(new int[] {
-            region[0] - hiddenOffset, region[1] - hiddenOffset });
+        region[0] = region[0] - hiddenOffset;
+        region[1] = region[1] - hiddenOffset;
       }
     }
     else
@@ -2044,7 +2045,7 @@ public class AlignFrame extends EmbmenuFrame implements ActionListener,
         {
           for (int i = 0; i < copiedHiddenColumns.size(); i++)
           {
-            int[] region = (int[]) copiedHiddenColumns.elementAt(i);
+            int[] region = copiedHiddenColumns.elementAt(i);
             af.viewport.hideColumns(region[0], region[1]);
           }
         }
index c49a5f3..bb67efc 100644 (file)
@@ -46,7 +46,7 @@ import java.awt.event.MouseEvent;
 import java.awt.event.MouseListener;
 import java.awt.event.TextEvent;
 import java.awt.event.TextListener;
-import java.util.Iterator;
+import java.util.ArrayList;
 import java.util.Vector;
 
 //import javax.swing.JPanel;
@@ -300,12 +300,14 @@ public class AnnotationColumnChooser extends AnnotationRowFilter implements
                 .getOldHiddenColumns();
         if (oldHidden != null)
         {
-          for (Iterator<int[]> itr = oldHidden.iterator(); itr.hasNext();)
+          ArrayList<int[]> regions = oldHidden.getHiddenColumnsCopyAsList();
+          for (int[] positions : regions)
           {
-            int positions[] = itr.next();
             av.hideColumns(positions[0], positions[1]);
           }
         }
+        // TODO not clear why we need to hide all the columns (above) if we are
+        // going to copy the hidden columns over wholesale anyway
         av.getAlignment().setHiddenColumns(oldHidden);
       }
       ap.paintAlignment(true);
index 5bb0f75..2abd65a 100755 (executable)
@@ -334,17 +334,8 @@ public class ScalePanel extends Panel implements MouseMotionListener,
     int res = (evt.getX() / av.getCharWidth())
             + av.getRanges().getStartRes();
 
-    res = av.getAlignment().getHiddenColumns().adjustForHiddenColumns(res);
-
-    reveal = null;
-    for (int[] region : av.getAlignment().getHiddenColumns())
-    {
-      if (res + 1 == region[0] || res - 1 == region[1])
-      {
-        reveal = region;
-        break;
-      }
-    }
+    reveal = av.getAlignment().getHiddenColumns()
+            .getRegionWithEdgeAtRes(res);
 
     repaint();
   }
index be10721..5966312 100644 (file)
@@ -67,13 +67,16 @@ public class HiddenColumns
     {
       LOCK.readLock().lock();
       StringBuilder regionBuilder = new StringBuilder();
-      for (int[] range : hiddenColumns)
+      if (hiddenColumns != null)
       {
-        regionBuilder.append(delimiter).append(range[0]).append(between)
-                .append(range[1]);
-      }
+        for (int[] range : hiddenColumns)
+        {
+          regionBuilder.append(delimiter).append(range[0]).append(between)
+                  .append(range[1]);
+        }
 
-      regionBuilder.deleteCharAt(0);
+        regionBuilder.deleteCharAt(0);
+      }
       return regionBuilder.toString();
     } finally
     {
@@ -331,29 +334,37 @@ public class HiddenColumns
     try
     {
       LOCK.readLock().lock();
-      List<Integer> positions = new ArrayList<>(
-              hiddenColumns.size());
+      List<Integer> positions = null;
 
-      positions.add(hiddenColumns.elementAt(0)[0]);
-      for (int i = 1; i < hiddenColumns.size(); ++i)
+      if (hiddenColumns != null)
       {
+        positions = new ArrayList<>(hiddenColumns.size());
 
-        int result = 0;
-        if (hiddenColumns != null)
+        positions.add(hiddenColumns.elementAt(0)[0]);
+        for (int i = 1; i < hiddenColumns.size(); ++i)
         {
-          int index = 0;
-          int gaps = 0;
-          do
+
+          int result = 0;
+          if (hiddenColumns != null)
           {
-            int[] region = hiddenColumns.elementAt(index);
-            gaps += region[1] + 1 - region[0];
-            result = region[1] + 1;
-            index++;
-          } while (index <= i);
+            int index = 0;
+            int gaps = 0;
+            do
+            {
+              int[] region = hiddenColumns.elementAt(index);
+              gaps += region[1] + 1 - region[0];
+              result = region[1] + 1;
+              index++;
+            } while (index <= i);
 
-          result -= gaps;
+            result -= gaps;
+          }
+          positions.add(result);
         }
-        positions.add(result);
+      }
+      else
+      {
+        positions = new ArrayList<>();
       }
 
       return positions;
@@ -665,6 +676,13 @@ public class HiddenColumns
     return copy;
   }
 
+  /**
+   * Returns a copy of the vector of hidden regions, as a vector. 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 vector of [start,end] pairs
+   */
   public Vector<int[]> getHiddenColumnsCopy()
   {
     try
@@ -677,6 +695,14 @@ 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<int[]> getHiddenColumnsCopyAsList()
   {
     try
@@ -1711,4 +1737,37 @@ public class HiddenColumns
 
   }
 
+  /**
+   * Finds the hidden region (if any) which starts or ends at res
+   * 
+   * @param res
+   *          visible residue position, unadjusted for hidden columns
+   * @return region as [start,end] or null if no matching region is found
+   */
+  public int[] getRegionWithEdgeAtRes(int res)
+  {
+    try
+    {
+      LOCK.readLock().lock();
+      int adjres = adjustForHiddenColumns(res);
+
+      int[] reveal = null;
+      if (hiddenColumns != null)
+      {
+        for (int[] region : hiddenColumns)
+        {
+          if (adjres + 1 == region[0] || adjres - 1 == region[1])
+          {
+            reveal = region;
+            break;
+          }
+        }
+      }
+      return reveal;
+    } finally
+    {
+      LOCK.readLock().unlock();
+    }
+  }
+
 }
index ae2e44a..2c7837f 100644 (file)
@@ -1885,7 +1885,9 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
       hiddenColumns = new ArrayList<>();
       int hiddenOffset = viewport.getSelectionGroup().getStartRes();
       int hiddenCutoff = viewport.getSelectionGroup().getEndRes();
-      for (int[] region : viewport.getAlignment().getHiddenColumns())
+      ArrayList<int[]> hiddenRegions = viewport.getAlignment()
+              .getHiddenColumns().getHiddenColumnsCopyAsList();
+      for (int[] region : hiddenRegions)
       {
         if (region[0] >= hiddenOffset && region[1] <= hiddenCutoff)
         {
index 405b43b..311b9d5 100644 (file)
@@ -36,7 +36,7 @@ import java.awt.event.ActionListener;
 import java.awt.event.ItemEvent;
 import java.awt.event.ItemListener;
 import java.awt.event.KeyEvent;
-import java.util.Iterator;
+import java.util.ArrayList;
 
 import javax.swing.ButtonGroup;
 import javax.swing.JCheckBox;
@@ -246,12 +246,14 @@ public class AnnotationColumnChooser extends AnnotationRowFilter implements
                 .getOldHiddenColumns();
         if (oldHidden != null)
         {
-          for (Iterator<int[]> itr = oldHidden.iterator(); itr.hasNext();)
+          ArrayList<int[]> regions = oldHidden.getHiddenColumnsCopyAsList();
+          for (int[] positions : regions)
           {
-            int positions[] = itr.next();
             av.hideColumns(positions[0], positions[1]);
           }
         }
+        // TODO not clear why we need to hide all the columns (above) if we are
+        // going to copy the hidden columns over wholesale anyway
         av.getAlignment().setHiddenColumns(oldHidden);
       }
       ap.paintAlignment(true);
index dcaafcd..3cdba8d 100755 (executable)
@@ -401,20 +401,15 @@ public class ScalePanel extends JPanel implements MouseMotionListener,
     int res = (evt.getX() / av.getCharWidth())
             + av.getRanges().getStartRes();
 
+    reveal = av.getAlignment().getHiddenColumns()
+            .getRegionWithEdgeAtRes(res);
+
     res = av.getAlignment().getHiddenColumns().adjustForHiddenColumns(res);
 
-    for (int[] region : av.getAlignment().getHiddenColumns())
-    {
-      if (res + 1 == region[0] || res - 1 == region[1])
-      {
-        reveal = region;
-        ToolTipManager.sharedInstance().registerComponent(this);
-        this.setToolTipText(
-                MessageManager.getString("label.reveal_hidden_columns"));
-        repaint();
-        return;
-      }
-    }
+    ToolTipManager.sharedInstance().registerComponent(this);
+    this.setToolTipText(
+            MessageManager.getString("label.reveal_hidden_columns"));
+    repaint();
   }
 
   /**
index 5b92677..62c2d22 100644 (file)
@@ -20,6 +20,7 @@
  */
 package jalview.datamodel;
 
+import static org.testng.Assert.assertNull;
 import static org.testng.AssertJUnit.assertEquals;
 import static org.testng.AssertJUnit.assertFalse;
 import static org.testng.AssertJUnit.assertSame;
@@ -590,11 +591,15 @@ public class HiddenColumnsTest
   public void testFindHiddenRegionPositions()
   {
     HiddenColumns hc = new HiddenColumns();
+
+    List<Integer> positions = hc.findHiddenRegionPositions();
+    assertTrue(positions.isEmpty());
+
     hc.hideColumns(3, 7);
     hc.hideColumns(10, 10);
     hc.hideColumns(14, 15);
 
-    List<Integer> positions = hc.findHiddenRegionPositions();
+    positions = hc.findHiddenRegionPositions();
     assertEquals(3, positions.size());
     assertEquals(3, positions.get(0).intValue());
     assertEquals(5, positions.get(1).intValue());
@@ -605,11 +610,15 @@ public class HiddenColumnsTest
   public void testRegionsToString()
   {
     HiddenColumns hc = new HiddenColumns();
+
+    String result = hc.regionsToString(",", "--");
+    assertEquals("", result);
+
     hc.hideColumns(3, 7);
     hc.hideColumns(10, 10);
     hc.hideColumns(14, 15);
 
-    String result = hc.regionsToString(",", "--");
+    result = hc.regionsToString(",", "--");
     assertEquals("3--7,10--10,14--15", result);
   }
 
@@ -642,4 +651,28 @@ public class HiddenColumnsTest
     assertEquals(23, startEnd[1]);
   }
 
+  @Test(groups = "Functional")
+  public void testGetRegionWithEdgeAtRes()
+  {
+    HiddenColumns hc = new HiddenColumns();
+
+    int[] result = hc.getRegionWithEdgeAtRes(5);
+    assertNull(result);
+
+    hc.hideColumns(3, 7);
+    hc.hideColumns(10, 10);
+    hc.hideColumns(14, 15);
+
+    result = hc.getRegionWithEdgeAtRes(3);
+    assertEquals(3, result[0]);
+    assertEquals(7, result[1]);
+
+    result = hc.getRegionWithEdgeAtRes(5);
+    assertEquals(10, result[0]);
+    assertEquals(10, result[1]);
+
+    result = hc.getRegionWithEdgeAtRes(6);
+    assertNull(result);
+  }
+
 }