JAL-2446 add call to SequenceFeatures.delete(); use varargs for type

diff --git a/src/jalview/datamodel/Sequence.java b/src/jalview/datamodel/Sequence.java
index 5a3ffa4..e1c8566 100755 (executable)
--- a/src/jalview/datamodel/Sequence.java
+++ b/src/jalview/datamodel/Sequence.java
@@ -359,6 +359,14 @@ public class Sequence extends ASequence implements SequenceI
       return;
     }
 
+    /*
+     * new way
+     */
+    sequenceFeatureStore.delete(sf);
+
+    /*
+     * old way - to be removed
+     */
     int index = 0;
     for (index = 0; index < sequenceFeatures.length; index++)
     {
@@ -1488,6 +1496,6 @@ public class Sequence extends ASequence implements SequenceI
     {
       return datasetSequence.findFeatures(type, from, to);
     }
-    return sequenceFeatureStore.findFeatures(type, from, to);
+    return sequenceFeatureStore.findFeatures(from, to, type);
   }
 }

diff --git a/src/jalview/datamodel/features/SequenceFeatures.java b/src/jalview/datamodel/features/SequenceFeatures.java
index 45d6ede..6165d0a 100644 (file)
--- a/src/jalview/datamodel/features/SequenceFeatures.java
+++ b/src/jalview/datamodel/features/SequenceFeatures.java
@@ -3,7 +3,7 @@ package jalview.datamodel.features;
 import jalview.datamodel.SequenceFeature;
 
 import java.util.ArrayList;
-import java.util.Collections;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -56,64 +56,40 @@ public class SequenceFeatures
   }
 
   /**
-   * Returns a (possibly empty) list of features of the given type which overlap
-   * the (inclusive) sequence position range
+   * Returns a (possibly empty) list of features, optionally restricted to
+   * specified types, which overlap the given (inclusive) sequence position
+   * range
    * 
-   * @param type
    * @param from
    * @param to
+   * @param type
    * @return
    */
-  public List<SequenceFeature> findFeatures(String type, int from,
-          int to)
-  {
-    FeatureStore features = featureStore.get(type);
-    if (features == null)
-    {
-      return Collections.emptyList();
-    }
-    return features.findOverlappingFeatures(from, to);
-  }
-
-  /**
-   * Answers a list of all positional features stored, in no particular
-   * guaranteed order
-   * 
-   * @return
-   */
-  public List<SequenceFeature> getPositionalFeatures()
+  public List<SequenceFeature> findFeatures(int from, int to,
+          String... type)
   {
     List<SequenceFeature> result = new ArrayList<SequenceFeature>();
-    for (FeatureStore featureSet : featureStore.values())
+
+    for (String featureType : varargToTypes(type))
     {
-      result.addAll(featureSet.getPositionalFeatures());
+      FeatureStore features = featureStore.get(featureType);
+      if (features != null)
+      {
+        result.addAll(features.findOverlappingFeatures(from, to));
+      }
     }
-    return result;
-  }
-
-  /**
-   * Answers a list of all features stored, in no particular guaranteed order
-   * 
-   * @return
-   */
-  public List<SequenceFeature> getAllFeatures()
-  {
-    List<SequenceFeature> result = new ArrayList<SequenceFeature>();
-
-    result.addAll(getPositionalFeatures());
-
-    result.addAll(getNonPositionalFeatures());
 
     return result;
   }
 
   /**
-   * Answers a list of all features stored of the specified type, in no
-   * particular guaranteed order
+   * Answers a list of all features stored, optionally restricted to specified
+   * types, in no particular guaranteed order
    * 
+   * @param type
    * @return
    */
-  public List<SequenceFeature> getAllFeatures(String type)
+  public List<SequenceFeature> getAllFeatures(String... type)
   {
     List<SequenceFeature> result = new ArrayList<SequenceFeature>();
 
@@ -125,84 +101,81 @@ public class SequenceFeatures
   }
 
   /**
-   * Answers a list of all non-positional features stored, in no particular
-   * guaranteed order
+   * Answers a list of all positional features, optionally restricted to
+   * specified types, in no particular guaranteed order
    * 
+   * @param type
    * @return
    */
-  public List<SequenceFeature> getNonPositionalFeatures()
+  public List<SequenceFeature> getPositionalFeatures(String... type)
   {
     List<SequenceFeature> result = new ArrayList<SequenceFeature>();
-    for (FeatureStore featureSet : featureStore.values())
-    {
-      result.addAll(featureSet.getNonPositionalFeatures());
-    }
-    return result;
-  }
 
