From 452f34ba2e3d65c29390df2b9d3637c4444ecbe2 Mon Sep 17 00:00:00 2001
From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Fri, 7 Apr 2017 11:35:27 +0100
Subject: [PATCH] JAL-2446 fix size maintenance during add

---
 src/jalview/datamodel/features/NCList.java      |   24 ++++++++++++++++------
 src/jalview/datamodel/features/NCNode.java      |   25 ++++++++++++++++++++++-
 test/jalview/datamodel/features/NCListTest.java |   13 ++++++------
 3 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/src/jalview/datamodel/features/NCList.java b/src/jalview/datamodel/features/NCList.java
index 9906884..afa31e8 100644
--- a/src/jalview/datamodel/features/NCList.java
+++ b/src/jalview/datamodel/features/NCList.java
@@ -50,6 +50,9 @@ public class NCList<T extends ContiguousI>
   }
 
   /**
+   * Sort and group ranges into sublists where each sublist represents a region
+   * and its contained subregions
+   * 
    * @param ranges
    */
   protected void build(List<T> ranges)
@@ -78,9 +81,8 @@ public class NCList<T extends ContiguousI>
   public NCList(T entry)
   {
     this();
-    List<T> ranges = new ArrayList<T>();
-    ranges.add(entry);
-    build(ranges);
+    subranges.add(new NCNode<T>(entry));
+    size = 1;
   }
 
   public NCList()
@@ -326,7 +328,7 @@ public class NCList<T extends ContiguousI>
           final int j)
   {
     NCList<T> newNCList = new NCList<T>();
-    newNCList.subranges.addAll(subranges.subList(i, j + 1));
+    newNCList.addNodes(subranges.subList(i, j + 1));
     NCNode<T> newNode = new NCNode<T>(entry, newNCList);
     for (int k = j; k >= i; k--)
     {
@@ -335,6 +337,15 @@ public class NCList<T extends ContiguousI>
     subranges.add(i, newNode);
   }
 
+  protected void addNodes(List<NCNode<T>> nodes)
+  {
+    for (NCNode<T> node : nodes)
+    {
+      subranges.add(node);
+      size += node.size();
+    }
+  }
+
   /**
    * Returns a (possibly empty) list of items whose extent overlaps the given
    * range
@@ -531,7 +542,7 @@ public class NCList<T extends ContiguousI>
    * 
    * @return
    */
-  public int getSize()
+  public int size()
   {
     return size;
   }
@@ -594,7 +605,7 @@ public class NCList<T extends ContiguousI>
         subranges.remove(i);
         if (subRegions != null)
         {
-          subranges.addAll(i, subRegions.subranges);
+          subranges.addAll(subRegions.subranges);
           Collections.sort(subranges, intervalSorter);
         }
         size--;
@@ -605,6 +616,7 @@ public class NCList<T extends ContiguousI>
         if (subRegions != null && subRegions.delete(entry))
         {
           size--;
+          subrange.deleteSubRegionsIfEmpty();
           return true;
         }
       }
diff --git a/src/jalview/datamodel/features/NCNode.java b/src/jalview/datamodel/features/NCNode.java
index 5fb2d0d..38c091e 100644
--- a/src/jalview/datamodel/features/NCNode.java
+++ b/src/jalview/datamodel/features/NCNode.java
@@ -49,7 +49,7 @@ class NCNode<V extends ContiguousI> implements ContiguousI
   {
     region = entry;
     subregions = newNCList;
-    size = 1 + newNCList.getSize();
+    size = 1 + newNCList.size();
   }
 
   /**
@@ -150,6 +150,7 @@ class NCNode<V extends ContiguousI> implements ContiguousI
     {
       subregions.add(entry);
     }
+    size++;
   }
 
   /**
@@ -227,4 +228,26 @@ class NCNode<V extends ContiguousI> implements ContiguousI
   {
     return subregions;
   }
+
+  /**
+   * Nulls the subregion reference if it is empty (after a delete entry
+   * operation)
+   */
+  void deleteSubRegionsIfEmpty()
+  {
+    if (subregions != null && subregions.size() == 0)
+    {
+      subregions = null;
+    }
+  }
+
+  /**
+   * Answers the (deep) size of this node i.e. the number of ranges it models
+   * 
+   * @return
+   */
+  int size()
+  {
+    return size;
+  }
 }
diff --git a/test/jalview/datamodel/features/NCListTest.java b/test/jalview/datamodel/features/NCListTest.java
index e874ce4..65c8c91 100644
--- a/test/jalview/datamodel/features/NCListTest.java
+++ b/test/jalview/datamodel/features/NCListTest.java
@@ -202,14 +202,14 @@ public class NCListTest
    * @param ncl
    * @param features
    */
-  void testDelete_pseudoRandom(NCList<SequenceFeature> ncl,
+  protected void testDelete_pseudoRandom(NCList<SequenceFeature> ncl,
           List<SequenceFeature> features)
   {
     int deleted = 0;
 
     while (!features.isEmpty())
     {
-      assertEquals(ncl.getSize(), features.size());
+      assertEquals(ncl.size(), features.size());
       int toDelete = random.nextInt(features.size());
       SequenceFeature entry = features.get(toDelete);
       String lastDeleted = "'" + entry.toString() + "'";
@@ -238,7 +238,7 @@ public class NCListTest
                 i, sf.toString(), lastDeleted));
       }
     }
-    assertEquals(ncl.getSize(), 0); // all gone
+    assertEquals(ncl.size(), 0); // all gone
   }
 
   /**
@@ -250,7 +250,8 @@ public class NCListTest
    * @param ncl
    * @param features
    */
-  void testAdd_pseudoRandom(Integer scale, NCList<SequenceFeature> ncl,
+  protected void testAdd_pseudoRandom(Integer scale,
+          NCList<SequenceFeature> ncl,
           List<SequenceFeature> features)
   {
     int count = 0;
@@ -291,7 +292,7 @@ public class NCListTest
        */
       assertTrue(ncl.isValid(),
               String.format("Failed for scale = %d, i=%d", scale, i));
-      assertEquals(ncl.getSize(), count);
+      assertEquals(ncl.size(), count);
     }
     // System.out.println(ncl.prettyPrint());
   }
@@ -488,7 +489,7 @@ public class NCListTest
     assertSame(features.getEntries().get(1), sf1);
     assertTrue(features.delete(sf1)); // first match only is deleted
     assertTrue(features.contains(sf1));
-    assertEquals(features.getSize(), 1);
+    assertEquals(features.size(), 1);
     assertTrue(features.delete(sf1));
     assertTrue(features.getEntries().isEmpty());
   }
-- 
1.7.10.2