JAL-2225 updates for review comments, Checkstyle

diff --git a/src/jalview/gui/StructureViewer.java b/src/jalview/gui/StructureViewer.java
index a1913c6..b142613 100644 (file)
--- a/src/jalview/gui/StructureViewer.java
+++ b/src/jalview/gui/StructureViewer.java
@@ -36,16 +36,17 @@ import java.util.Map;
 import java.util.Map.Entry;
 
 /**
- * proxy for handling structure viewers.
- * 
- * this allows new views to be created with the currently configured viewer, the
- * preferred viewer to be set/read and existing views created previously with a
- * particular viewer to be recovered
+ * A proxy for handling structure viewers, that orchestrates adding selected
+ * structures, associated with sequences in Jalview, to an existing viewer, or
+ * opening a new one. Currently supports either Jmol or Chimera as the structure
+ * viewer.
  * 
  * @author jprocter
  */
 public class StructureViewer
 {
+  private static final String UNKNOWN_VIEWER_TYPE = "Unknown structure viewer type ";
+
   StructureSelectionManager ssm;
 
   public enum ViewerType
@@ -53,6 +54,16 @@ public class StructureViewer
     JMOL, CHIMERA
   };
 
+  /**
+   * Constructor
+   * 
+   * @param structureSelectionManager
+   */
+  public StructureViewer(StructureSelectionManager structureSelectionManager)
+  {
+    ssm = structureSelectionManager;
+  }
+
   public ViewerType getViewerType()
   {
     String viewType = Cache.getDefault(Preferences.STRUCTURE_DISPLAY,
@@ -65,24 +76,18 @@ public class StructureViewer
     Cache.setProperty(Preferences.STRUCTURE_DISPLAY, type.name());
   }
 
-  public StructureViewer(
-          StructureSelectionManager structureSelectionManager)
-  {
-    ssm = structureSelectionManager;
-  }
-
   /**
    * View multiple PDB entries, each with associated sequences
    * 
    * @param pdbs
-   * @param seqsForPdbs
+   * @param seqs
    * @param ap
    * @return
    */
   public JalviewStructureDisplayI viewStructures(PDBEntry[] pdbs,
-          SequenceI[] seqsForPdbs, AlignmentPanel ap)
+          SequenceI[] seqs, AlignmentPanel ap)
   {
-    JalviewStructureDisplayI viewer = onlyOnePdb(pdbs, seqsForPdbs, ap);
+    JalviewStructureDisplayI viewer = onlyOnePdb(pdbs, seqs, ap);
     if (viewer != null)
     {
       /*
@@ -93,31 +98,24 @@ public class StructureViewer
 
     ViewerType viewerType = getViewerType();
 
-    // old way:
-    // PDBEntry[] pdbsForFile = getUniquePdbFiles(pdbs);
-
-    // new way:
-    Map<PDBEntry, SequenceI[]> seqsForPdb = getSequencesForPdbs(pdbs,
-            seqsForPdbs);
-    PDBEntry[] pdbsForFile = seqsForPdb.keySet().toArray(
-            new PDBEntry[seqsForPdb.size()]);
-    SequenceI[][] theSeqs = seqsForPdb.values().toArray(
-            new SequenceI[seqsForPdb.size()][]);
+    Map<PDBEntry, SequenceI[]> seqsForPdbs = getSequencesForPdbs(pdbs,
+            seqs);
+    PDBEntry[] pdbsForFile = seqsForPdbs.keySet().toArray(
+            new PDBEntry[seqsForPdbs.size()]);
+    SequenceI[][] theSeqs = seqsForPdbs.values().toArray(
+            new SequenceI[seqsForPdbs.size()][]);
     JalviewStructureDisplayI sview = null;
     if (viewerType.equals(ViewerType.JMOL))
     {
       sview = new AppJmol(ap, pdbsForFile, theSeqs);
-      // ap.av.collateForPDB(pdbsForFile));
     }
     else if (viewerType.equals(ViewerType.CHIMERA))
     {
       sview = new ChimeraViewFrame(pdbsForFile, theSeqs, ap);
-      // ap.av.collateForPDB(pdbsForFile), ap);
     }
     else
     {
-      Cache.log.error("Unknown structure viewer type "
-              + getViewerType().toString());
+      Cache.log.error(UNKNOWN_VIEWER_TYPE + getViewerType().toString());
     }
     return sview;
   }
@@ -132,7 +130,7 @@ public class StructureViewer
    * @param seqs
    * @return
    */
-  static Map<PDBEntry, SequenceI[]> getSequencesForPdbs(PDBEntry[] pdbs,
+  Map<PDBEntry, SequenceI[]> getSequencesForPdbs(PDBEntry[] pdbs,
           SequenceI[] seqs)
   {
     if (pdbs == null || seqs == null || pdbs.length != seqs.length)
@@ -240,8 +238,7 @@ public class StructureViewer
     }
     else
     {
-      Cache.log.error("Unknown structure viewer type "
-              + getViewerType().toString());
+      Cache.log.error(UNKNOWN_VIEWER_TYPE + getViewerType().toString());
     }
     return sview;
   }
@@ -281,7 +278,7 @@ public class StructureViewer
               "Unsupported structure viewer type " + type.toString());
       break;
     default:
-      Cache.log.error("Unknown structure viewer type " + type.toString());
+      Cache.log.error(UNKNOWN_VIEWER_TYPE + type.toString());
     }
     return sview;
   }

diff --git a/test/jalview/gui/StructureViewerTest.java b/test/jalview/gui/StructureViewerTest.java
index 335863b..4d5b114 100644 (file)
--- a/test/jalview/gui/StructureViewerTest.java
+++ b/test/jalview/gui/StructureViewerTest.java
@@ -29,7 +29,9 @@ public class StructureViewerTest
   @Test(groups = "Functional")
   public void testGetSequencesForPdbs()
   {
-    assertNull(StructureViewer.getSequencesForPdbs(null, null));
+    StructureViewer sv = new StructureViewer(null);
+
+    assertNull(sv.getSequencesForPdbs(null, null));
 
     PDBEntry pdbe1 = new PDBEntry("1A70", "A", Type.PDB, "path1");
     PDBEntry pdbe2 = new PDBEntry("3A6S", "A", Type.PDB, "path2");
@@ -52,8 +54,7 @@ public class StructureViewerTest
     /*
      * pdbe3/5/6 should get removed as having a duplicate file path
      */
-    Map<PDBEntry, SequenceI[]> uniques = StructureViewer
-            .getSequencesForPdbs(pdbs, seqs);
+    Map<PDBEntry, SequenceI[]> uniques = sv.getSequencesForPdbs(pdbs, seqs);
     assertTrue(uniques.containsKey(pdbe1));
     assertTrue(uniques.containsKey(pdbe2));
     assertFalse(uniques.containsKey(pdbe3));