-  /**
-   * Answers a list of all contact features stored, in no particular guaranteed
-   * order
-   * 
-   * @return
-   */
-  public List<SequenceFeature> getContactFeatures()
-  {
-    List<SequenceFeature> result = new ArrayList<SequenceFeature>();
-    for (FeatureStore featureSet : featureStore.values())
+    for (String featureType : varargToTypes(type))
     {
-      result.addAll(featureSet.getContactFeatures());
+      FeatureStore featureSet = featureStore.get(featureType);
+      if (featureSet != null)
+      {
+        result.addAll(featureSet.getPositionalFeatures());
+      }
     }
     return result;
   }
 
   /**
-   * Answers a list of all positional features of the given type, in no
-   * particular guaranteed order
+   * A convenience method that converts a vararg for feature types to an
+   * Iterable, replacing the value with the stored feature types if it is null
+   * or empty
    * 
+   * @param type
    * @return
    */
-  public List<SequenceFeature> getPositionalFeatures(String type)
+  protected Iterable<String> varargToTypes(String... type)
   {
-    List<SequenceFeature> result = new ArrayList<SequenceFeature>();
-    FeatureStore featureSet = featureStore.get(type);
-    if (featureSet != null)
-    {
-      result.addAll(featureSet.getPositionalFeatures());
-    }
-    return result;
+    return type == null || type.length == 0 ? featureStore
+            .keySet() : Arrays.asList(type);
   }
 
   /**
-   * Answers a list of all contact features of the given type, in no particular
-   * guaranteed order
+   * Answers a list of all contact features, optionally restricted to specified
+   * types, in no particular guaranteed order
    * 
    * @return
    */
-  public List<SequenceFeature> getContactFeatures(String type)
+  public List<SequenceFeature> getContactFeatures(String... type)
   {
     List<SequenceFeature> result = new ArrayList<SequenceFeature>();
-    FeatureStore featureSet = featureStore.get(type);
-    if (featureSet != null)
+
+    for (String featureType : varargToTypes(type))
     {
-      result.addAll(featureSet.getContactFeatures());
+      FeatureStore featureSet = featureStore.get(featureType);
+      if (featureSet != null)
+      {
+        result.addAll(featureSet.getContactFeatures());
+      }
     }
     return result;
   }
 
   /**
-   * Answers a list of all non-positional features of the given type, in no
-   * particular guaranteed order
+   * Answers a list of all non-positional features, optionally restricted to
+   * specified types, in no particular guaranteed order
    * 
+   * @param type
+   *          if no type is specified, all are returned
    * @return
    */
-  public List<SequenceFeature> getNonPositionalFeatures(String type)
+  public List<SequenceFeature> getNonPositionalFeatures(String... type)
   {
     List<SequenceFeature> result = new ArrayList<SequenceFeature>();
-    FeatureStore featureSet = featureStore.get(type);
-    if (featureSet != null)
+
+    for (String featureType : varargToTypes(type))
     {
-      result.addAll(featureSet.getNonPositionalFeatures());
+      FeatureStore featureSet = featureStore.get(featureType);
+      if (featureSet != null)
+      {
+        result.addAll(featureSet.getNonPositionalFeatures());
+      }
     }
     return result;
   }
@@ -246,19 +219,31 @@ public class SequenceFeatures
 
   /**
    * Returns a set of the distinct feature groups present in the collection. The
-   * set may include null. The parameter determines whether the groups for
-   * positional or for non-positional features are returned.
+   * set may include null. The boolean parameter determines whether the groups
+   * for positional or for non-positional features are returned. The optional
+   * type parameter may be used to restrict to groups for specified feature
+   * types.
    * 
    * @param positionalFeatures
+   * @param type
    * @return
    */
-  public Set<String> getFeatureGroups(boolean positionalFeatures)
+  public Set<String> getFeatureGroups(boolean positionalFeatures,
+          String... type)
   {
     Set<String> groups = new HashSet<String>();
-    for (FeatureStore featureSet : featureStore.values())
+
+    Iterable<String> types = varargToTypes(type);
+
+    for (String featureType : types)
     {
-      groups.addAll(featureSet.getFeatureGroups(positionalFeatures));
+      FeatureStore featureSet = featureStore.get(featureType);
+      if (featureSet != null)
+      {
+        groups.addAll(featureSet.getFeatureGroups(positionalFeatures));
+      }
     }
+
     return groups;
   }
 
