♻️ refactor(delete): Split "no-child" and "exactly-one-child" cases.

The coverage report seems to indicate that some additional pruning is
possible in delete_one_child. Need to investigate.

Author: make-github-pseudonymous-again

src/deletion/delete_no_child.js

@@ -0,0 +1,48 @@
+import assert from 'assert';
+import BLACK from '../color/BLACK.js';
+import RED from '../color/RED.js';
+import Node from '../types/Node.js';
+import Leaf from '../types/Leaf.js';
+
+import replace_node from './replace_node.js';
+import delete_case2 from './delete_case2.js';
+
+import prune_subtree from './prune_subtree.js';
+
+/**
+ * Delete a node <code>n</code> that has no non-leaf child.
+ *
+ * Precondition:
+ *   - n has no non-leaf child.
+ *   - n is not the root
+ *
+ * @param {Node} n - The node to delete.
+ */
+const delete_no_child = (n) => {
+	assert(n instanceof Node);
+	assert(n.parent !== null);
+	assert(n.left === null);
+	assert(n.right === null);
+
+	if (n._color === RED) {
+		prune_subtree(n);
+		return;
+	}
+
+	assert(n._color === BLACK);
+
+	// Mock leaf since there is no left child
+	const leaf = new Leaf(null);
+
+	// Replace n with the mocked leaf
+	replace_node(n, leaf);
+
+	// If n is black, deleting it reduces the black-height of every path going
+	// through it by 1. Leaf is black, so there are more things to fix.
+	delete_case2(leaf);
+
+	// Delete mocked leaf
+	prune_subtree(leaf);
+};
+
+export default delete_no_child;

src/deletion/delete_one_child.js

@@ -2,68 +2,50 @@ import assert from 'assert';
 import BLACK from '../color/BLACK.js';
 import RED from '../color/RED.js';
 import Node from '../types/Node.js';
-import Leaf from '../types/Leaf.js';
 
 import replace_node from './replace_node.js';
 import delete_case2 from './delete_case2.js';
 
-import prune_subtree from './prune_subtree.js';
-
 /**
- * Delete a node <code>n</code> that has at most a single non-leaf child.
+ * Delete a node <code>n</code> with one non-leaf left child and one leaf right
+ * child.
  *
  * Precondition:
- *   - n has at most one non-leaf child.
+ *   - n has exactly one non-leaf child.
  *   - n is not the root
- *   - if n has a non-leaf child, then it is its left child.
+ *   - n's only non-leaf child is n's left child.
  *   - hence, n's right child is a leaf
  *
  * @param {Node} n - The node to delete.
  */
 const delete_one_child = (n) => {
 	assert(n instanceof Node);
 	assert(n.parent !== null);
-	// Precondition: n's right child is a leaf.
-	// The right child of n is always a LEAF because either n is a subtree
-	// predecessor or it is the only child of its parent by the red-black tree
-	// properties
+	assert(n.left instanceof Node);
 	assert(n.right === null);
 
-	if (n._color === RED) {
-		//    If n is red then its child can only be black. Replacing n with its
-		//    child suffices.
-		const child = n.left;
-		if (child === null) {
-			prune_subtree(n);
-		} else {
-			assert(child._color === BLACK);
-			replace_node(n, child);
-		}
-
-		return;
-	}
-
-	assert(n._color === BLACK);
-
-	// Mock leaf if there is no left child
-	const child = n.left === null ? new Leaf(null) : n.left;
+	const child = n.left;
 
-	// Replace n with its left child
+	// Replace n with its only child
 	replace_node(n, child);
 
-	// If n is black, deleting it reduces the black-height of every path going
-	// through it by 1.
-	// We can easily fix this when its left child is an
-	// internal red node: change the color of the left child to black and
-	// replace n with it.
-	if (child._color === RED) child._color = BLACK;
-	// Otherwise, there are more things to fix.
-	else {
-		delete_case2(child);
+	if (n._color === BLACK) {
+		// If n is black, deleting it reduces the black-height of every path going
+		// through it by 1.
+		// We can easily fix this when its only child is an
+		// internal red node: change the color of the child to black and
+		// replace n with it.
+		if (child._color === RED) child._color = BLACK;
+		// Otherwise, there are more things to fix.
+		else {
+			delete_case2(child);
+		}
+	} else {
+		assert(n._color === RED);
+		// If n is red then its child can only be black. Replacing n with its
+		// child suffices. This has already been done above.
+		assert(child._color === BLACK);
 	}
-
-	// Delete mocked leaf
-	if (child instanceof Leaf) prune_subtree(child);
 };
 
 export default delete_one_child;

src/deletion/replace_node.js

@@ -11,7 +11,7 @@ import Leaf from '../types/Leaf.js';
 const replace_node = (A, B) => {
 	assert(A instanceof Node);
 	assert(B instanceof Node || B instanceof Leaf);
-	// We never apply delete_one_child on the root
+	// We never apply delete_one_child or delete_no_child on the root
 	assert(A.parent !== null);
 
 	if (A === A.parent.left) A.parent.left = B;

src/types/RedBlackTree.js

@@ -6,6 +6,7 @@ import predecessor from '../family/predecessor.js';
 import insert from '../insertion/insert.js';
 import insert_case2 from '../insertion/insert_case2.js';
 import delete_one_child from '../deletion/delete_one_child.js';
+import delete_no_child from '../deletion/delete_no_child.js';
 import search from '../search/search.js';
 import inordertraversal from '../traversal/inordertraversal.js';
 import rangetraversal from '../traversal/rangetraversal.js';
@@ -21,6 +22,7 @@ export default class RedBlackTree {
 	 * @param {Function} compare - The comparison function for node keys.
 	 */
 	constructor(compare) {
+		assert(compare instanceof Function);
 		/** @member {Function} The comparison function for node keys. */
 		this.compare = compare;
 		/** @member {Node} The root of the tree. */
@@ -94,31 +96,31 @@ export default class RedBlackTree {
 	 * @param {Node} node - The input node to delete.
 	 */
 	_delete(node) {
+		assert(node instanceof Node);
 		if (node.left !== null) {
 			// Replace node's key with predecessor's key
 			const pred = predecessor(node);
 			node.key = pred.key;
 			// Delete predecessor node
-			// note: this node can only have one non-leaf child
-			//       because the tree is a red-black tree
-			delete_one_child(pred);
+			// NOTE: this node can only have one non-leaf (left) child because
+			// of red-black tree invariant.
+			if (pred.left === null) {
+				delete_no_child(pred);
+			} else {
+				delete_one_child(pred);
+			}
 		} else if (node.right !== null) {
 			// Replace node's key with successor's key
-			// If there is no left child, then there can only be one right
-			// child.
+			// NOTE: Since there is no left child, then there can only be one
+			// right child by the red-black tree invariant.
 			const succ = node.right;
-			assert(succ instanceof Node);
-			assert(succ.left === null);
-			assert(succ.right === null);
 			node.key = succ.key;
 			// Delete successor node
-			// note: this node can only have one non-leaf child
-			//       because the tree is a red-black tree
-			delete_one_child(succ);
+			delete_no_child(succ);
 		} else if (node === this.root) {
 			this.root = null;
 		} else {
-			delete_one_child(node);
+			delete_no_child(node);
 		}
 	}