JAL-2490 tidy code and tests for feature export in Jalview format

[jalview.git] / src / jalview / io / FeaturesFile.java

diff --git a/src/jalview/io/FeaturesFile.java b/src/jalview/io/FeaturesFile.java
index 028b14f..869b18b 100755 (executable)
--- a/src/jalview/io/FeaturesFile.java
+++ b/src/jalview/io/FeaturesFile.java
@@ -497,43 +497,24 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
     {
       sf.addLink(link);
     }
-
   }
 
   /**
-   * generate a features file for seqs includes non-pos features by default.
-   * 
-   * @param sequences
-   *          source of sequence features
-   * @param visible
-   *          hash of feature types and colours
-   * @return features file contents
-   */
-  public String printJalviewFormat(SequenceI[] sequences,
-          Map<String, FeatureColourI> visible)
-  {
-    return printJalviewFormat(sequences, visible, true, true);
-  }
-
-  /**
-   * generate a features file for seqs with colours from visible (if any)
+   * Returns contents of a Jalview format features file
    * 
    * @param sequences
    *          source of features
    * @param visible
-   *          hash of Colours for each feature type
-   * @param visOnly
-   *          when true only feature types in 'visible' will be output
-   * @param nonpos
-   *          indicates if non-positional features should be output (regardless
-   *          of group or type)
-   * @return features file contents
+   *          map of colour for each visible feature type
+   * @param includeNonPositional
+   *          if true, include non-positional features (regardless of group or
+   *          type)
+   * @return
    */
   public String printJalviewFormat(SequenceI[] sequences,
-          Map<String, FeatureColourI> visible, boolean visOnly,
-          boolean nonpos)
+          Map<String, FeatureColourI> visible, boolean includeNonPositional)
   {
-    if (visOnly && !nonpos && (visible == null || visible.isEmpty()))
+    if (!includeNonPositional && (visible == null || visible.isEmpty()))
     {
       // no point continuing.
       return "No Features Visible";
@@ -544,7 +525,7 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
      */
     // TODO: decide if feature links should also be written here ?
     StringBuilder out = new StringBuilder(256);
-    if (visible != null && visOnly) // todo why visOnly test?
+    if (visible != null)
     {
       for (Entry<String, FeatureColourI> featureColour : visible.entrySet())
       {
@@ -556,14 +537,14 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
 
     // Work out which groups are both present and visible
     Set<String> groups = new HashSet<String>();
-    String[] types = visible == null ? null : visible.keySet().toArray(
-            new String[visible.keySet().size()]);
+    String[] types = visible == null ? new String[0] : visible.keySet()
+            .toArray(new String[visible.keySet().size()]);
 
     for (int i = 0; i < sequences.length; i++)
     {
       groups.addAll(sequences[i].getFeatures()
               .getFeatureGroups(true, types));
-      if (nonpos)
+      if (includeNonPositional)
       {
         groups.addAll(sequences[i].getFeatures().getFeatureGroups(false,
                 types));
@@ -594,13 +575,16 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
       for (int i = 0; i < sequences.length; i++)
       {
         List<SequenceFeature> features = new ArrayList<SequenceFeature>();
-        if (nonpos)
+        if (includeNonPositional)
         {
           features.addAll(sequences[i].getFeatures().getFeaturesForGroup(
                   false, group, types));
         }
-        features.addAll(sequences[i].getFeatures().getFeaturesForGroup(
-                true, group, types));
+        if (types.length > 0)
+        {
+          features.addAll(sequences[i].getFeatures().getFeaturesForGroup(
+                  true, group, types));
+        }
 
         for (SequenceFeature sequenceFeature : features)
         {
@@ -722,34 +706,17 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
   }
 
   /**
-   * Returns features output in GFF2 format, including hidden and non-positional
-   * features
-   * 
-   * @param sequences
-   *          the sequences whose features are to be output
-   * @param visible
-   *          a map whose keys are the type names of visible features
-   * @return
-   */
-  public String printGffFormat(SequenceI[] sequences,
-          Map<String, FeatureColourI> visible)
-  {
-    return printGffFormat(sequences, visible, true, true);
-  }
-
-  /**
    * Returns features output in GFF2 format
    * 
    * @param sequences
    *          the sequences whose features are to be output
    * @param visible
    *          a map whose keys are the type names of visible features
-   * @param outputVisibleOnly
    * @param includeNonPositionalFeatures
    * @return
    */
   public String printGffFormat(SequenceI[] sequences,
-          Map<String, FeatureColourI> visible, boolean outputVisibleOnly,
+          Map<String, FeatureColourI> visible,
           boolean includeNonPositionalFeatures)
   {
     StringBuilder out = new StringBuilder(256);
@@ -772,10 +739,7 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
              */
             continue;
           }
-          // TODO why the test !isnonpos here?
-          // what about not visible non-positional features?
-          if (!isnonpos && outputVisibleOnly
-                  && !visible.containsKey(sf.type))
+          if (!isnonpos && !visible.containsKey(sf.type))
           {
             /*
              * ignore not visible features if not wanted