@@ -276,6 +261,7 @@ public class SequenceFeatures
           String... groups)
   {
     Set<String> result = new HashSet<String>();
+
     for (Entry<String, FeatureStore> featureType : featureStore.entrySet())
     {
       Set<String> featureGroups = featureType.getValue().getFeatureGroups(

diff --git a/test/jalview/datamodel/features/SequenceFeaturesTest.java b/test/jalview/datamodel/features/SequenceFeaturesTest.java
index 4b7609a..84a07b4 100644 (file)
--- a/test/jalview/datamodel/features/SequenceFeaturesTest.java
+++ b/test/jalview/datamodel/features/SequenceFeaturesTest.java
@@ -70,7 +70,7 @@ public class SequenceFeaturesTest
     /*
      * get features by type
      */
-    assertTrue(store.getPositionalFeatures(null).isEmpty());
+    assertTrue(store.getPositionalFeatures((String) null).isEmpty());
     assertTrue(store.getPositionalFeatures("Cath").isEmpty());
     assertTrue(store.getPositionalFeatures("METAL").isEmpty());
 
@@ -131,7 +131,7 @@ public class SequenceFeaturesTest
     /*
      * get contact features by type
      */
-    assertTrue(store.getContactFeatures(null).isEmpty());
+    assertTrue(store.getContactFeatures((String) null).isEmpty());
     assertTrue(store.getContactFeatures("Cath").isEmpty());
     assertTrue(store.getContactFeatures("Pfam").isEmpty());
     assertTrue(store.getContactFeatures("DISULPHIDE BOND").isEmpty());
@@ -188,7 +188,7 @@ public class SequenceFeaturesTest
     /*
      * get non-positional features by type
      */
-    assertTrue(store.getNonPositionalFeatures(null).isEmpty());
+    assertTrue(store.getNonPositionalFeatures((String) null).isEmpty());
     assertTrue(store.getNonPositionalFeatures("Cath").isEmpty());
     assertTrue(store.getNonPositionalFeatures("PFAM").isEmpty());
   
@@ -240,36 +240,36 @@ public class SequenceFeaturesTest
     // null type is weird but possible:
     SequenceFeature sf13 = addFeature(sf, null, 5, 12);
   
-    List<SequenceFeature> overlaps = sf.findFeatures("Pfam", 200, 200);
+    List<SequenceFeature> overlaps = sf.findFeatures(200, 200, "Pfam");
     assertTrue(overlaps.isEmpty());
   
-    overlaps = sf.findFeatures("Pfam", 1, 9);
+    overlaps = sf.findFeatures( 1, 9, "Pfam");
     assertEquals(overlaps.size(), 1);
     assertTrue(overlaps.contains(sf2));
   
-    overlaps = sf.findFeatures("Pfam", 5, 18);
+    overlaps = sf.findFeatures( 5, 18, "Pfam");
     assertEquals(overlaps.size(), 2);
     assertTrue(overlaps.contains(sf1));
     assertTrue(overlaps.contains(sf2));
   
-    overlaps = sf.findFeatures("Pfam", 30, 40);
+    overlaps = sf.findFeatures(30, 40, "Pfam");
     assertEquals(overlaps.size(), 3);
     assertTrue(overlaps.contains(sf1));
     assertTrue(overlaps.contains(sf3));
     assertTrue(overlaps.contains(sf4));
   
-    overlaps = sf.findFeatures("Pfam", 80, 90);
+    overlaps = sf.findFeatures( 80, 90, "Pfam");
     assertEquals(overlaps.size(), 2);
     assertTrue(overlaps.contains(sf4));
     assertTrue(overlaps.contains(sf5));
   
-    overlaps = sf.findFeatures("Pfam", 68, 70);
+    overlaps = sf.findFeatures( 68, 70, "Pfam");
     assertEquals(overlaps.size(), 3);
     assertTrue(overlaps.contains(sf4));
     assertTrue(overlaps.contains(sf5));
     assertTrue(overlaps.contains(sf6));
 
-    overlaps = sf.findFeatures("Cath", 16, 69);
+    overlaps = sf.findFeatures(16, 69, "Cath");
     assertEquals(overlaps.size(), 4);
     assertTrue(overlaps.contains(sf7));
     assertFalse(overlaps.contains(sf8));
@@ -278,9 +278,9 @@ public class SequenceFeaturesTest
     assertTrue(overlaps.contains(sf11));
     assertFalse(overlaps.contains(sf12));
 
-    assertTrue(sf.findFeatures("Metal", 0, 1000).isEmpty());
+    assertTrue(sf.findFeatures(0, 1000, "Metal").isEmpty());
 
-    overlaps = sf.findFeatures(null, 7, 7);
+    overlaps = sf.findFeatures(7, 7, (String) null);
     assertEquals(overlaps.size(), 1);
     assertTrue(overlaps.contains(sf13));
   }