From cbf1095b9a0ccd661d48c9aca53bad3f6681a40f Mon Sep 17 00:00:00 2001
From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Tue, 18 Apr 2017 11:53:38 +0100
Subject: [PATCH] JAL-2480 subtract from total length on deleting feature

---
 src/jalview/datamodel/features/FeatureStore.java   |   40 +++++++++++++++---
 .../datamodel/features/FeatureStoreTest.java       |   43 +++++++++++++++++---
 2 files changed, 72 insertions(+), 11 deletions(-)

diff --git a/src/jalview/datamodel/features/FeatureStore.java b/src/jalview/datamodel/features/FeatureStore.java
index cb0c36e..23edade 100644
--- a/src/jalview/datamodel/features/FeatureStore.java
+++ b/src/jalview/datamodel/features/FeatureStore.java
@@ -184,20 +184,38 @@ public class FeatureStore
 
     /*
      * record the total extent of positional features, to make
-     * getAverageFeatureLength possible; we count the length of a 
+     * getTotalFeatureLength possible; we count the length of a 
      * contact feature as 1
      */
-    if (added && !feature.isNonPositional())
+    if (added)
     {
-      int featureLength = feature.isContactFeature() ? 1 : 1
-              + feature.getEnd() - feature.getBegin();
-      totalExtent += featureLength;
+      totalExtent += getFeatureLength(feature);
     }
 
     return added;
   }
 
   /**
+   * Answers the 'length' of the feature, counting 0 for non-positional features
+   * and 1 for contact features
+   * 
+   * @param feature
+   * @return
+   */
+  protected static int getFeatureLength(SequenceFeature feature)
+  {
+    if (feature.isNonPositional())
+    {
+      return 0;
+    }
+    if (feature.isContactFeature())
+    {
+      return 1;
+    }
+    return 1 + feature.getEnd() - feature.getBegin();
+  }
+
+  /**
    * Adds the feature to the list of non-positional features (with lazy
    * instantiation of the list if it is null), and returns true. If the
    * non-positional features already include the new feature (by equality test),
@@ -639,8 +657,18 @@ public class FeatureStore
 
     if (removed)
     {
+      /*
+       * rescan (positional or non-positional) features to rebuild the
+       * set of distinct feature groups present
+       */
       rebuildFeatureGroups(sf.getFeatureGroup(), removedNonPositional);
-      // TODO and recalculate totalExtent (feature may have changed length!)
+
+      /*
+       * subtract deleted feature's length from stored total length
+       * TODO: can start/end have changed since the feature was added?
+       */
+      int extent = getFeatureLength(sf);
+      totalExtent = Math.max(0, totalExtent - extent);
     }
 
     return removed;
diff --git a/test/jalview/datamodel/features/FeatureStoreTest.java b/test/jalview/datamodel/features/FeatureStoreTest.java
index 2886b47..6e30990 100644
--- a/test/jalview/datamodel/features/FeatureStoreTest.java
+++ b/test/jalview/datamodel/features/FeatureStoreTest.java
@@ -549,19 +549,52 @@ public class FeatureStoreTest
     addFeature(fs, 10, 20); // 11
     assertEquals(fs.getTotalFeatureLength(), 11);
     addFeature(fs, 17, 37); // 21
-    addFeature(fs, 14, 74); // 61
+    SequenceFeature sf1 = addFeature(fs, 14, 74); // 61
     assertEquals(fs.getTotalFeatureLength(), 93);
 
     // non-positional features don't count
-    SequenceFeature sf1 = new SequenceFeature("Cath", "desc", 0, 0, 1f,
+    SequenceFeature sf2 = new SequenceFeature("Cath", "desc", 0, 0, 1f,
             "group1");
-    fs.addFeature(sf1);
+    fs.addFeature(sf2);
     assertEquals(fs.getTotalFeatureLength(), 93);
 
     // contact features count 1
-    SequenceFeature sf2 = new SequenceFeature("disulphide bond", "desc",
+    SequenceFeature sf3 = new SequenceFeature("disulphide bond", "desc",
             15, 35, 1f, "group1");
-    fs.addFeature(sf2);
+    fs.addFeature(sf3);
     assertEquals(fs.getTotalFeatureLength(), 94);
+
+    assertTrue(fs.delete(sf1));
+    assertEquals(fs.getTotalFeatureLength(), 33);
+    assertFalse(fs.delete(sf1));
+    assertEquals(fs.getTotalFeatureLength(), 33);
+    assertTrue(fs.delete(sf2));
+    assertEquals(fs.getTotalFeatureLength(), 33);
+    assertTrue(fs.delete(sf3));
+    assertEquals(fs.getTotalFeatureLength(), 32);
+  }
+
+  @Test(groups = "Functional")
+  public void testGetFeatureLength()
+  {
+    /*
+     * positional feature
+     */
+    SequenceFeature sf1 = new SequenceFeature("Cath", "desc", 10, 20, 1f, "group1");
+    assertEquals(FeatureStore.getFeatureLength(sf1), 11);
+  
+    /*
+     * non-positional feature
+     */
+    SequenceFeature sf2 = new SequenceFeature("Cath", "desc", 0, 0, 1f,
+            "CathGroup");
+    assertEquals(FeatureStore.getFeatureLength(sf2), 0);
+
+    /*
+     * contact feature counts 1
+     */
+    SequenceFeature sf3 = new SequenceFeature("Disulphide Bond", "desc",
+            14, 28, 1f, "AGroup");
+    assertEquals(FeatureStore.getFeatureLength(sf3), 1);
   }
 }
-- 
1.7.10.2