Always assert the correctness of graph operations

Author: ibbem

src/main/java/org/variantsync/diffdetective/util/Assert.java

@@ -68,6 +68,14 @@ public static void assertFalse(boolean condition) {
         assertTrue(!condition);
     }
 
+    public static void assertFalse(boolean condition, final Supplier<String> errorMessage) {
+        assertTrue(!condition, errorMessage);
+    }
+
+    public static void assertFalse(boolean condition, String errorMessage) {
+        assertTrue(!condition, errorMessage);
+    }
+
     /** Throws {@link AssertionError} with {@code errorMessage} as error message. */
     public static void fail(String errorMessage) {
         throw new AssertionError(errorMessage);

src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java

@@ -231,30 +231,20 @@ public int getChangeAmount(Time time) {
         return getParent(time).getChangeAmount(time);
     }
 
-    /**
-     * Sets the parent at {@code time} checking that this node doesn't currently have a parent.
-     */
-    private void setParent(final DiffNode newParent, Time time) {
-        Assert.assertTrue(getParent(time) == null);
-        parents[time.ordinal()] = newParent;
-    }
-
     /**
      * Adds this subtree below the given parents.
      * Inverse of drop.
      * @param newBeforeParent Node that should be this node's before parent. May be null.
      * @param newAfterParent Node that should be this node's after parent. May be null.
      * @return True iff this node could be added as child to at least one of the given non-null parents.
      */
-    public boolean addBelow(final DiffNode newBeforeParent, final DiffNode newAfterParent) {
-        boolean success = false;
-        if (newBeforeParent != null) {
-            success |= newBeforeParent.addChild(this, BEFORE);
+    public void addBelow(final DiffNode newBeforeParent, final DiffNode newAfterParent) {
+        if (getDiffType().existsAtTime(BEFORE) && newBeforeParent != null) {
+            newBeforeParent.addChild(this, BEFORE);
         }
-        if (newAfterParent != null) {
-            success |= newAfterParent.addChild(this, AFTER);
+        if (getDiffType().existsAtTime(AFTER) && newAfterParent != null) {
+            newAfterParent.addChild(this, AFTER);
         }
-        return success;
     }
 
     /**
@@ -263,25 +253,19 @@ public boolean addBelow(final DiffNode newBeforeParent, final DiffNode newAfterP
      */
     public void drop() {
         Time.forAll(time -> {
-            drop(time);
+            if (getParent(time) != null) {
+                drop(time);
+            }
         });
     }
 
     /**
      * Removes this subtree from its parents at the time {@code time}.
      */
     public void drop(Time time) {
-        if (getParent(time) != null) {
-            getParent(time).removeChild(this, time);
-        }
-    }
+        Assert.assertTrue(getParent(time) != null);
 
-    /**
-     * Remove this node as the parent of {@code child} but it doesn't change {@link children}.
-     */
-    private void dropChild(final DiffNode child, Time time) {
-        Assert.assertTrue(child.getParent(time) == this);
-        child.parents[time.ordinal()] = null;
+        getParent(time).removeChild(this, time);
     }
 
     /**
@@ -295,34 +279,26 @@ public int indexOfChild(final DiffNode child, Time time) {
     /**
      * Insert {@code child} as child at the time {@code time} at the position {@code index}.
      */
-    public boolean insertChild(final DiffNode child, int index, Time time) {
-        if (child.getDiffType().existsAtTime(time)) {
-            if (child.getParent(time) != null) {
-                throw new IllegalArgumentException("Given child " + child + " already has a " + time + " parent (" + child.getParent(time) + ")!");
-            }
+    public void insertChild(final DiffNode child, int index, Time time) {
+        Assert.assertTrue(child.getDiffType().existsAtTime(time));
+        Assert.assertFalse(isChild(child, time), () ->
+            "Given child " + child + " already has a " + time + " parent (" + child.getParent(time) + ")!");
 
-            children[time.ordinal()].add(index, child);
-            child.setParent(this, time);
-            return true;
-        }
-        return false;
+        children[time.ordinal()].add(index, child);
+        child.parents[time.ordinal()] = this;
     }
 
     /**
      * The same as {@link DiffNode#insertChild} but puts the node at the end of the children
      * list instead of inserting it at a specific index.
      */
-    public boolean addChild(final DiffNode child, Time time) {
-        if (child.getDiffType().existsAtTime(time)) {
-            if (child.getParent(time) != null) {
-                throw new IllegalArgumentException("Given child " + child + " already has a " + time + " parent (" + child.getParent(time) + ")!");
-            }
+    public void addChild(final DiffNode child, Time time) {
+        Assert.assertTrue(child.getDiffType().existsAtTime(time));
+        Assert.assertFalse(isChild(child, time), () ->
+            "Given child " + child + " already has a " + time + " parent (" + child.getParent(time) + ")!");
 
-            children[time.ordinal()].add(child);
-            child.setParent(this, time);
-            return true;
-        }
-        return false;
+        children[time.ordinal()].add(child);
+        child.parents[time.ordinal()] = this;
     }
 
     /**
@@ -343,13 +319,11 @@ public void addChildren(final Collection<DiffNode> children, Time time) {
      * @param time whether {@code child} should be removed before or after the edit
      * @return True iff the child was removed, false iff it's not a child at {@code time}.
      */
-    public boolean removeChild(final DiffNode child, Time time) {
-        if (isChild(child, time)) {
-            dropChild(child, time);
-            children[time.ordinal()].remove(child);
-            return true;
-        }
-        return false;
+    public void removeChild(final DiffNode child, Time time) {
+        Assert.assertTrue(isChild(child, time));
+
+        child.parents[time.ordinal()] = null;
+        children[time.ordinal()].remove(child);
     }
 
     /**
@@ -359,7 +333,11 @@ public boolean removeChild(final DiffNode child, Time time) {
      */
     public void removeChildren(final Collection<DiffNode> childrenToRemove) {
         for (final DiffNode childToRemove : childrenToRemove) {
-            Time.forAll(time -> removeChild(childToRemove, time));
+            Time.forAll(time -> {
+                if (isChild(childToRemove, time)) {
+                    removeChild(childToRemove, time);
+                }
+            });
         }
     }
 
@@ -371,7 +349,7 @@ public void removeChildren(final Collection<DiffNode> childrenToRemove) {
      */
     public List<DiffNode> removeChildren(Time time) {
         for (var child : children[time.ordinal()]) {
-            dropChild(child, time);
+            child.parents[time.ordinal()] = null;
         }
 
         final List<DiffNode> orphans = children[time.ordinal()];

src/main/java/org/variantsync/diffdetective/variation/diff/DiffTree.java

@@ -607,7 +607,9 @@ private static void addUnmapped(MappingStore mappings, DiffNode parent, WrappedV
             );
         } else {
             diffNode = ((WrappedDiffTree)src).getDiffNode();
-            diffNode.drop(AFTER);
+            if (diffNode.getParent(AFTER) != null) {
+                diffNode.drop(AFTER);
+            }
         }
         parent.addChild(diffNode, AFTER);
 

src/main/java/org/variantsync/diffdetective/variation/diff/Projection.java

@@ -102,8 +102,8 @@ public void insertChild(final Projection child, int index) {
     }
 
     @Override
-    public boolean removeChild(final Projection child) {
-        return getBackingNode().removeChild(child.getBackingNode(), time);
+    public void removeChild(final Projection child) {
+        getBackingNode().removeChild(child.getBackingNode(), time);
     }
 
     @Override

src/main/java/org/variantsync/diffdetective/variation/diff/serialize/edgeformat/EdgeLabelFormat.java

@@ -1,6 +1,5 @@
 package org.variantsync.diffdetective.variation.diff.serialize.edgeformat;
 
-import org.variantsync.diffdetective.util.Assert;
 import org.variantsync.diffdetective.variation.diff.DiffNode;
 import org.variantsync.diffdetective.variation.diff.Time;
 import org.variantsync.diffdetective.variation.diff.serialize.LineGraphConstants;
@@ -99,13 +98,13 @@ public Direction getEdgeDirection() {
     protected void connectAccordingToLabel(final DiffNode child, final DiffNode parent, final String edgeLabel) {
         if (edgeLabel.startsWith(LineGraphConstants.BEFORE_AND_AFTER_PARENT)) {
             // Nothing has been changed. The child-parent relationship remains the same
-            Time.forAll(time -> Assert.assertTrue(parent.addChild(child, time)));
+            Time.forAll(time -> parent.addChild(child, time));
         } else if (edgeLabel.startsWith(LineGraphConstants.BEFORE_PARENT)) {
             // The child DiffNode lost its parent DiffNode (an orphan DiffNode)
-            Assert.assertTrue(parent.addChild(child, BEFORE));
+            parent.addChild(child, BEFORE);
         } else if (edgeLabel.startsWith(LineGraphConstants.AFTER_PARENT)) {
             // The parent DiffNode has a new child DiffNode
-            Assert.assertTrue(parent.addChild(child, AFTER));
+            parent.addChild(child, AFTER);
         } else {
             throw new IllegalArgumentException("Syntax error. Invalid name in edge label " + edgeLabel);
         }

src/main/java/org/variantsync/diffdetective/variation/tree/VariationNode.java

@@ -228,7 +228,7 @@ public void addChildren(final Collection<T> children) {
      * @see removeChildren
      * @see getChildren
      */
-    public abstract boolean removeChild(final T child);
+    public abstract void removeChild(final T child);
 
     /**
      * Removes the given nodes from the children list using {@link removeChild}.

src/main/java/org/variantsync/diffdetective/variation/tree/VariationTreeNode.java

@@ -228,13 +228,10 @@ public void insertChild(final VariationTreeNode child, int index) {
     }
 
     @Override
-    public boolean removeChild(final VariationTreeNode child) {
-        if (isChild(child)) {
-            child.parent = null;
-            childOrder.remove(child);
-            return true;
-        }
-        return false;
+    public void removeChild(final VariationTreeNode child) {
+        Assert.assertTrue(isChild(child));
+        child.parent = null;
+        childOrder.remove(child);
     }
 
     @Override