Fix (some of?) the crashes that occur if you delete the leftmost, rightmost, or rootmost nodes. I'm the guy who tests his code. You must be the other guy. Addresses #1405

Author: AngledLuffa

src/edu/stanford/nlp/util/IntervalTree.java

@@ -18,6 +18,7 @@ public class IntervalTree<E extends Comparable<E>, T extends HasInterval<E>> ext
   private static final boolean debug = false;
 
   private TreeNode<E,T> root = new TreeNode<>();
+  TreeNode<E, T> root() { return root; }
 
   // Tree node
   public static class TreeNode<E extends Comparable<E>, T extends HasInterval<E>> {
@@ -40,6 +41,10 @@ public void clear() {
       right = null;
 //      parent = null;
     }
+
+    public String toString() {
+      return "TreeNode(value=" + value + ", size=" + size + ", maxEnd=" + maxEnd + ")";
+    }    
   }
 
   @Override
@@ -237,8 +242,9 @@ public boolean remove(TreeNode<E,T> node, T target)
         node.value = node.left.value;
         node.size = node.left.size;
         node.maxEnd = node.left.maxEnd;
-        node.left = node.left.left;
+        // Don't overwrite the left child before getting all of its values!
         node.right = node.left.right;
+        node.left = node.left.left;
         if (node.left != null) node.left.parent = node;
         if (node.right != null) node.right.parent = node;
       } else {
@@ -270,7 +276,18 @@ public boolean remove(TreeNode<E,T> node, T target)
         }
         boolean res = remove(node.left, target);
         if (res) {
-          node.maxEnd = Interval.max(node.maxEnd, node.left.maxEnd);
+          node.maxEnd = node.value.getInterval().getEnd();
+          if (node.right != null) {
+            node.maxEnd = Interval.max(node.maxEnd, node.right.maxEnd);
+          }
+          if (node.left != null) {
+            if (node.left.size == 0) {
+              // got called clear() in a pathway which could have been left or right
+              node.left = null;
+            } else {
+              node.maxEnd = Interval.max(node.maxEnd, node.left.maxEnd);
+            }
+          }
           node.size--;
         }
         return res;
@@ -281,7 +298,18 @@ public boolean remove(TreeNode<E,T> node, T target)
         }
         boolean res = remove(node.right, target);
         if (res) {
-          node.maxEnd = Interval.max(node.maxEnd, node.right.maxEnd);
+          node.maxEnd = node.value.getInterval().getEnd();
+          if (node.left != null) {
+            node.maxEnd = Interval.max(node.maxEnd, node.left.maxEnd);
+          }
+          if (node.right != null) {
+            if (node.right.size == 0) {
+              // got called clear() in a pathway which could have been left or right
+              node.right = null;
+            } else {
+              node.maxEnd = Interval.max(node.maxEnd, node.right.maxEnd);
+            }
+          }
           node.size--;
         }
         return res;

test/src/edu/stanford/nlp/util/IntervalTreeTest.java

@@ -160,4 +160,76 @@ public void testIteratorOrdered() throws Exception
     }
     assertFalse("No more items", iterator.hasNext());
   }
+
+  public void testSizeOne() {
+    IntervalTree<Integer, Interval<Integer>> tree = new IntervalTree<>();
+    tree.add(Interval.toInterval(0, 1));
+    assertEquals(tree.size(), 1);
+    tree.remove(Interval.toInterval(0, 1));
+    assertEquals(tree.size(), 0);
+  }
+
+  public void testSizeTwoDeleteLeft() {
+    IntervalTree<Integer, Interval<Integer>> tree = new IntervalTree<>();
+    tree.add(Interval.toInterval(0, 1));
+    assertEquals(tree.size(), 1);
+
+    tree.add(Interval.toInterval(2, 5));
+    assertEquals(tree.size(), 2);
+    assertEquals(tree.root().maxEnd.intValue(), 5);
+
+    tree.remove(Interval.toInterval(0, 1));
+    assertEquals(tree.size(), 1);
+    assertEquals(tree.root().maxEnd.intValue(), 5);
+
+    // new tree, insert in opposite order
+    tree = new IntervalTree<>();
+    tree.add(Interval.toInterval(2, 5));
+    assertEquals(tree.size(), 1);
+    assertEquals(tree.root().maxEnd.intValue(), 5);
+
+    tree.add(Interval.toInterval(0, 1));
+    assertEquals(tree.size(), 2);
+    assertEquals(tree.root().maxEnd.intValue(), 5);
+
+    tree.remove(Interval.toInterval(0, 1));
+    assertEquals(tree.size(), 1);
+    assertEquals(tree.root().maxEnd.intValue(), 5);
+  }
+
+  public void testSizeTwoDeleteRight() {
+    IntervalTree<Integer, Interval<Integer>> tree = new IntervalTree<>();
+    tree.add(Interval.toInterval(0, 1));
+    assertEquals(tree.size(), 1);
+    assertEquals(tree.root().maxEnd.intValue(), 1);
+
+    tree.add(Interval.toInterval(2, 5));
+    assertEquals(tree.size(), 2);
+    assertEquals(tree.root().maxEnd.intValue(), 5);
+
+    tree.remove(Interval.toInterval(2, 5));
+    assertEquals(tree.size(), 1);
+    assertEquals(tree.root().maxEnd.intValue(), 1);
+
+    tree.remove(Interval.toInterval(0, 1));
+    assertEquals(tree.size(), 0);
+
+    // new tree, insert in opposite order
+    tree = new IntervalTree<>();
+    tree.add(Interval.toInterval(2, 5));
+    assertEquals(tree.size(), 1);
+    assertEquals(tree.root().maxEnd.intValue(), 5);
+
+    tree.add(Interval.toInterval(0, 1));
+    assertEquals(tree.size(), 2);
+    assertEquals(tree.root().maxEnd.intValue(), 5);
+
+    tree.remove(Interval.toInterval(2, 5));
+    assertEquals(tree.size(), 1);
+    assertEquals(tree.root().maxEnd.intValue(), 1);
+
+    tree.remove(Interval.toInterval(0, 1));
+    assertEquals(tree.size(), 0);
+
+  }
 }