From b5cb773990fd80387d786f5389d40c8023c5c3c3 Mon Sep 17 00:00:00 2001
From: William Fiset <fisetwill@gmail.com>
Date: Fri, 6 Mar 2026 18:03:48 -0800
Subject: [PATCH 1/7] Refactor AVLTreeRecursive: fix bugs, improve robustness,
 expand tests (#1256)

- Fix NPE in iterator when tree is empty (ArrayDeque rejects null)
- Replace deprecated java.util.Stack with ArrayDeque in iterator
- Add null guard in public contains() method
- Fix typo: validateBSTInvarient -> validateBSTInvariant
- Expand test suite from 10 to 31 tests covering edge cases,
  removal scenarios, iterator behavior, and invariant validation

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
---
 .../balancedtree/AVLTreeRecursive.java        |  11 +-
 .../balancedtree/AVLTreeTest.java             | 202 +++++++++++++++++-
 2 files changed, 207 insertions(+), 6 deletions(-)

diff --git a/src/main/java/com/williamfiset/algorithms/datastructures/balancedtree/AVLTreeRecursive.java b/src/main/java/com/williamfiset/algorithms/datastructures/balancedtree/AVLTreeRecursive.java
index 03b034538..e416ab7ea 100644
--- a/src/main/java/com/williamfiset/algorithms/datastructures/balancedtree/AVLTreeRecursive.java
+++ b/src/main/java/com/williamfiset/algorithms/datastructures/balancedtree/AVLTreeRecursive.java
@@ -71,6 +71,7 @@ public boolean isEmpty() {
 
   // Return true/false depending on whether a value exists in the tree.
   public boolean contains(T value) {
+    if (value == null) return false;
     return contains(root, value);
   }
 
@@ -299,12 +300,12 @@ private T findMax(Node node) {
     return node.value;
   }
 
-  // Returns as iterator to traverse the tree in order.
+  // Returns an iterator to traverse the tree in order.
   public java.util.Iterator<T> iterator() {
 
     final int expectedNodeCount = nodeCount;
-    final java.util.Stack<Node> stack = new java.util.Stack<>();
-    stack.push(root);
+    final java.util.Deque<Node> stack = new java.util.ArrayDeque<>();
+    if (root != null) stack.push(root);
 
     return new java.util.Iterator<T>() {
       Node trav = root;
@@ -350,12 +351,12 @@ public String toString() {
   // Make sure all left child nodes are smaller in value than their parent and
   // make sure all right child nodes are greater in value than their parent.
   // (Used only for testing)
-  public boolean validateBSTInvarient(Node node) {
+  public boolean validateBSTInvariant(Node node) {
     if (node == null) return true;
     T val = node.value;
     boolean isValid = true;
     if (node.left != null) isValid = isValid && node.left.value.compareTo(val) < 0;
     if (node.right != null) isValid = isValid && node.right.value.compareTo(val) > 0;
-    return isValid && validateBSTInvarient(node.left) && validateBSTInvarient(node.right);
+    return isValid && validateBSTInvariant(node.left) && validateBSTInvariant(node.right);
   }
 }
diff --git a/src/test/java/com/williamfiset/algorithms/datastructures/balancedtree/AVLTreeTest.java b/src/test/java/com/williamfiset/algorithms/datastructures/balancedtree/AVLTreeTest.java
index 9461291e3..474092193 100644
--- a/src/test/java/com/williamfiset/algorithms/datastructures/balancedtree/AVLTreeTest.java
+++ b/src/test/java/com/williamfiset/algorithms/datastructures/balancedtree/AVLTreeTest.java
@@ -1,9 +1,12 @@
 package com.williamfiset.algorithms.datastructures.balancedtree;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.ConcurrentModificationException;
+import java.util.Iterator;
 import java.util.List;
 import java.util.TreeSet;
 import org.junit.jupiter.api.*;
@@ -37,6 +40,43 @@ public void testTreeContainsNull() {
     assertThat(tree.contains(null)).isFalse();
   }
 
+  @Test
+  public void testEmptyTree() {
+    assertThat(tree.isEmpty()).isTrue();
+    assertThat(tree.size()).isEqualTo(0);
+    assertThat(tree.height()).isEqualTo(0);
+    assertThat(tree.contains(1)).isFalse();
+  }
+
+  @Test
+  public void testSingleElement() {
+    tree.insert(5);
+    assertThat(tree.isEmpty()).isFalse();
+    assertThat(tree.size()).isEqualTo(1);
+    assertThat(tree.height()).isEqualTo(0);
+    assertThat(tree.contains(5)).isTrue();
+    assertThat(tree.contains(4)).isFalse();
+  }
+
+  @Test
+  public void testDuplicateInsertion() {
+    assertThat(tree.insert(5)).isTrue();
+    assertThat(tree.insert(5)).isFalse();
+    assertThat(tree.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testRemoveNonExistentElement() {
+    tree.insert(5);
+    assertThat(tree.remove(99)).isFalse();
+    assertThat(tree.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testRemoveFromEmptyTree() {
+    assertThat(tree.remove(1)).isFalse();
+  }
+
   @Test
   public void testLeftLeftCase() {
 
@@ -105,6 +145,65 @@ public void testRightLeftCase() {
     assertThat(tree.root.right.right).isNull();
   }
 
+  @Test
+  public void testRemoveLeafNode() {
+    tree.insert(2);
+    tree.insert(1);
+    tree.insert(3);
+
+    assertThat(tree.remove(1)).isTrue();
+    assertThat(tree.size()).isEqualTo(2);
+    assertThat(tree.contains(1)).isFalse();
+    assertThat(tree.validateBSTInvariant(tree.root)).isTrue();
+  }
+
+  @Test
+  public void testRemoveNodeWithOneChild() {
+    tree.insert(2);
+    tree.insert(1);
+    tree.insert(3);
+    tree.insert(4);
+
+    assertThat(tree.remove(3)).isTrue();
+    assertThat(tree.size()).isEqualTo(3);
+    assertThat(tree.contains(3)).isFalse();
+    assertThat(tree.contains(4)).isTrue();
+    assertThat(tree.validateBSTInvariant(tree.root)).isTrue();
+  }
+
+  @Test
+  public void testRemoveNodeWithTwoChildren() {
+    tree.insert(3);
+    tree.insert(1);
+    tree.insert(5);
+    tree.insert(2);
+    tree.insert(4);
+
+    assertThat(tree.remove(3)).isTrue();
+    assertThat(tree.size()).isEqualTo(4);
+    assertThat(tree.contains(3)).isFalse();
+    assertThat(tree.validateBSTInvariant(tree.root)).isTrue();
+    assertThat(validateBalanceFactorValues(tree.root)).isTrue();
+  }
+
+  @Test
+  public void testRemoveRoot() {
+    tree.insert(5);
+    assertThat(tree.remove(5)).isTrue();
+    assertThat(tree.isEmpty()).isTrue();
+    assertThat(tree.root).isNull();
+  }
+
+  @Test
+  public void testInsertAndRemoveAll() {
+    for (int i = 0; i < 10; i++) tree.insert(i);
+    for (int i = 0; i < 10; i++) {
+      assertThat(tree.remove(i)).isTrue();
+      assertThat(tree.contains(i)).isFalse();
+    }
+    assertThat(tree.isEmpty()).isTrue();
+  }
+
   @Test
   public void testRandomizedBalanceFactorTest() {
     for (int i = 0; i < TEST_SZ; i++) {
@@ -113,6 +212,17 @@ public void testRandomizedBalanceFactorTest() {
     }
   }
 
+  @Test
+  public void testBalanceFactorAfterRemovals() {
+    List<Integer> values = genRandList(500);
+    for (int v : values) tree.insert(v);
+    Collections.shuffle(values);
+    for (int v : values) {
+      tree.remove(v);
+      assertThat(validateBalanceFactorValues(tree.root)).isTrue();
+    }
+  }
+
   // Make sure all balance factor values are either -1, 0 or +1
   static boolean validateBalanceFactorValues(AVLTreeRecursive<Integer>.Node node) {
     if (node == null) return true;
@@ -129,7 +239,18 @@ public void testRandomizedValueInsertionsAgainstTreeSet() {
 
       assertThat(tree.insert(v)).isEqualTo(set.add(v));
       assertThat(tree.size()).isEqualTo(set.size());
-      assertThat(tree.validateBSTInvarient(tree.root)).isTrue();
+      assertThat(tree.validateBSTInvariant(tree.root)).isTrue();
+    }
+  }
+
+  @Test
+  public void testBSTInvariantAfterRemovals() {
+    List<Integer> values = genRandList(500);
+    for (int v : values) tree.insert(v);
+    Collections.shuffle(values);
+    for (int v : values) {
+      tree.remove(v);
+      assertThat(tree.validateBSTInvariant(tree.root)).isTrue();
     }
   }
 
@@ -178,6 +299,85 @@ public void randomRemoveTests() {
     }
   }
 
+  @Test
+  public void testIteratorInOrder() {
+    int[] values = {5, 3, 7, 1, 4, 6, 8};
+    for (int v : values) tree.insert(v);
+
+    List<Integer> result = new ArrayList<>();
+    for (int v : tree) result.add(v);
+
+    assertThat(result).containsExactly(1, 3, 4, 5, 6, 7, 8).inOrder();
+  }
+
+  @Test
+  public void testIteratorOnEmptyTree() {
+    Iterator<Integer> it = tree.iterator();
+    assertThat(it.hasNext()).isFalse();
+  }
+
+  @Test
+  public void testIteratorOnSingleElement() {
+    tree.insert(42);
+    Iterator<Integer> it = tree.iterator();
+    assertThat(it.hasNext()).isTrue();
+    assertThat(it.next()).isEqualTo(42);
+    assertThat(it.hasNext()).isFalse();
+  }
+
+  @Test
+  public void testIteratorConcurrentModificationOnInsert() {
+    tree.insert(1);
+    tree.insert(2);
+    tree.insert(3);
+
+    Iterator<Integer> it = tree.iterator();
+    tree.insert(4);
+
+    assertThrows(ConcurrentModificationException.class, it::hasNext);
+  }
+
+  @Test
+  public void testIteratorConcurrentModificationOnRemove() {
+    tree.insert(1);
+    tree.insert(2);
+    tree.insert(3);
+
+    Iterator<Integer> it = tree.iterator();
+    tree.remove(2);
+
+    assertThrows(ConcurrentModificationException.class, it::next);
+  }
+
+  @Test
+  public void testIteratorRemoveUnsupported() {
+    tree.insert(1);
+    Iterator<Integer> it = tree.iterator();
+    assertThrows(UnsupportedOperationException.class, it::remove);
+  }
+
+  @Test
+  public void testToString() {
+    tree.insert(2);
+    tree.insert(1);
+    tree.insert(3);
+    // Just verify it doesn't throw and returns non-empty output.
+    assertThat(tree.toString()).isNotEmpty();
+  }
+
+  @Test
+  public void testContainsAfterRemoval() {
+    tree.insert(1);
+    tree.insert(2);
+    tree.insert(3);
+
+    assertThat(tree.contains(2)).isTrue();
+    tree.remove(2);
+    assertThat(tree.contains(2)).isFalse();
+    assertThat(tree.contains(1)).isTrue();
+    assertThat(tree.contains(3)).isTrue();
+  }
+
   static List<Integer> genRandList(int sz) {
     List<Integer> lst = new ArrayList<>(sz);
     for (int i = 0; i < sz; i++) lst.add(i); // unique values.

From 72daaf242f89f69f31af6df078146e8c4fc073de Mon Sep 17 00:00:00 2001
From: William Fiset <fisetwill@gmail.com>
Date: Fri, 6 Mar 2026 18:14:32 -0800
Subject: [PATCH 2/7] =?UTF-8?q?Refactor=20RedBlackTree:=20encapsulate=20No?=
 =?UTF-8?q?de=20fields,=20standardize=20naming,=20a=E2=80=A6=20(#1255)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Refactor RedBlackTree: encapsulate Node fields, standardize naming, and expand tests (#1255)

- Encapsulate Node fields (color, value, left, right, parent) with private access and provide getters/setters
- Standardize variable naming conventions (camelCase)
- Unified search logic in contains()
- Remove unused methods: updateParentChildLink, findMin, findMax, swapColors
- Expand RedBlackTreeTest to include comprehensive invariant verification:
  - Root and NIL node color checks
  - Red-node property (red nodes must have black children)
  - Black-height property (consistent black nodes on all paths to NIL)
- Integrated invariant verification into randomized stress tests

* Simplify RedBlackTree implementation: consolidate symmetric cases and rotations

* Use more descriptive variable names in RedBlackTree for better readability

* Further simplify RedBlackTree: use public Node fields to match project style
---
 .../balancedtree/RedBlackTree.java            | 495 +++++-------------
 .../balancedtree/RedBlackTreeTest.java        |  60 ++-
 2 files changed, 180 insertions(+), 375 deletions(-)

diff --git a/src/main/java/com/williamfiset/algorithms/datastructures/balancedtree/RedBlackTree.java b/src/main/java/com/williamfiset/algorithms/datastructures/balancedtree/RedBlackTree.java
index 4059d435b..a249b3d28 100644
--- a/src/main/java/com/williamfiset/algorithms/datastructures/balancedtree/RedBlackTree.java
+++ b/src/main/java/com/williamfiset/algorithms/datastructures/balancedtree/RedBlackTree.java
@@ -9,7 +9,7 @@
  */
 package com.williamfiset.algorithms.datastructures.balancedtree;
 
-import java.awt.*;
+import java.util.*;
 
 public class RedBlackTree<T extends Comparable<T>> implements Iterable<T> {
 
@@ -17,443 +17,194 @@ public class RedBlackTree<T extends Comparable<T>> implements Iterable<T> {
   public static final boolean BLACK = false;
 
   public class Node {
-
-    // The color of this node. By default all nodes start red.
     public boolean color = RED;
-
-    // The value/data contained within the node.
     public T value;
-
-    // The left, right and parent references of this node.
     public Node left, right, parent;
 
-    public Node(T value, Node parent) {
+    public Node(T value, boolean color, Node parent, Node left, Node right) {
       this.value = value;
-      this.parent = parent;
-    }
-
-    public Node(boolean color, T value) {
       this.color = color;
-      this.value = value;
-    }
-
-    Node(T key, boolean color, Node parent, Node left, Node right) {
-      this.value = key;
-      this.color = color;
-
-      if (parent == null && left == null && right == null) {
-        parent = this;
-        left = this;
-        right = this;
-      }
-
       this.parent = parent;
       this.left = left;
       this.right = right;
     }
-
-    public boolean getColor() {
-      return color;
-    }
-
-    public void setColor(boolean color) {
-      this.color = color;
-    }
-
-    public T getValue() {
-      return value;
-    }
-
-    public void setValue(T value) {
-      this.value = value;
-    }
-
-    public Node getLeft() {
-      return left;
-    }
-
-    public void setLeft(Node left) {
-      this.left = left;
-    }
-
-    public Node getRight() {
-      return right;
-    }
-
-    public void setRight(Node right) {
-      this.right = right;
-    }
-
-    public Node getParent() {
-      return parent;
-    }
-
-    public void setParent(Node parent) {
-      this.parent = parent;
-    }
   }
 
-  // The root node of the RB tree.
   public Node root;
-
-  // Tracks the number of nodes inside the tree.
   private int nodeCount = 0;
-
   public final Node NIL;
 
   public RedBlackTree() {
-    NIL = new Node(BLACK, null);
-    NIL.left = NIL;
-    NIL.right = NIL;
-    NIL.parent = NIL;
-
+    NIL = new Node(null, BLACK, null, null, null);
+    NIL.left = NIL.right = NIL.parent = NIL;
     root = NIL;
   }
 
-  // Returns the number of nodes in the tree.
-  public int size() {
-    return nodeCount;
-  }
-
-  // Returns whether or not the tree is empty.
-  public boolean isEmpty() {
-    return size() == 0;
-  }
-
-  public boolean contains(T value) {
-
-    Node node = root;
-
-    if (node == null || value == null) return false;
-
-    while (node != NIL) {
-
-      // Compare current value to the value in the node.
-      int cmp = value.compareTo(node.value);
-
-      // Dig into left subtree.
-      if (cmp < 0) node = node.left;
-
-      // Dig into right subtree.
-      else if (cmp > 0) node = node.right;
-
-      // Found value in tree.
-      else return true;
-    }
-
-    return false;
-  }
+  public int size() { return nodeCount; }
+  public boolean isEmpty() { return nodeCount == 0; }
+  public boolean contains(T value) { return search(value) != NIL; }
 
   public boolean insert(T val) {
-    if (val == null) {
-      throw new IllegalArgumentException("Red-Black tree does not allow null values.");
-    }
-
-    Node x = root, y = NIL;
-
-    while (x != NIL) {
-      y = x;
-
-      if (x.getValue().compareTo(val) > 0) {
-        x = x.left;
-      } else if (x.getValue().compareTo(val) < 0) {
-        x = x.right;
-      } else {
-        return false;
-      }
-    }
-
-    Node z = new Node(val, RED, y, NIL, NIL);
-
-    if (y == NIL) {
-      root = z;
-    } else if (z.getValue().compareTo(y.getValue()) < 0) {
-      y.left = z;
-    } else {
-      y.right = z;
-    }
-    insertFix(z);
-
+    if (val == null) throw new IllegalArgumentException("Null values not allowed.");
+    Node parent = NIL, current = root;
+    while (current != NIL) {
+      parent = current;
+      int cmp = val.compareTo(current.value);
+      if (cmp < 0) current = current.left;
+      else if (cmp > 0) current = current.right;
+      else return false;
+    }
+    Node newNode = new Node(val, RED, parent, NIL, NIL);
+    if (parent == NIL) root = newNode;
+    else if (val.compareTo(parent.value) < 0) parent.left = newNode;
+    else parent.right = newNode;
+    insertFix(newNode);
     nodeCount++;
     return true;
   }
 
-  private void insertFix(Node z) {
-    Node y;
-    while (z.parent.color == RED) {
-      if (z.parent == z.parent.parent.left) {
-        y = z.parent.parent.right;
-        if (y.color == RED) {
-          z.parent.color = BLACK;
-          y.color = BLACK;
-          z.parent.parent.color = RED;
-          z = z.parent.parent;
-        } else {
-          if (z == z.parent.right) {
-            z = z.parent;
-            leftRotate(z);
-          }
-          z.parent.color = BLACK;
-          z.parent.parent.color = RED;
-          rightRotate(z.parent.parent);
-        }
+  private void insertFix(Node node) {
+    while (node.parent.color == RED) {
+      boolean isLeft = (node.parent == node.parent.parent.left);
+      Node uncle = isLeft ? node.parent.parent.right : node.parent.parent.left;
+      if (uncle.color == RED) {
+        node.parent.color = uncle.color = BLACK;
+        node.parent.parent.color = RED;
+        node = node.parent.parent;
       } else {
-        y = z.parent.parent.left;
-        if (y.color == RED) {
-          z.parent.color = BLACK;
-          y.color = BLACK;
-          z.parent.parent.color = RED;
-          z = z.parent.parent;
-        } else {
-          if (z == z.parent.left) {
-            z = z.parent;
-            rightRotate(z);
-          }
-          z.parent.color = BLACK;
-          z.parent.parent.color = RED;
-          leftRotate(z.parent.parent);
+        if (node == (isLeft ? node.parent.right : node.parent.left)) {
+          node = node.parent;
+          rotate(node, isLeft);
         }
+        node.parent.color = BLACK;
+        node.parent.parent.color = RED;
+        rotate(node.parent.parent, !isLeft);
       }
     }
-    root.setColor(BLACK);
-    NIL.setParent(null);
-  }
-
-  private void leftRotate(Node x) {
-    Node y = x.right;
-    x.setRight(y.getLeft());
-    if (y.getLeft() != NIL) y.getLeft().setParent(x);
-    y.setParent(x.getParent());
-    if (x.getParent() == NIL) root = y;
-    if (x == x.getParent().getLeft()) x.getParent().setLeft(y);
-    else x.getParent().setRight(y);
-    y.setLeft(x);
-    x.setParent(y);
-  }
-
-  private void rightRotate(Node y) {
-    Node x = y.left;
-    y.left = x.right;
-    if (x.right != NIL) x.right.parent = y;
-    x.parent = y.parent;
-    if (y.parent == NIL) root = x;
-    if (y == y.parent.left) y.parent.left = x;
-    else y.parent.right = x;
-    x.right = y;
-    y.parent = x;
+    root.color = BLACK;
   }
 
   public boolean delete(T key) {
-    Node z;
-    if (key == null || (z = (search(key, root))) == NIL) return false;
-    Node x;
-    Node y = z; // temporary reference y
-    boolean y_original_color = y.getColor();
-
-    if (z.getLeft() == NIL) {
-      x = z.getRight();
-      transplant(z, z.getRight());
-    } else if (z.getRight() == NIL) {
-      x = z.getLeft();
-      transplant(z, z.getLeft());
-    } else {
-      y = successor(z.getRight());
-      y_original_color = y.getColor();
-      x = y.getRight();
-      if (y.getParent() == z) x.setParent(y);
+    Node nodeToDelete = search(key);
+    if (nodeToDelete == NIL) return false;
+    Node successor = nodeToDelete, replacement;
+    boolean successorOriginalColor = successor.color;
+    if (nodeToDelete.left == NIL) transplant(nodeToDelete, replacement = nodeToDelete.right);
+    else if (nodeToDelete.right == NIL) transplant(nodeToDelete, replacement = nodeToDelete.left);
+    else {
+      successor = findMin(nodeToDelete.right);
+      successorOriginalColor = successor.color;
+      replacement = successor.right;
+      if (successor.parent == nodeToDelete) replacement.parent = successor;
       else {
-        transplant(y, y.getRight());
-        y.setRight(z.getRight());
-        y.getRight().setParent(y);
+        transplant(successor, successor.right);
+        successor.right = nodeToDelete.right;
+        successor.right.parent = successor;
       }
-      transplant(z, y);
-      y.setLeft(z.getLeft());
-      y.getLeft().setParent(y);
-      y.setColor(z.getColor());
+      transplant(nodeToDelete, successor);
+      successor.left = nodeToDelete.left;
+      successor.left.parent = successor;
+      successor.color = nodeToDelete.color;
     }
-    if (y_original_color == BLACK) deleteFix(x);
+    if (successorOriginalColor == BLACK) deleteFix(replacement);
     nodeCount--;
     return true;
   }
 
-  private void deleteFix(Node x) {
-    while (x != root && x.getColor() == BLACK) {
-      if (x == x.getParent().getLeft()) {
-        Node w = x.getParent().getRight();
-        if (w.getColor() == RED) {
-          w.setColor(BLACK);
-          x.getParent().setColor(RED);
-          leftRotate(x.parent);
-          w = x.getParent().getRight();
-        }
-        if (w.getLeft().getColor() == BLACK && w.getRight().getColor() == BLACK) {
-          w.setColor(RED);
-          x = x.getParent();
-          continue;
-        } else if (w.getRight().getColor() == BLACK) {
-          w.getLeft().setColor(BLACK);
-          w.setColor(RED);
-          rightRotate(w);
-          w = x.getParent().getRight();
-        }
-        if (w.getRight().getColor() == RED) {
-          w.setColor(x.getParent().getColor());
-          x.getParent().setColor(BLACK);
-          w.getRight().setColor(BLACK);
-          leftRotate(x.getParent());
-          x = root;
-        }
+  private void deleteFix(Node node) {
+    while (node != root && node.color == BLACK) {
+      boolean isLeft = (node == node.parent.left);
+      Node sibling = isLeft ? node.parent.right : node.parent.left;
+      if (sibling.color == RED) {
+        sibling.color = BLACK;
+        node.parent.color = RED;
+        rotate(node.parent, isLeft);
+        sibling = isLeft ? node.parent.right : node.parent.left;
+      }
+      if (sibling.left.color == BLACK && sibling.right.color == BLACK) {
+        sibling.color = RED;
+        node = node.parent;
       } else {
-        Node w = (x.getParent().getLeft());
-        if (w.color == RED) {
-          w.color = BLACK;
-          x.getParent().setColor(RED);
-          rightRotate(x.getParent());
-          w = (x.getParent()).getLeft();
-        }
-        if (w.right.color == BLACK && w.left.color == BLACK) {
-          w.color = RED;
-          x = x.getParent();
-          continue;
-        } else if (w.left.color == BLACK) {
-          w.right.color = BLACK;
-          w.color = RED;
-          leftRotate(w);
-          w = (x.getParent().getLeft());
-        }
-        if (w.left.color == RED) {
-          w.color = x.getParent().getColor();
-          x.getParent().setColor(BLACK);
-          w.left.color = BLACK;
-          rightRotate(x.getParent());
-          x = root;
+        if ((isLeft ? sibling.right : sibling.left).color == BLACK) {
+          (isLeft ? sibling.left : sibling.right).color = BLACK;
+          sibling.color = RED;
+          rotate(sibling, !isLeft);
+          sibling = isLeft ? node.parent.right : node.parent.left;
         }
+        sibling.color = node.parent.color;
+        node.parent.color = BLACK;
+        (isLeft ? sibling.right : sibling.left).color = BLACK;
+        rotate(node.parent, isLeft);
+        node = root;
       }
     }
-    x.setColor(BLACK);
-  }
-
-  private Node successor(Node root) {
-    if (root == NIL || root.left == NIL) return root;
-    else return successor(root.left);
-  }
-
-  private void transplant(Node u, Node v) {
-    if (u.parent == NIL) {
-      root = v;
-    } else if (u == u.parent.left) {
-      u.parent.left = v;
-    } else u.parent.right = v;
-    v.parent = u.parent;
-  }
-
-  private Node search(T val, Node curr) {
-    if (curr == NIL) return NIL;
-    else if (curr.value.equals(val)) return curr;
-    else if (curr.value.compareTo(val) < 0) return search(val, curr.right);
-    else return search(val, curr.left);
-  }
-
-  public int height() {
-    return height(root);
+    node.color = BLACK;
   }
 
-  private int height(Node curr) {
-    if (curr == NIL) {
-      return 0;
-    }
-    if (curr.left == NIL && curr.right == NIL) {
-      return 1;
+  private void rotate(Node node, boolean left) {
+    Node child = left ? node.right : node.left;
+    if (left) {
+      node.right = child.left;
+      if (child.left != NIL) child.left.parent = node;
+    } else {
+      node.left = child.right;
+      if (child.right != NIL) child.right.parent = node;
     }
-
-    return 1 + Math.max(height(curr.left), height(curr.right));
+    child.parent = node.parent;
+    if (node.parent == NIL) root = child;
+    else if (node == node.parent.left) node.parent.left = child;
+    else node.parent.right = child;
+    if (left) child.left = node; else child.right = node;
+    node.parent = child;
   }
 
-  private void swapColors(Node a, Node b) {
-    boolean tmpColor = a.color;
-    a.color = b.color;
-    b.color = tmpColor;
+  private void transplant(Node target, Node source) {
+    if (target.parent == NIL) root = source;
+    else if (target == target.parent.left) target.parent.left = source;
+    else target.parent.right = source;
+    source.parent = target.parent;
   }
 
-  // Sometimes the left or right child node of a parent changes and the
-  // parent's reference needs to be updated to point to the new child.
-  // This is a helper method to do just that.
-  private void updateParentChildLink(Node parent, Node oldChild, Node newChild) {
-    if (parent != NIL) {
-      if (parent.left == oldChild) {
-        parent.left = newChild;
-      } else {
-        parent.right = newChild;
-      }
-    }
-  }
-
-  // Helper method to find the leftmost node (which has the smallest value)
   private Node findMin(Node node) {
     while (node.left != NIL) node = node.left;
     return node;
   }
 
-  // Helper method to find the rightmost node (which has the largest value)
-  private Node findMax(Node node) {
-    while (node.right != NIL) node = node.right;
-    return node;
+  private Node search(T val) {
+    Node current = root;
+    while (current != NIL) {
+      int cmp = val.compareTo(current.value);
+      if (cmp < 0) current = current.left;
+      else if (cmp > 0) current = current.right;
+      else return current;
+    }
+    return NIL;
   }
 
-  // Returns as iterator to traverse the tree in order.
-  @Override
-  public java.util.Iterator<T> iterator() {
-
-    final int expectedNodeCount = nodeCount;
-    final java.util.Stack<Node> stack = new java.util.Stack<>();
-    stack.push(root);
-
-    return new java.util.Iterator<T>() {
-      Node trav = root;
+  public int height() { return height(root); }
+  private int height(Node node) {
+    return node == NIL ? 0 : 1 + Math.max(height(node.left), height(node.right));
+  }
 
-      @Override
+  @Override
+  public Iterator<T> iterator() {
+    int expectedCount = nodeCount;
+    Stack<Node> stack = new Stack<>();
+    pushLeft(root, stack);
+    return new Iterator<T>() {
       public boolean hasNext() {
-        if (expectedNodeCount != nodeCount) throw new java.util.ConcurrentModificationException();
-        return root != NIL && !stack.isEmpty();
+        if (expectedCount != nodeCount) throw new ConcurrentModificationException();
+        return !stack.isEmpty();
       }
-
-      @Override
       public T next() {
-
-        if (expectedNodeCount != nodeCount) throw new java.util.ConcurrentModificationException();
-
-        while (trav != NIL && trav.left != NIL) {
-          stack.push(trav.left);
-          trav = trav.left;
-        }
-
+        if (!hasNext()) throw new NoSuchElementException();
         Node node = stack.pop();
-
-        if (node.right != NIL) {
-          stack.push(node.right);
-          trav = node.right;
-        }
-
+        pushLeft(node.right, stack);
         return node.value;
       }
-
-      @Override
-      public void remove() {
-        throw new UnsupportedOperationException();
-      }
     };
   }
-
-  // Example usage of RB tree:
-  public static void main(String[] args) {
-
-    int[] values = {5, 8, 1, -4, 6, -2, 0, 7};
-    RedBlackTree<Integer> rbTree = new RedBlackTree<>();
-    for (int v : values) rbTree.insert(v);
-
-    System.out.printf("RB tree contains %d: %s\n", 6, rbTree.contains(6));
-    System.out.printf("RB tree contains %d: %s\n", -5, rbTree.contains(-5));
-    System.out.printf("RB tree contains %d: %s\n", 1, rbTree.contains(1));
-    System.out.printf("RB tree contains %d: %s\n", 99, rbTree.contains(99));
+  private void pushLeft(Node node, Stack<Node> stack) {
+    while (node != NIL) { stack.push(node); node = node.left; }
   }
 }
diff --git a/src/test/java/com/williamfiset/algorithms/datastructures/balancedtree/RedBlackTreeTest.java b/src/test/java/com/williamfiset/algorithms/datastructures/balancedtree/RedBlackTreeTest.java
index dd4e536cf..e75ad95ed 100644
--- a/src/test/java/com/williamfiset/algorithms/datastructures/balancedtree/RedBlackTreeTest.java
+++ b/src/test/java/com/williamfiset/algorithms/datastructures/balancedtree/RedBlackTreeTest.java
@@ -50,6 +50,7 @@ public void testLeftLeftRotation() {
 
     assertNullChildren(tree, tree.root.left, tree.root.right);
     assertCorrectParentLinks(tree, tree.root, tree.NIL);
+    verifyProperties(tree);
   }
 
   @Test
@@ -72,6 +73,7 @@ public void testLeftRightRotation() {
 
     assertNullChildren(tree, tree.root.left, tree.root.right);
     assertCorrectParentLinks(tree, tree.root, tree.NIL);
+    verifyProperties(tree);
   }
 
   @Test
@@ -94,6 +96,7 @@ public void testRightLeftRotation() {
 
     assertNullChildren(tree, tree.root.left, tree.root.right);
     assertCorrectParentLinks(tree, tree.root, tree.NIL);
+    verifyProperties(tree);
   }
 
   @Test
@@ -116,6 +119,7 @@ public void testRightRightRotation() {
 
     assertNullChildren(tree, tree.root.left, tree.root.right);
     assertCorrectParentLinks(tree, tree.root, tree.NIL);
+    verifyProperties(tree);
   }
 
   @Test
@@ -141,6 +145,7 @@ public void testLeftUncleCase() {
     assertThat(tree.root.right.left).isEqualTo(tree.NIL);
     assertNullChildren(tree, tree.root.left, tree.root.right.right);
     assertCorrectParentLinks(tree, tree.root, tree.NIL);
+    verifyProperties(tree);
 
     /* Black left uncle case. */
 
@@ -158,6 +163,7 @@ public void testLeftUncleCase() {
     assertThat(tree.root.right.left.color).isEqualTo(RedBlackTree.RED);
     assertThat(tree.root.right.right.color).isEqualTo(RedBlackTree.RED);
     assertCorrectParentLinks(tree, tree.root, tree.NIL);
+    verifyProperties(tree);
   }
 
   @Test
@@ -184,6 +190,7 @@ public void testRightUncleCase() {
     assertThat(tree.root.left.right).isEqualTo(tree.NIL);
     assertNullChildren(tree, tree.root.right, tree.root.left.left);
     assertCorrectParentLinks(tree, tree.root, tree.NIL);
+    verifyProperties(tree);
 
     /* Black right uncle case. */
 
@@ -201,6 +208,7 @@ public void testRightUncleCase() {
     assertThat(tree.root.left.left.color).isEqualTo(RedBlackTree.RED);
     assertThat(tree.root.left.right.color).isEqualTo(RedBlackTree.RED);
     assertCorrectParentLinks(tree, tree.root, tree.NIL);
+    verifyProperties(tree);
   }
 
   @Test
@@ -231,6 +239,7 @@ public void interestingCase1() {
     assertThat(tree.root.right.left.right.color).isEqualTo(RedBlackTree.RED);
     assertThat(tree.root.right.right.left.color).isEqualTo(RedBlackTree.RED);
     assertThat(tree.root.right.right.right.color).isEqualTo(RedBlackTree.RED);
+    verifyProperties(tree);
   }
 
   @Test
@@ -240,10 +249,12 @@ public void testRandomizedValueInsertionsAgainstTreeSet() {
     for (int i = 0; i < TEST_SZ; i++) {
       int v = randValue();
       assertThat(tree.insert(v)).isEqualTo(set.add(v));
-      assertThat(tree.size()).isEqualTo(tree.size());
+      assertThat(tree.size()).isEqualTo(set.size());
       assertThat(tree.contains(v)).isTrue();
       assertBinarySearchTreeInvariant(tree, tree.root);
+      if (i % 100 == 0) verifyProperties(tree);
     }
+    verifyProperties(tree);
   }
 
   @Test
@@ -254,12 +265,15 @@ public void testRemoval() {
 
     tree.delete(5);
     assertThat(tree.contains(5)).isFalse();
+    verifyProperties(tree);
 
     tree.delete(7);
     assertThat(tree.contains(7)).isFalse();
+    verifyProperties(tree);
 
     tree.delete(9);
     assertThat(tree.contains(9)).isFalse();
+    verifyProperties(tree);
   }
 
   @Test
@@ -275,7 +289,7 @@ public void testNumberDoesntExist() {
   @Test
   public void randomRemoveTests() {
     TreeSet<Integer> ts = new TreeSet<>();
-    for (int i = 0; i < TEST_SZ; i++) {
+    for (int i = 0; i < 100; i++) { // Reduced TEST_SZ for faster property verification
 
       List<Integer> lst = genRandList(i);
       for (Integer value : lst) {
@@ -292,6 +306,7 @@ public void randomRemoveTests() {
         assertThat(treeSetRemove).isEqualTo(treeRemove);
         assertThat(tree.contains(value)).isFalse();
         assertThat(tree.size()).isEqualTo(i - j - 1);
+        verifyProperties(tree);
       }
       assertThat(ts.isEmpty()).isEqualTo(tree.isEmpty());
     }
@@ -338,7 +353,8 @@ boolean assertBinarySearchTreeInvariant(RedBlackTree tree, RedBlackTree<Integer>
     if (node == tree.NIL) return true;
     boolean isValid = true;
     if (node.left != tree.NIL) isValid = node.left.value.compareTo(node.value) < 0;
-    if (node.right != tree.NIL) isValid = isValid && node.right.value.compareTo(node.value) > 0;
+    if (node.right != tree.NIL)
+      isValid = isValid && node.right.value.compareTo(node.value) > 0;
     return isValid
         && assertBinarySearchTreeInvariant(tree, node.left)
         && assertBinarySearchTreeInvariant(tree, node.right);
@@ -352,6 +368,44 @@ boolean validateParentLinksAreCorrect(RedBlackTree.Node node, RedBlackTree.Node
         && validateParentLinksAreCorrect(node.right, node);
   }
 
+  // Verify RB tree properties
+  private void verifyProperties(RedBlackTree<Integer> tree) {
+    if (tree.root == tree.NIL) return;
+
+    // 1. Every node is either red or black (Implicit)
+
+    // 2. The root is black
+    assertThat(tree.root.color).isEqualTo(RedBlackTree.BLACK);
+
+    // 3. Every leaf (NIL) is black
+    assertThat(tree.NIL.color).isEqualTo(RedBlackTree.BLACK);
+
+    verifyRedNodeProperty(tree, tree.root);
+    verifyBlackHeightProperty(tree, tree.root);
+  }
+
+  private void verifyRedNodeProperty(RedBlackTree<Integer> tree, RedBlackTree<Integer>.Node node) {
+    if (node == tree.NIL) return;
+    if (node.color == RedBlackTree.RED) {
+      assertThat(node.left.color).isEqualTo(RedBlackTree.BLACK);
+      assertThat(node.right.color).isEqualTo(RedBlackTree.BLACK);
+    }
+    verifyRedNodeProperty(tree, node.left);
+    verifyRedNodeProperty(tree, node.right);
+  }
+
+  private int verifyBlackHeightProperty(
+      RedBlackTree<Integer> tree, RedBlackTree<Integer>.Node node) {
+    if (node == tree.NIL) return 1;
+
+    int leftHeight = verifyBlackHeightProperty(tree, node.left);
+    int rightHeight = verifyBlackHeightProperty(tree, node.right);
+
+    assertThat(leftHeight).isEqualTo(rightHeight);
+
+    return leftHeight + (node.color == RedBlackTree.BLACK ? 1 : 0);
+  }
+
   static List<Integer> genRandList(int sz) {
     List<Integer> lst = new ArrayList<>(sz);
     for (int i = 0; i < sz; i++) lst.add(i); // unique values.

From 60aaed848ceaf31c074e05c78c65520e15abfcba Mon Sep 17 00:00:00 2001
From: William Fiset <fisetwill@gmail.com>
Date: Fri, 6 Mar 2026 18:36:25 -0800
Subject: [PATCH 3/7] Cleanup Fenwick tree implementations and expand tests
 (#1257)

- Make N field private in both FenwickTree classes
- Add proper import for java.util.Arrays, fix misleading constructor comment
- Add input validation: negative size, out-of-range indices in sum()
- Add size() method to FenwickTreeRangeQueryPointUpdate
- Remove commented-out debug prints and dead assertions in tests
- Expand RangeQueryPointUpdate tests from 7 to 14
- Expand RangeUpdatePointQuery tests from 6 to 10

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
---
 .../FenwickTreeRangeQueryPointUpdate.java     |  16 ++-
 .../FenwickTreeRangeUpdatePointQuery.java     |   2 +-
 .../FenwickTreeRangeQueryPointUpdateTest.java | 105 ++++++++++++++----
 .../FenwickTreeRangeUpdatePointQueryTest.java |  49 ++++++++
 4 files changed, 149 insertions(+), 23 deletions(-)

diff --git a/src/main/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeQueryPointUpdate.java b/src/main/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeQueryPointUpdate.java
index 3fa9c7205..1925c4d40 100644
--- a/src/main/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeQueryPointUpdate.java
+++ b/src/main/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeQueryPointUpdate.java
@@ -5,16 +5,19 @@
  */
 package com.williamfiset.algorithms.datastructures.fenwicktree;
 
+import java.util.Arrays;
+
 public class FenwickTreeRangeQueryPointUpdate {
 
   // The size of the array holding the Fenwick tree values
-  final int N;
+  private final int N;
 
   // This array contains the Fenwick tree ranges
   private long[] tree;
 
-  // Create an empty Fenwick Tree with 'sz' parameter zero based.
+  // Create an empty Fenwick Tree with 'sz' elements (one-based internally).
   public FenwickTreeRangeQueryPointUpdate(int sz) {
+    if (sz < 0) throw new IllegalArgumentException("Size cannot be negative!");
     tree = new long[(N = sz + 1)];
   }
 
@@ -66,6 +69,8 @@ private long prefixSum(int i) {
   // Returns the sum of the interval [left, right], O(log(n))
   public long sum(int left, int right) {
     if (right < left) throw new IllegalArgumentException("Make sure right >= left");
+    if (left < 1 || right >= N)
+      throw new IndexOutOfBoundsException("Index out of range [1, " + (N - 1) + "]");
     return prefixSum(right) - prefixSum(left - 1);
   }
 
@@ -87,8 +92,13 @@ public void set(int i, long v) {
     add(i, v - sum(i, i));
   }
 
+  // Returns the number of elements in the Fenwick tree.
+  public int size() {
+    return N - 1;
+  }
+
   @Override
   public String toString() {
-    return java.util.Arrays.toString(tree);
+    return Arrays.toString(tree);
   }
 }
diff --git a/src/main/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeUpdatePointQuery.java b/src/main/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeUpdatePointQuery.java
index bc38ab20b..21930b530 100644
--- a/src/main/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeUpdatePointQuery.java
+++ b/src/main/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeUpdatePointQuery.java
@@ -8,7 +8,7 @@
 public class FenwickTreeRangeUpdatePointQuery {
 
   // The size of the array holding the Fenwick tree values
-  final int N;
+  private final int N;
 
   // This array contains the original Fenwick tree range
   // values from when it was first created.
diff --git a/src/test/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeQueryPointUpdateTest.java b/src/test/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeQueryPointUpdateTest.java
index 5f5672ced..5c35fe466 100644
--- a/src/test/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeQueryPointUpdateTest.java
+++ b/src/test/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeQueryPointUpdateTest.java
@@ -20,10 +20,87 @@ public void setup() {
     UNUSED_VAL = randValue();
   }
 
+  @Test
+  public void testIllegalCreation() {
+    assertThrows(IllegalArgumentException.class, () -> new FenwickTreeRangeQueryPointUpdate(null));
+  }
+
+  @Test
+  public void testNegativeSizeCreation() {
+    assertThrows(IllegalArgumentException.class, () -> new FenwickTreeRangeQueryPointUpdate(-1));
+  }
+
+  @Test
+  public void testEmptyTreeCreation() {
+    FenwickTreeRangeQueryPointUpdate ft = new FenwickTreeRangeQueryPointUpdate(0);
+    assertThat(ft.size()).isEqualTo(0);
+  }
+
+  @Test
+  public void testSingleElement() {
+    long[] ar = {UNUSED_VAL, 7};
+    FenwickTreeRangeQueryPointUpdate ft = new FenwickTreeRangeQueryPointUpdate(ar);
+    assertThat(ft.get(1)).isEqualTo(7);
+    assertThat(ft.sum(1, 1)).isEqualTo(7);
+    assertThat(ft.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testGetValue() {
+    long[] ar = {UNUSED_VAL, 1, 2, 3, 4, 5};
+    FenwickTreeRangeQueryPointUpdate ft = new FenwickTreeRangeQueryPointUpdate(ar);
+
+    for (int i = 1; i <= 5; i++) {
+      assertThat(ft.get(i)).isEqualTo(i);
+    }
+  }
+
+  @Test
+  public void testSetValue() {
+    long[] ar = {UNUSED_VAL, 1, 2, 3, 4, 5};
+    FenwickTreeRangeQueryPointUpdate ft = new FenwickTreeRangeQueryPointUpdate(ar);
+
+    ft.set(3, 99);
+    assertThat(ft.get(3)).isEqualTo(99);
+    // Other values should be unchanged.
+    assertThat(ft.get(1)).isEqualTo(1);
+    assertThat(ft.get(2)).isEqualTo(2);
+    assertThat(ft.get(4)).isEqualTo(4);
+    assertThat(ft.get(5)).isEqualTo(5);
+    // Sum should reflect the new value.
+    assertThat(ft.sum(1, 5)).isEqualTo(1 + 2 + 99 + 4 + 5);
+  }
+
+  @Test
+  public void testAddValue() {
+    long[] ar = {UNUSED_VAL, 1, 2, 3, 4, 5};
+    FenwickTreeRangeQueryPointUpdate ft = new FenwickTreeRangeQueryPointUpdate(ar);
+
+    ft.add(3, 10);
+    assertThat(ft.get(3)).isEqualTo(13);
+    assertThat(ft.sum(1, 5)).isEqualTo(25);
+  }
+
+  @Test
+  public void testSumRangeInvalid() {
+    long[] ar = {UNUSED_VAL, 1, 2, 3};
+    FenwickTreeRangeQueryPointUpdate ft = new FenwickTreeRangeQueryPointUpdate(ar);
+
+    assertThrows(IllegalArgumentException.class, () -> ft.sum(3, 1));
+  }
+
+  @Test
+  public void testSumRangeOutOfBounds() {
+    long[] ar = {UNUSED_VAL, 1, 2, 3};
+    FenwickTreeRangeQueryPointUpdate ft = new FenwickTreeRangeQueryPointUpdate(ar);
+
+    assertThrows(IndexOutOfBoundsException.class, () -> ft.sum(0, 2));
+    assertThrows(IndexOutOfBoundsException.class, () -> ft.sum(1, 4));
+  }
+
   @Test
   public void testIntervalSumPositiveValues() {
 
-    // System.out.println("testIntervalSumPositiveValues");
     long[] ar = {UNUSED_VAL, 1, 2, 3, 4, 5, 6};
     FenwickTreeRangeQueryPointUpdate ft = new FenwickTreeRangeQueryPointUpdate(ar);
 
@@ -33,7 +110,6 @@ public void testIntervalSumPositiveValues() {
     assertThat(ft.sum(1, 3)).isEqualTo(6);
     assertThat(ft.sum(1, 2)).isEqualTo(3);
     assertThat(ft.sum(1, 1)).isEqualTo(1);
-    // assertThat(ft.sum(1, 0)).isEqualTo(0);
 
     assertThat(ft.sum(3, 4)).isEqualTo(7);
     assertThat(ft.sum(2, 6)).isEqualTo(20);
@@ -49,7 +125,6 @@ public void testIntervalSumPositiveValues() {
   @Test
   public void testIntervalSumNegativeValues() {
 
-    // System.out.println("testIntervalSumNegativeValues");
     long[] ar = {UNUSED_VAL, -1, -2, -3, -4, -5, -6};
     FenwickTreeRangeQueryPointUpdate ft = new FenwickTreeRangeQueryPointUpdate(ar);
 
@@ -71,18 +146,12 @@ public void testIntervalSumNegativeValues() {
   @Test
   public void testIntervalSumNegativeValues2() {
 
-    // System.out.println("testIntervalSumNegativeValues2");
     long[] ar = {UNUSED_VAL, -76871, -164790};
     FenwickTreeRangeQueryPointUpdate ft = new FenwickTreeRangeQueryPointUpdate(ar);
 
     for (int i = 0; i < LOOPS; i++) {
-      assertThat(ft.sum(1, 1)).isEqualTo(-76871);
       assertThat(ft.sum(1, 1)).isEqualTo(-76871);
       assertThat(ft.sum(1, 2)).isEqualTo(-241661);
-      assertThat(ft.sum(1, 2)).isEqualTo(-241661);
-      assertThat(ft.sum(1, 2)).isEqualTo(-241661);
-      assertThat(ft.sum(2, 2)).isEqualTo(-164790);
-      assertThat(ft.sum(2, 2)).isEqualTo(-164790);
       assertThat(ft.sum(2, 2)).isEqualTo(-164790);
     }
   }
@@ -90,7 +159,6 @@ public void testIntervalSumNegativeValues2() {
   @Test
   public void testRandomizedStaticSumQueries() {
 
-    // System.out.println("testRandomizedStaticSumQueries");
     for (int i = 1; i <= LOOPS; i++) {
 
       long[] randList = genRandList(i);
@@ -110,8 +178,6 @@ public void doRandomRangeQuery(long[] arr, FenwickTreeRangeQueryPointUpdate ft)
     int lo = lowBound(N);
     int hi = highBound(lo, N);
 
-    // System.out.println("LO: " + lo + " HI: " + hi + " N: " + N);
-
     for (int k = lo; k <= hi; k++) sum += arr[k];
 
     assertThat(ft.sum(lo, hi)).isEqualTo(sum);
@@ -120,7 +186,6 @@ public void doRandomRangeQuery(long[] arr, FenwickTreeRangeQueryPointUpdate ft)
   @Test
   public void testRandomizedSetSumQueries() {
 
-    // System.out.println("testRandomizedSetSumQueries");
     for (int n = 2; n <= LOOPS; n++) {
 
       long[] randList = genRandList(n);
@@ -128,7 +193,7 @@ public void testRandomizedSetSumQueries() {
 
       for (int j = 0; j < LOOPS / 10; j++) {
 
-        int index = 1 + (int)(Math.random() * n);
+        int index = 1 + (int) (Math.random() * n);
         long rand_val = randValue();
 
         randList[index] += rand_val;
@@ -157,6 +222,13 @@ public void testReusability() {
     }
   }
 
+  @Test
+  public void testToString() {
+    long[] ar = {UNUSED_VAL, 1, 2, 3};
+    FenwickTreeRangeQueryPointUpdate ft = new FenwickTreeRangeQueryPointUpdate(ar);
+    assertThat(ft.toString()).isNotEmpty();
+  }
+
   public static int lowBound(int N) {
     return 1 + (int) (Math.random() * N);
   }
@@ -169,11 +241,6 @@ public static long randValue() {
     return (long) (Math.random() * MAX_RAND_NUM * 2) + MIN_RAND_NUM;
   }
 
-  @Test
-  public void testIllegalCreation() {
-    assertThrows(IllegalArgumentException.class, () -> new FenwickTreeRangeQueryPointUpdate(null));
-  }
-
   // Generate a list of random numbers, one based
   static long[] genRandList(int sz) {
     long[] lst = new long[sz + 1];
diff --git a/src/test/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeUpdatePointQueryTest.java b/src/test/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeUpdatePointQueryTest.java
index 13e34e35f..470c530c6 100644
--- a/src/test/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeUpdatePointQueryTest.java
+++ b/src/test/java/com/williamfiset/algorithms/datastructures/fenwicktree/FenwickTreeRangeUpdatePointQueryTest.java
@@ -41,6 +41,21 @@ public void testIllegalCreation() {
     assertThrows(IllegalArgumentException.class, () -> new FenwickTreeRangeUpdatePointQuery(null));
   }
 
+  @Test
+  public void testSingleElement() {
+    long[] values = {UNUSED_VAL, 42};
+    FenwickTreeRangeUpdatePointQuery ft = new FenwickTreeRangeUpdatePointQuery(values);
+    assertThat(ft.get(1)).isEqualTo(42);
+  }
+
+  @Test
+  public void testSingleElementUpdate() {
+    long[] values = {UNUSED_VAL, 42};
+    FenwickTreeRangeUpdatePointQuery ft = new FenwickTreeRangeUpdatePointQuery(values);
+    ft.updateRange(1, 1, 8);
+    assertThat(ft.get(1)).isEqualTo(50);
+  }
+
   @Test
   public void testFenwickTreeRangeUpdatePointQueryNegativeNumbers() {
 
@@ -98,6 +113,40 @@ public void testFenwickTreeRangeUpdatePointQueryRepeatedAddition() {
     }
   }
 
+  @Test
+  public void testFullRangeUpdate() {
+    long[] values = {UNUSED_VAL, 0, 0, 0, 0, 0};
+    FenwickTreeRangeUpdatePointQuery ft = new FenwickTreeRangeUpdatePointQuery(values);
+    ft.updateRange(1, 5, 5);
+    for (int i = 1; i <= 5; i++) {
+      assertThat(ft.get(i)).isEqualTo(5);
+    }
+  }
+
+  @Test
+  public void testMultipleNonOverlappingUpdates() {
+    long[] values = {UNUSED_VAL, 0, 0, 0, 0, 0, 0};
+    FenwickTreeRangeUpdatePointQuery ft = new FenwickTreeRangeUpdatePointQuery(values);
+    ft.updateRange(1, 2, 10);
+    ft.updateRange(4, 5, 20);
+    assertThat(ft.get(1)).isEqualTo(10);
+    assertThat(ft.get(2)).isEqualTo(10);
+    assertThat(ft.get(3)).isEqualTo(0);
+    assertThat(ft.get(4)).isEqualTo(20);
+    assertThat(ft.get(5)).isEqualTo(20);
+    assertThat(ft.get(6)).isEqualTo(0);
+  }
+
+  @Test
+  public void testNegativeRangeUpdate() {
+    long[] values = {UNUSED_VAL, 10, 10, 10};
+    FenwickTreeRangeUpdatePointQuery ft = new FenwickTreeRangeUpdatePointQuery(values);
+    ft.updateRange(1, 3, -5);
+    assertThat(ft.get(1)).isEqualTo(5);
+    assertThat(ft.get(2)).isEqualTo(5);
+    assertThat(ft.get(3)).isEqualTo(5);
+  }
+
   @Test
   public void testFenwickTreeRangeUpdatePointQueryOverlappingRanges() {
 

From 571e2d5522404a6c543549a017528caf952c1456 Mon Sep 17 00:00:00 2001
From: William Fiset <fisetwill@gmail.com>
Date: Fri, 6 Mar 2026 18:43:29 -0800
Subject: [PATCH 4/7] Restore educational comments in RedBlackTree (#1259)

---
 .../balancedtree/RedBlackTree.java            | 137 ++++++++++++++----
 1 file changed, 107 insertions(+), 30 deletions(-)

diff --git a/src/main/java/com/williamfiset/algorithms/datastructures/balancedtree/RedBlackTree.java b/src/main/java/com/williamfiset/algorithms/datastructures/balancedtree/RedBlackTree.java
index a249b3d28..0f9661f39 100644
--- a/src/main/java/com/williamfiset/algorithms/datastructures/balancedtree/RedBlackTree.java
+++ b/src/main/java/com/williamfiset/algorithms/datastructures/balancedtree/RedBlackTree.java
@@ -17,8 +17,13 @@ public class RedBlackTree<T extends Comparable<T>> implements Iterable<T> {
   public static final boolean BLACK = false;
 
   public class Node {
+    // The color of this node. By default all nodes start red.
     public boolean color = RED;
+
+    // The value/data contained within the node.
     public T value;
+
+    // The left, right and parent references of this node.
     public Node left, right, parent;
 
     public Node(T value, boolean color, Node parent, Node left, Node right) {
@@ -30,8 +35,12 @@ public Node(T value, boolean color, Node parent, Node left, Node right) {
     }
   }
 
+  // The root node of the RB tree.
   public Node root;
+
+  // Tracks the number of nodes inside the tree.
   private int nodeCount = 0;
+
   public final Node NIL;
 
   public RedBlackTree() {
@@ -40,24 +49,50 @@ public RedBlackTree() {
     root = NIL;
   }
 
-  public int size() { return nodeCount; }
-  public boolean isEmpty() { return nodeCount == 0; }
-  public boolean contains(T value) { return search(value) != NIL; }
+  // Returns the number of nodes in the tree.
+  public int size() {
+    return nodeCount;
+  }
+
+  // Returns whether or not the tree is empty.
+  public boolean isEmpty() {
+    return nodeCount == 0;
+  }
+
+  // Return true/false depending on whether a value exists in the tree.
+  public boolean contains(T value) {
+    return search(value) != NIL;
+  }
 
   public boolean insert(T val) {
-    if (val == null) throw new IllegalArgumentException("Null values not allowed.");
+    if (val == null) {
+      throw new IllegalArgumentException("Red-Black tree does not allow null values.");
+    }
+
     Node parent = NIL, current = root;
+
     while (current != NIL) {
       parent = current;
       int cmp = val.compareTo(current.value);
-      if (cmp < 0) current = current.left;
-      else if (cmp > 0) current = current.right;
-      else return false;
+      if (cmp < 0) {
+        current = current.left;
+      } else if (cmp > 0) {
+        current = current.right;
+      } else {
+        return false;
+      }
     }
+
     Node newNode = new Node(val, RED, parent, NIL, NIL);
-    if (parent == NIL) root = newNode;
-    else if (val.compareTo(parent.value) < 0) parent.left = newNode;
-    else parent.right = newNode;
+
+    if (parent == NIL) {
+      root = newNode;
+    } else if (val.compareTo(parent.value) < 0) {
+      parent.left = newNode;
+    } else {
+      parent.right = newNode;
+    }
+
     insertFix(newNode);
     nodeCount++;
     return true;
@@ -87,16 +122,23 @@ private void insertFix(Node node) {
   public boolean delete(T key) {
     Node nodeToDelete = search(key);
     if (nodeToDelete == NIL) return false;
+
     Node successor = nodeToDelete, replacement;
     boolean successorOriginalColor = successor.color;
-    if (nodeToDelete.left == NIL) transplant(nodeToDelete, replacement = nodeToDelete.right);
-    else if (nodeToDelete.right == NIL) transplant(nodeToDelete, replacement = nodeToDelete.left);
-    else {
+
+    if (nodeToDelete.left == NIL) {
+      replacement = nodeToDelete.right;
+      transplant(nodeToDelete, nodeToDelete.right);
+    } else if (nodeToDelete.right == NIL) {
+      replacement = nodeToDelete.left;
+      transplant(nodeToDelete, nodeToDelete.left);
+    } else {
       successor = findMin(nodeToDelete.right);
       successorOriginalColor = successor.color;
       replacement = successor.right;
-      if (successor.parent == nodeToDelete) replacement.parent = successor;
-      else {
+      if (successor.parent == nodeToDelete) {
+        replacement.parent = successor;
+      } else {
         transplant(successor, successor.right);
         successor.right = nodeToDelete.right;
         successor.right.parent = successor;
@@ -106,7 +148,11 @@ public boolean delete(T key) {
       successor.left.parent = successor;
       successor.color = nodeToDelete.color;
     }
-    if (successorOriginalColor == BLACK) deleteFix(replacement);
+
+    if (successorOriginalColor == BLACK) {
+      deleteFix(replacement);
+    }
+
     nodeCount--;
     return true;
   }
@@ -141,6 +187,8 @@ private void deleteFix(Node node) {
     node.color = BLACK;
   }
 
+  // Unified rotation method. If left is true, performs a left rotation on node x.
+  // Otherwise performs a right rotation.
   private void rotate(Node node, boolean left) {
     Node child = left ? node.right : node.left;
     if (left) {
@@ -151,20 +199,30 @@ private void rotate(Node node, boolean left) {
       if (child.right != NIL) child.right.parent = node;
     }
     child.parent = node.parent;
-    if (node.parent == NIL) root = child;
-    else if (node == node.parent.left) node.parent.left = child;
-    else node.parent.right = child;
-    if (left) child.left = node; else child.right = node;
+    if (node.parent == NIL) {
+      root = child;
+    } else if (node == node.parent.left) {
+      node.parent.left = child;
+    } else {
+      node.parent.right = child;
+    }
+    if (left) child.left = node;
+    else child.right = node;
     node.parent = child;
   }
 
   private void transplant(Node target, Node source) {
-    if (target.parent == NIL) root = source;
-    else if (target == target.parent.left) target.parent.left = source;
-    else target.parent.right = source;
+    if (target.parent == NIL) {
+      root = source;
+    } else if (target == target.parent.left) {
+      target.parent.left = source;
+    } else {
+      target.parent.right = source;
+    }
     source.parent = target.parent;
   }
 
+  // Helper method to find the leftmost node (which has the smallest value)
   private Node findMin(Node node) {
     while (node.left != NIL) node = node.left;
     return node;
@@ -174,28 +232,43 @@ private Node search(T val) {
     Node current = root;
     while (current != NIL) {
       int cmp = val.compareTo(current.value);
-      if (cmp < 0) current = current.left;
-      else if (cmp > 0) current = current.right;
-      else return current;
+      if (cmp < 0) {
+        current = current.left;
+      } else if (cmp > 0) {
+        current = current.right;
+      } else {
+        return current;
+      }
     }
     return NIL;
   }
 
-  public int height() { return height(root); }
+  // The height of a rooted tree is the number of edges between the tree's
+  // root and its furthest leaf. This means that a tree containing a single
+  // node has a height of 0.
+  public int height() {
+    return height(root);
+  }
+
   private int height(Node node) {
     return node == NIL ? 0 : 1 + Math.max(height(node.left), height(node.right));
   }
 
+  // Returns as iterator to traverse the tree in order.
   @Override
   public Iterator<T> iterator() {
-    int expectedCount = nodeCount;
-    Stack<Node> stack = new Stack<>();
+    final int expectedCount = nodeCount;
+    final Stack<Node> stack = new Stack<>();
     pushLeft(root, stack);
+
     return new Iterator<T>() {
+      @Override
       public boolean hasNext() {
         if (expectedCount != nodeCount) throw new ConcurrentModificationException();
         return !stack.isEmpty();
       }
+
+      @Override
       public T next() {
         if (!hasNext()) throw new NoSuchElementException();
         Node node = stack.pop();
@@ -204,7 +277,11 @@ public T next() {
       }
     };
   }
+
   private void pushLeft(Node node, Stack<Node> stack) {
-    while (node != NIL) { stack.push(node); node = node.left; }
+    while (node != NIL) {
+      stack.push(node);
+      node = node.left;
+    }
   }
 }

From 2dde6dc4ea65e218895a4e7d07411a3827277a78 Mon Sep 17 00:00:00 2001
From: William Fiset <fisetwill@gmail.com>
Date: Fri, 6 Mar 2026 18:43:49 -0800
Subject: [PATCH 5/7] Cleanup and simplify hashtable implementations and
 improve test coverage (#1258)

* Cleanup and simplify hashtable implementations and improve test coverage

* Restore educational comments and update .gemini.md
---
 .gemini.md                                    |   5 +
 .../hashtable/DoubleHashingTestObject.java    | 100 +++++++++---------
 .../HashTableOpenAddressingBase.java          |  65 +++++-------
 .../hashtable/HashTableSeparateChaining.java  |  41 ++++---
 .../hashtable/DoubleHashingTestObject.java    | 100 ------------------
 .../hashtable/HashTableDoubleHashingTest.java |  31 ++++++
 .../hashtable/HashTableLinearProbingTest.java |  64 +++++++++--
 .../HashTableQuadraticProbingTest.java        |  27 +++++
 .../HashTableSeparateChainingTest.java        |  51 +++++++--
 9 files changed, 260 insertions(+), 224 deletions(-)
 delete mode 100644 src/test/java/com/williamfiset/algorithms/datastructures/hashtable/DoubleHashingTestObject.java

diff --git a/.gemini.md b/.gemini.md
index 70997c9be..61e69caa0 100644
--- a/.gemini.md
+++ b/.gemini.md
@@ -47,6 +47,11 @@ java -cp classes com.williamfiset.algorithms.<package>.<ClassName>
 
 ## Development Conventions
 
+### Documentation and Comments
+- **Educational Context:** This repository is an educational project. Most comments (except for the most trivial ones) must be preserved during refactoring.
+- **Purpose:** Comments should explain *how* and *why* an algorithm or data structure works to aid understanding for students and developers.
+- **Refactoring:** When simplifying code, ensure that the conceptual explanations remain intact.
+
 ### Project Structure
 - `src/main/java/com/williamfiset/algorithms/`: Implementation source code.
 - `src/test/java/com/williamfiset/algorithms/`: Unit tests (mirrors source structure).
diff --git a/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/DoubleHashingTestObject.java b/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/DoubleHashingTestObject.java
index 59a66ddd7..4f29d9376 100644
--- a/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/DoubleHashingTestObject.java
+++ b/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/DoubleHashingTestObject.java
@@ -6,76 +6,75 @@
  */
 package com.williamfiset.algorithms.datastructures.hashtable;
 
+import java.util.Arrays;
 import java.util.Random;
 
 public class DoubleHashingTestObject implements SecondaryHash {
 
   private int hash, hash2;
-  Integer intData = null;
-  int[] vectorData = null;
-  String stringData = null;
+  private final Integer intData;
+  private final int[] vectorData;
+  private final String stringData;
 
-  static long[] randomVector;
-  static Random R = new Random();
-  static int MAX_VECTOR_SIZE = 10000;
+  private static final long[] RANDOM_VECTOR;
+  private static final int MAX_VECTOR_SIZE = 10000;
 
   static {
-    randomVector = new long[MAX_VECTOR_SIZE];
+    RANDOM_VECTOR = new long[MAX_VECTOR_SIZE];
+    Random r = new Random();
     for (int i = 0; i < MAX_VECTOR_SIZE; i++) {
-      long val = R.nextLong();
-      while (val % 2 == 0) val = R.nextLong();
-      randomVector[i] = val;
+      long val = r.nextLong();
+      while (val % 2 == 0) val = r.nextLong();
+      RANDOM_VECTOR[i] = val;
     }
   }
 
   public DoubleHashingTestObject(int data) {
-    intData = data;
-    intHash();
-    computeHash();
+    this.intData = data;
+    this.vectorData = null;
+    this.stringData = null;
+    computeHashes();
   }
 
   public DoubleHashingTestObject(int[] data) {
     if (data == null) throw new IllegalArgumentException("Cannot be null");
-    vectorData = data;
-    vectorHash();
-    computeHash();
+    this.intData = null;
+    this.vectorData = data;
+    this.stringData = null;
+    computeHashes();
   }
 
   public DoubleHashingTestObject(String data) {
     if (data == null) throw new IllegalArgumentException("Cannot be null");
-    stringData = data;
-    stringHash();
-    computeHash();
+    this.intData = null;
+    this.vectorData = null;
+    this.stringData = data;
+    computeHashes();
   }
 
-  private void intHash() {
-    hash2 = intData;
-  }
-
-  private void vectorHash() {
-    for (int i = 0; i < vectorData.length; i++) hash2 += randomVector[i] * vectorData[i];
-  }
-
-  private void stringHash() {
-
-    // Multipicative hash function:
-    final int INITIAL_VALUE = 0;
-    int prime = 17;
-    int power = 1;
-    hash = INITIAL_VALUE;
-    for (int i = 0; i < stringData.length(); i++) {
-      int ch = stringData.charAt(i);
-      hash2 += power * ch;
-      power *= prime;
+  private void computeHashes() {
+    if (intData != null) {
+      hash = intData.hashCode();
+      hash2 = intData;
+    } else if (stringData != null) {
+      hash = stringData.hashCode();
+      // Multiplicative hash function for hash2
+      int prime = 17;
+      int power = 1;
+      hash2 = 0;
+      for (int i = 0; i < stringData.length(); i++) {
+        hash2 += power * stringData.charAt(i);
+        power *= prime;
+      }
+    } else {
+      hash = Arrays.hashCode(vectorData);
+      hash2 = 0;
+      for (int i = 0; i < vectorData.length; i++) {
+        hash2 += RANDOM_VECTOR[i] * vectorData[i];
+      }
     }
   }
 
-  private void computeHash() {
-    if (intData != null) hash = intData.hashCode();
-    else if (stringData != null) hash = stringData.hashCode();
-    else hash = java.util.Arrays.hashCode(vectorData);
-  }
-
   @Override
   public int hashCode() {
     return hash;
@@ -89,12 +88,11 @@ public int hashCode2() {
   @Override
   public boolean equals(Object o) {
     if (this == o) return true;
-    else if (o instanceof DoubleHashingTestObject) {
-      DoubleHashingTestObject obj = (DoubleHashingTestObject) o;
-      if (hash != obj.hash) return false;
-      if (intData != null) return intData.equals(obj.intData);
-      if (vectorData != null) return java.util.Arrays.equals(vectorData, obj.vectorData);
-      return stringData.equals(obj.stringData);
-    } else return false;
+    if (!(o instanceof DoubleHashingTestObject)) return false;
+    DoubleHashingTestObject obj = (DoubleHashingTestObject) o;
+    if (hash != obj.hash) return false;
+    if (intData != null) return intData.equals(obj.intData);
+    if (vectorData != null) return Arrays.equals(vectorData, obj.vectorData);
+    return stringData.equals(obj.stringData);
   }
 }
diff --git a/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableOpenAddressingBase.java b/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableOpenAddressingBase.java
index 45996ef89..efd112539 100644
--- a/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableOpenAddressingBase.java
+++ b/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableOpenAddressingBase.java
@@ -6,10 +6,7 @@
  */
 package com.williamfiset.algorithms.datastructures.hashtable;
 
-import java.util.ArrayList;
-import java.util.ConcurrentModificationException;
-import java.util.Iterator;
-import java.util.List;
+import java.util.*;
 
 @SuppressWarnings("unchecked")
 public abstract class HashTableOpenAddressingBase<K, V> implements Iterable<K> {
@@ -27,7 +24,7 @@ public abstract class HashTableOpenAddressingBase<K, V> implements Iterable<K> {
   protected V[] values;
 
   // Special marker token used to indicate the deletion of a key-value pair
-  protected final K TOMBSTONE = (K) (new Object());
+  protected final K TOMBSTONE = (K) new Object();
 
   private static final int DEFAULT_CAPACITY = 7;
   private static final double DEFAULT_LOAD_FACTOR = 0.65;
@@ -73,10 +70,8 @@ protected void increaseCapacity() {
   }
 
   public void clear() {
-    for (int i = 0; i < capacity; i++) {
-      keys[i] = null;
-      values[i] = null;
-    }
+    Arrays.fill(keys, null);
+    Arrays.fill(values, null);
     keyCount = usedBuckets = 0;
     modificationCount++;
   }
@@ -112,16 +107,22 @@ public boolean containsKey(K key) {
   // Returns a list of keys found in the hash table
   public List<K> keys() {
     List<K> hashtableKeys = new ArrayList<>(size());
-    for (int i = 0; i < capacity; i++)
-      if (keys[i] != null && keys[i] != TOMBSTONE) hashtableKeys.add(keys[i]);
+    for (int i = 0; i < capacity; i++) {
+      if (keys[i] != null && keys[i] != TOMBSTONE) {
+        hashtableKeys.add(keys[i]);
+      }
+    }
     return hashtableKeys;
   }
 
   // Returns a list of non-unique values found in the hash table
   public List<V> values() {
     List<V> hashtableValues = new ArrayList<>(size());
-    for (int i = 0; i < capacity; i++)
-      if (keys[i] != null && keys[i] != TOMBSTONE) hashtableValues.add(values[i]);
+    for (int i = 0; i < capacity; i++) {
+      if (keys[i] != null && keys[i] != TOMBSTONE) {
+        hashtableValues.add(values[i]);
+      }
+    }
     return hashtableValues;
   }
 
@@ -132,26 +133,20 @@ protected void resizeTable() {
 
     threshold = (int) (capacity * loadFactor);
 
-    K[] oldKeyTable = (K[]) new Object[capacity];
-    V[] oldValueTable = (V[]) new Object[capacity];
-
-    // Perform key table pointer swap
-    K[] keyTableTmp = keys;
-    keys = oldKeyTable;
-    oldKeyTable = keyTableTmp;
+    K[] oldKeyTable = keys;
+    V[] oldValueTable = values;
 
-    // Perform value table pointer swap
-    V[] valueTableTmp = values;
-    values = oldValueTable;
-    oldValueTable = valueTableTmp;
+    keys = (K[]) new Object[capacity];
+    values = (V[]) new Object[capacity];
 
     // Reset the key count and buckets used since we are about to
     // re-insert all the keys into the hash-table.
     keyCount = usedBuckets = 0;
 
     for (int i = 0; i < oldKeyTable.length; i++) {
-      if (oldKeyTable[i] != null && oldKeyTable[i] != TOMBSTONE)
+      if (oldKeyTable[i] != null && oldKeyTable[i] != TOMBSTONE) {
         insert(oldKeyTable[i], oldValueTable[i]);
+      }
       oldValueTable[i] = null;
       oldKeyTable[i] = null;
     }
@@ -189,7 +184,6 @@ public V insert(K key, V val) {
         // The key we're trying to insert already exists in the hash-table,
         // so update its value with the most recent value
         if (keys[i].equals(key)) {
-
           V oldValue = values[i];
           if (j == -1) {
             values[i] = val;
@@ -220,7 +214,6 @@ public V insert(K key, V val) {
           keys[j] = key;
           values[j] = val;
         }
-
         modificationCount++;
         return null;
       }
@@ -241,15 +234,12 @@ public boolean hasKey(K key) {
       // Ignore deleted cells, but record where the first index
       // of a deleted cell is found to perform lazy relocation later.
       if (keys[i] == TOMBSTONE) {
-
         if (j == -1) j = i;
 
         // We hit a non-null key, perhaps it's the one we're looking for.
       } else if (keys[i] != null) {
-
         // The key we want is in the hash-table!
         if (keys[i].equals(key)) {
-
           // If j != -1 this means we previously encountered a deleted cell.
           // We can perform an optimization by swapping the entries in cells
           // i and j so that the next time we search for this key it will be
@@ -285,15 +275,12 @@ public V get(K key) {
       // Ignore deleted cells, but record where the first index
       // of a deleted cell is found to perform lazy relocation later.
       if (keys[i] == TOMBSTONE) {
-
         if (j == -1) j = i;
 
         // We hit a non-null key, perhaps it's the one we're looking for.
       } else if (keys[i] != null) {
-
         // The key we want is in the hash-table!
         if (keys[i].equals(key)) {
-
           // If j != -1 this means we previously encountered a deleted cell.
           // We can perform an optimization by swapping the entries in cells
           // i and j so that the next time we search for this key it will be
@@ -350,12 +337,16 @@ public V remove(K key) {
   @Override
   public String toString() {
     StringBuilder sb = new StringBuilder();
-
     sb.append("{");
-    for (int i = 0; i < capacity; i++)
-      if (keys[i] != null && keys[i] != TOMBSTONE) sb.append(keys[i] + " => " + values[i] + ", ");
+    boolean first = true;
+    for (int i = 0; i < capacity; i++) {
+      if (keys[i] != null && keys[i] != TOMBSTONE) {
+        if (!first) sb.append(", ");
+        sb.append(keys[i]).append(" => ").append(values[i]);
+        first = false;
+      }
+    }
     sb.append("}");
-
     return sb.toString();
   }
 
diff --git a/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChaining.java b/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChaining.java
index 5660a3a52..3de53deb6 100644
--- a/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChaining.java
+++ b/src/main/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChaining.java
@@ -19,9 +19,16 @@ public Entry(K key, V value) {
     this.hash = key.hashCode();
   }
 
-  // We are not overriding the Object equals method
-  // No casting is required with this method.
-  public boolean equals(Entry<K, V> other) {
+  @Override
+  public int hashCode() {
+    return hash;
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (this == obj) return true;
+    if (obj == null || getClass() != obj.getClass()) return false;
+    Entry<?, ?> other = (Entry<?, ?>) obj;
     if (hash != other.hash) return false;
     return key.equals(other.key);
   }
@@ -103,7 +110,6 @@ public V add(K key, V value) {
   }
 
   public V insert(K key, V value) {
-
     if (key == null) throw new IllegalArgumentException("Null key");
     Entry<K, V> newEntry = new Entry<>(key, value);
     int bucketIndex = normalizeIndex(newEntry.hash);
@@ -114,7 +120,6 @@ public V insert(K key, V value) {
   // NOTE: returns null if the value is null AND also returns
   // null if the key does not exists, so watch out..
   public V get(K key) {
-
     if (key == null) return null;
     int bucketIndex = normalizeIndex(key.hashCode());
     Entry<K, V> entry = bucketSeekEntry(bucketIndex, key);
@@ -126,7 +131,6 @@ public V get(K key) {
   // NOTE: returns null if the value is null AND also returns
   // null if the key does not exists.
   public V remove(K key) {
-
     if (key == null) return null;
     int bucketIndex = normalizeIndex(key.hashCode());
     return bucketRemoveEntry(bucketIndex, key);
@@ -134,7 +138,6 @@ public V remove(K key) {
 
   // Removes an entry from a given bucket if it exists
   private V bucketRemoveEntry(int bucketIndex, K key) {
-
     Entry<K, V> entry = bucketSeekEntry(bucketIndex, key);
     if (entry != null) {
       LinkedList<Entry<K, V>> links = table[bucketIndex];
@@ -147,7 +150,6 @@ private V bucketRemoveEntry(int bucketIndex, K key) {
   // Inserts an entry in a given bucket only if the entry does not already
   // exist in the given bucket, but if it does then update the entry value
   private V bucketInsertEntry(int bucketIndex, Entry<K, V> entry) {
-
     LinkedList<Entry<K, V>> bucket = table[bucketIndex];
     if (bucket == null) table[bucketIndex] = bucket = new LinkedList<>();
 
@@ -165,7 +167,6 @@ private V bucketInsertEntry(int bucketIndex, Entry<K, V> entry) {
 
   // Finds and returns a particular entry in a given bucket if it exists, returns null otherwise
   private Entry<K, V> bucketSeekEntry(int bucketIndex, K key) {
-
     if (key == null) return null;
     LinkedList<Entry<K, V>> bucket = table[bucketIndex];
     if (bucket == null) return null;
@@ -175,7 +176,6 @@ private Entry<K, V> bucketSeekEntry(int bucketIndex, K key) {
 
   // Resizes the internal table holding buckets of entries
   private void resizeTable() {
-
     capacity *= 2;
     threshold = (int) (capacity * maxLoadFactor);
 
@@ -183,7 +183,6 @@ private void resizeTable() {
 
     for (int i = 0; i < table.length; i++) {
       if (table[i] != null) {
-
         for (Entry<K, V> entry : table[i]) {
           int bucketIndex = normalizeIndex(entry.hash);
           LinkedList<Entry<K, V>> bucket = newTable[bucketIndex];
@@ -196,13 +195,11 @@ private void resizeTable() {
         table[i] = null;
       }
     }
-
     table = newTable;
   }
 
   // Returns the list of keys found within the hash table
   public List<K> keys() {
-
     List<K> keys = new ArrayList<>(size());
     for (LinkedList<Entry<K, V>> bucket : table)
       if (bucket != null) for (Entry<K, V> entry : bucket) keys.add(entry.key);
@@ -211,7 +208,6 @@ public List<K> keys() {
 
   // Returns the list of values found within the hash table
   public List<V> values() {
-
     List<V> values = new ArrayList<>(size());
     for (LinkedList<Entry<K, V>> bucket : table)
       if (bucket != null) for (Entry<K, V> entry : bucket) values.add(entry.value);
@@ -221,25 +217,21 @@ public List<V> values() {
   // Return an iterator to iterate over all the keys in this map
   @Override
   public java.util.Iterator<K> iterator() {
-    final int elementCount = size();
+    final int MODIFICATION_COUNT = size; // Using size as a proxy for modifications for now, but better to have a dedicated counter
     return new java.util.Iterator<K>() {
-
       int bucketIndex = 0;
       java.util.Iterator<Entry<K, V>> bucketIter = (table[0] == null) ? null : table[0].iterator();
 
       @Override
       public boolean hasNext() {
-
         // An item was added or removed while iterating
-        if (elementCount != size) throw new java.util.ConcurrentModificationException();
+        if (MODIFICATION_COUNT != size) throw new java.util.ConcurrentModificationException();
 
         // No iterator or the current iterator is empty
         if (bucketIter == null || !bucketIter.hasNext()) {
-
           // Search next buckets until a valid iterator is found
           while (++bucketIndex < capacity) {
             if (table[bucketIndex] != null) {
-
               // Make sure this iterator actually has elements -_-
               java.util.Iterator<Entry<K, V>> nextIter = table[bucketIndex].iterator();
               if (nextIter.hasNext()) {
@@ -254,6 +246,7 @@ public boolean hasNext() {
 
       @Override
       public K next() {
+        if (!hasNext()) throw new NoSuchElementException();
         return bucketIter.next().key;
       }
 
@@ -267,12 +260,16 @@ public void remove() {
   // Returns a string representation of this hash table
   @Override
   public String toString() {
-
     StringBuilder sb = new StringBuilder();
     sb.append("{");
+    boolean first = true;
     for (int i = 0; i < capacity; i++) {
       if (table[i] == null) continue;
-      for (Entry<K, V> entry : table[i]) sb.append(entry + ", ");
+      for (Entry<K, V> entry : table[i]) {
+        if (!first) sb.append(", ");
+        sb.append(entry);
+        first = false;
+      }
     }
     sb.append("}");
     return sb.toString();
diff --git a/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/DoubleHashingTestObject.java b/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/DoubleHashingTestObject.java
deleted file mode 100644
index a65c6c85d..000000000
--- a/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/DoubleHashingTestObject.java
+++ /dev/null
@@ -1,100 +0,0 @@
-/**
- * The DoubleHashingTestObject is an object to test the functionality of the hash-table implemented
- * with double hashing.
- *
- * @author William Fiset, william.alexandre.fiset@gmail.com
- */
-package com.williamfiset.algorithms.datastructures.hashtable;
-
-import java.util.Random;
-
-public class DoubleHashingTestObject implements SecondaryHash {
-
-  private int hash, hash2;
-  Integer intData = null;
-  int[] vectorData = null;
-  String stringData = null;
-
-  static long[] randomVector;
-  static Random R = new Random();
-  static int MAX_VECTOR_SIZE = 100000;
-
-  static {
-    randomVector = new long[MAX_VECTOR_SIZE];
-    for (int i = 0; i < MAX_VECTOR_SIZE; i++) {
-      long val = R.nextLong();
-      while (val % 2 == 0) val = R.nextLong();
-      randomVector[i] = val;
-    }
-  }
-
-  public DoubleHashingTestObject(int data) {
-    intData = data;
-    intHash();
-    computeHash();
-  }
-
-  public DoubleHashingTestObject(int[] data) {
-    if (data == null) throw new IllegalArgumentException("Cannot be null");
-    vectorData = data;
-    vectorHash();
-    computeHash();
-  }
-
-  public DoubleHashingTestObject(String data) {
-    if (data == null) throw new IllegalArgumentException("Cannot be null");
-    stringData = data;
-    stringHash();
-    computeHash();
-  }
-
-  private void intHash() {
-    hash2 = intData;
-  }
-
-  private void vectorHash() {
-    for (int i = 0; i < vectorData.length; i++) hash2 += randomVector[i] * vectorData[i];
-  }
-
-  private void stringHash() {
-
-    // Multipicative hash function:
-    final int INITIAL_VALUE = 0;
-    int prime = 17;
-    int power = 1;
-    hash = INITIAL_VALUE;
-    for (int i = 0; i < stringData.length(); i++) {
-      int ch = stringData.charAt(i);
-      hash2 += power * ch;
-      power *= prime;
-    }
-  }
-
-  private void computeHash() {
-    if (intData != null) hash = intData.hashCode();
-    else if (stringData != null) hash = stringData.hashCode();
-    else hash = java.util.Arrays.hashCode(vectorData);
-  }
-
-  @Override
-  public int hashCode() {
-    return hash;
-  }
-
-  @Override
-  public int hashCode2() {
-    return hash2;
-  }
-
-  @Override
-  public boolean equals(Object o) {
-    if (this == o) return true;
-    else if (o instanceof DoubleHashingTestObject) {
-      DoubleHashingTestObject obj = (DoubleHashingTestObject) o;
-      if (hash != obj.hash) return false;
-      if (intData != null) return intData.equals(obj.intData);
-      if (vectorData != null) return java.util.Arrays.equals(vectorData, obj.vectorData);
-      return stringData.equals(obj.stringData);
-    } else return false;
-  }
-}
diff --git a/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableDoubleHashingTest.java b/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableDoubleHashingTest.java
index 982658a61..9db93d8bb 100644
--- a/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableDoubleHashingTest.java
+++ b/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableDoubleHashingTest.java
@@ -47,6 +47,37 @@ public void testLegalCreation() {
     assertDoesNotThrow(() -> new HashTableDoubleHashing<>(6, 0.9));
   }
 
+  @Test
+  public void testKeys() {
+    map.put(new DoubleHashingTestObject(1), 10);
+    map.put(new DoubleHashingTestObject(2), 20);
+    map.put(new DoubleHashingTestObject(3), 30);
+    List<DoubleHashingTestObject> keys = map.keys();
+    assertThat(keys)
+        .containsExactly(
+            new DoubleHashingTestObject(1),
+            new DoubleHashingTestObject(2),
+            new DoubleHashingTestObject(3));
+  }
+
+  @Test
+  public void testValues() {
+    map.put(new DoubleHashingTestObject(1), 10);
+    map.put(new DoubleHashingTestObject(2), 20);
+    map.put(new DoubleHashingTestObject(3), 30);
+    List<Integer> values = map.values();
+    assertThat(values).containsExactly(10, 20, 30);
+  }
+
+  @Test
+  public void testToString() {
+    map.put(new DoubleHashingTestObject(1), 10);
+    String s = map.toString();
+    assertThat(s).startsWith("{");
+    assertThat(s).endsWith("}");
+    assertThat(s).contains(" => 10");
+  }
+
   @Test
   public void testUpdatingValue() {
     // System.out.println("testUpdatingValue");
diff --git a/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableLinearProbingTest.java b/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableLinearProbingTest.java
index f18806fd0..d22193603 100644
--- a/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableLinearProbingTest.java
+++ b/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableLinearProbingTest.java
@@ -78,16 +78,66 @@ public void testLegalCreation() {
   }
 
   @Test
-  public void testUpdatingValue() {
+  public void testKeys() {
+    map.put(1, 10);
+    map.put(2, 20);
+    map.put(3, 30);
+    List<Integer> keys = map.keys();
+    assertThat(keys).containsExactly(1, 2, 3);
+  }
+
+  @Test
+  public void testValues() {
+    map.put(1, 10);
+    map.put(2, 20);
+    map.put(3, 30);
+    List<Integer> values = map.values();
+    assertThat(values).containsExactly(10, 20, 30);
+  }
 
-    map.add(1, 1);
-    assertThat(map.get(1)).isEqualTo(1);
+  @Test
+  public void testToString() {
+    map.put(1, 10);
+    map.put(2, 20);
+    String s = map.toString();
+    assertThat(s).contains("1 => 10");
+    assertThat(s).contains("2 => 20");
+    assertThat(s).startsWith("{");
+    assertThat(s).endsWith("}");
+  }
 
-    map.add(1, 5);
-    assertThat(map.get(1)).isEqualTo(5);
+  @Test
+  public void testLazyRelocation() {
+    // Probing sequence for keys with same hash
+    // We can use HashObject to force collisions
+    HashTableLinearProbing<HashObject, Integer> collisionMap = new HashTableLinearProbing<>(10);
+    HashObject o1 = new HashObject(5, 1);
+    HashObject o2 = new HashObject(5, 2);
+    HashObject o3 = new HashObject(5, 3);
+
+    collisionMap.put(o1, 100);
+    collisionMap.put(o2, 200);
+    collisionMap.put(o3, 300);
+
+    // Remove o2, it leaves a TOMBSTONE
+    collisionMap.remove(o2);
+    assertThat(collisionMap.containsKey(o3)).isTrue(); // Should trigger lazy relocation
+    assertThat(collisionMap.get(o3)).isEqualTo(300);
+  }
 
-    map.add(1, -7);
-    assertThat(map.get(1)).isEqualTo(-7);
+  @Test
+  public void testGetNullKey() {
+    assertThrows(IllegalArgumentException.class, () -> map.get(null));
+  }
+
+  @Test
+  public void testHasKeyNullKey() {
+    assertThrows(IllegalArgumentException.class, () -> map.hasKey(null));
+  }
+
+  @Test
+  public void testRemoveNullKey() {
+    assertThrows(IllegalArgumentException.class, () -> map.remove(null));
   }
 
   @Test
diff --git a/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableQuadraticProbingTest.java b/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableQuadraticProbingTest.java
index 13c1daad5..0c88a29fc 100644
--- a/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableQuadraticProbingTest.java
+++ b/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableQuadraticProbingTest.java
@@ -76,6 +76,33 @@ public void testLegalCreation() {
     assertDoesNotThrow(() -> new HashTableQuadraticProbing<>(6, 0.9));
   }
 
+  @Test
+  public void testKeys() {
+    map.put(1, 10);
+    map.put(2, 20);
+    map.put(3, 30);
+    List<Integer> keys = map.keys();
+    assertThat(keys).containsExactly(1, 2, 3);
+  }
+
+  @Test
+  public void testValues() {
+    map.put(1, 10);
+    map.put(2, 20);
+    map.put(3, 30);
+    List<Integer> values = map.values();
+    assertThat(values).containsExactly(10, 20, 30);
+  }
+
+  @Test
+  public void testToString() {
+    map.put(1, 10);
+    String s = map.toString();
+    assertThat(s).startsWith("{");
+    assertThat(s).endsWith("}");
+    assertThat(s).contains("1 => 10");
+  }
+
   @Test
   public void testUpdatingValue() {
 
diff --git a/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChainingTest.java b/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChainingTest.java
index 84518b394..a943f95e1 100644
--- a/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChainingTest.java
+++ b/src/test/java/com/williamfiset/algorithms/datastructures/hashtable/HashTableSeparateChainingTest.java
@@ -72,16 +72,53 @@ public void testLegalCreation() {
   }
 
   @Test
-  public void testUpdatingValue() {
+  public void testKeys() {
+    map.put(1, 10);
+    map.put(2, 20);
+    map.put(3, 30);
+    List<Integer> keys = map.keys();
+    assertThat(keys).containsExactly(1, 2, 3);
+  }
 
-    map.add(1, 1);
-    assertThat(map.get(1)).isEqualTo(1);
+  @Test
+  public void testValues() {
+    map.put(1, 10);
+    map.put(2, 20);
+    map.put(3, 30);
+    List<Integer> values = map.values();
+    assertThat(values).containsExactly(10, 20, 30);
+  }
 
-    map.add(1, 5);
-    assertThat(map.get(1)).isEqualTo(5);
+  @Test
+  public void testToString() {
+    map.put(1, 10);
+    map.put(2, 20);
+    String s = map.toString();
+    // Order might vary, but should contain both entries
+    assertThat(s).contains("1 => 10");
+    assertThat(s).contains("2 => 20");
+    assertThat(s).startsWith("{");
+    assertThat(s).endsWith("}");
+  }
 
-    map.add(1, -7);
-    assertThat(map.get(1)).isEqualTo(-7);
+  @Test
+  public void testClear() {
+    map.put(1, 10);
+    map.put(2, 20);
+    map.clear();
+    assertThat(map.size()).isEqualTo(0);
+    assertThat(map.isEmpty()).isTrue();
+    assertThat(map.get(1)).isNull();
+  }
+
+  @Test
+  public void testGetNullKey() {
+    assertThat(map.get(null)).isNull();
+  }
+
+  @Test
+  public void testRemoveNullKey() {
+    assertThat(map.remove(null)).isNull();
   }
 
   @Test

From d5dc0b85f8ef5ee6d35526de57146349e38a4d6f Mon Sep 17 00:00:00 2001
From: William Fiset <fisetwill@gmail.com>
Date: Fri, 6 Mar 2026 18:57:25 -0800
Subject: [PATCH 6/7] Refactor BinaryHeap: improve naming, robustness, and test
 coverage (#1257) (#1261)

- Standardize variable naming to camelCase (removedData, elemI, elemJ)
- Add null checks in constructors and add/contains/remove methods
- Expand test suite to cover:
  - Empty heap peek/poll behavior
  - Null input handling for all public methods
  - Constructor validation for arrays and collections with null elements
  - toString representation
- Preserve all educational comments
---
 .../priorityqueue/BinaryHeap.java             | 33 +++++-----
 .../priorityqueue/BinaryHeapTest.java         | 61 +++++++++++++++++++
 2 files changed, 80 insertions(+), 14 deletions(-)

diff --git a/src/main/java/com/williamfiset/algorithms/datastructures/priorityqueue/BinaryHeap.java b/src/main/java/com/williamfiset/algorithms/datastructures/priorityqueue/BinaryHeap.java
index 9d6789d1b..bddae09e2 100644
--- a/src/main/java/com/williamfiset/algorithms/datastructures/priorityqueue/BinaryHeap.java
+++ b/src/main/java/com/williamfiset/algorithms/datastructures/priorityqueue/BinaryHeap.java
@@ -27,12 +27,15 @@ public BinaryHeap(int sz) {
   // Construct a priority queue using heapify in O(n) time, a great explanation can be found at:
   // http://www.cs.umd.edu/~meesh/351/mount/lectures/lect14-heapsort-analysis-part.pdf
   public BinaryHeap(T[] elems) {
-
+    if (elems == null) throw new IllegalArgumentException("Input array cannot be null");
     int heapSize = elems.length;
     heap = new ArrayList<T>(heapSize);
 
     // Place all element in heap
-    for (int i = 0; i < heapSize; i++) heap.add(elems[i]);
+    for (int i = 0; i < heapSize; i++) {
+      if (elems[i] == null) throw new IllegalArgumentException("Elements cannot be null");
+      heap.add(elems[i]);
+    }
 
     // Heapify process, O(n)
     for (int i = Math.max(0, (heapSize / 2) - 1); i >= 0; i--) sink(i);
@@ -40,12 +43,15 @@ public BinaryHeap(T[] elems) {
 
   // Priority queue construction, O(n)
   public BinaryHeap(Collection<T> elems) {
-
+    if (elems == null) throw new IllegalArgumentException("Input collection cannot be null");
     int heapSize = elems.size();
     heap = new ArrayList<T>(heapSize);
 
     // Add all elements of the given collection to the heap
-    heap.addAll(elems);
+    for (T elem : elems) {
+      if (elem == null) throw new IllegalArgumentException("Elements cannot be null");
+      heap.add(elem);
+    }
 
     // Heapify process, O(n)
     for (int i = Math.max(0, (heapSize / 2) - 1); i >= 0; i--) sink(i);
@@ -81,6 +87,7 @@ public T poll() {
 
   // Test if an element is in heap, O(n)
   public boolean contains(T elem) {
+    if (elem == null) return false;
     // Linear scan to check containment
     for (int i = 0; i < size(); i++) if (heap.get(i).equals(elem)) return true;
     return false;
@@ -89,8 +96,7 @@ public boolean contains(T elem) {
   // Adds an element to the priority queue, the
   // element must not be null, O(log(n))
   public void add(T elem) {
-
-    if (elem == null) throw new IllegalArgumentException();
+    if (elem == null) throw new IllegalArgumentException("Cannot add null element");
 
     heap.add(elem);
 
@@ -108,7 +114,6 @@ private boolean less(int i, int j) {
 
   // Perform bottom up node swim, O(log(n))
   private void swim(int k) {
-
     // Grab the index of the next parent node WRT to k
     int parent = (k - 1) / 2;
 
@@ -148,11 +153,11 @@ private void sink(int k) {
 
   // Swap two nodes. Assumes i & j are valid, O(1)
   private void swap(int i, int j) {
-    T elem_i = heap.get(i);
-    T elem_j = heap.get(j);
+    T elemI = heap.get(i);
+    T elemJ = heap.get(j);
 
-    heap.set(i, elem_j);
-    heap.set(j, elem_i);
+    heap.set(i, elemJ);
+    heap.set(j, elemI);
   }
 
   // Removes a particular element in the heap, O(n)
@@ -173,14 +178,14 @@ private T removeAt(int i) {
     if (isEmpty()) return null;
 
     int indexOfLastElem = size() - 1;
-    T removed_data = heap.get(i);
+    T removedData = heap.get(i);
     swap(i, indexOfLastElem);
 
     // Obliterate the value
     heap.remove(indexOfLastElem);
 
     // Check if the last element was removed
-    if (i == indexOfLastElem) return removed_data;
+    if (i == indexOfLastElem) return removedData;
     T elem = heap.get(i);
 
     // Try sinking element
@@ -188,7 +193,7 @@ private T removeAt(int i) {
 
     // If sinking did not work try swimming
     if (heap.get(i).equals(elem)) swim(i);
-    return removed_data;
+    return removedData;
   }
 
   // Recursively checks if this heap is a min heap
diff --git a/src/test/java/com/williamfiset/algorithms/datastructures/priorityqueue/BinaryHeapTest.java b/src/test/java/com/williamfiset/algorithms/datastructures/priorityqueue/BinaryHeapTest.java
index 1f74e578b..60a5a34ff 100644
--- a/src/test/java/com/williamfiset/algorithms/datastructures/priorityqueue/BinaryHeapTest.java
+++ b/src/test/java/com/williamfiset/algorithms/datastructures/priorityqueue/BinaryHeapTest.java
@@ -25,6 +25,67 @@ public void testEmpty() {
     assertThat(q.peek()).isNull();
   }
 
+  @Test
+  public void testPeekPollEmpty() {
+    BinaryHeap<Integer> pq = new BinaryHeap<>();
+    assertThat(pq.peek()).isNull();
+    assertThat(pq.poll()).isNull();
+  }
+
+  @Test
+  public void testAddNull() {
+    BinaryHeap<Integer> pq = new BinaryHeap<>();
+    Assertions.assertThrows(IllegalArgumentException.class, () -> pq.add(null));
+  }
+
+  @Test
+  public void testHeapifyNullArray() {
+    Assertions.assertThrows(IllegalArgumentException.class, () -> new BinaryHeap<Integer>((Integer[]) null));
+  }
+
+  @Test
+  public void testHeapifyArrayWithNullElement() {
+    Integer[] array = {1, null, 3};
+    Assertions.assertThrows(IllegalArgumentException.class, () -> new BinaryHeap<Integer>(array));
+  }
+
+  @Test
+  public void testHeapifyNullCollection() {
+    Assertions.assertThrows(IllegalArgumentException.class, () -> new BinaryHeap<Integer>((java.util.Collection<Integer>) null));
+  }
+
+  @Test
+  public void testHeapifyCollectionWithNullElement() {
+    List<Integer> list = new ArrayList<>();
+    list.add(1);
+    list.add(null);
+    Assertions.assertThrows(IllegalArgumentException.class, () -> new BinaryHeap<Integer>(list));
+  }
+
+  @Test
+  public void testContainsNull() {
+    BinaryHeap<Integer> pq = new BinaryHeap<>();
+    pq.add(1);
+    assertThat(pq.contains(null)).isFalse();
+  }
+
+  @Test
+  public void testRemoveNull() {
+    BinaryHeap<Integer> pq = new BinaryHeap<>();
+    pq.add(1);
+    assertThat(pq.remove(null)).isFalse();
+    assertThat(pq.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testToString() {
+    BinaryHeap<Integer> pq = new BinaryHeap<>();
+    pq.add(1);
+    pq.add(2);
+    assertThat(pq.toString()).contains("1");
+    assertThat(pq.toString()).contains("2");
+  }
+
   @Test
   public void testHeapProperty() {
 

From be9ea81123c0e3da76819ee7310cfc4d94d6f283 Mon Sep 17 00:00:00 2001
From: William Fiset <fisetwill@gmail.com>
Date: Fri, 6 Mar 2026 19:02:01 -0800
Subject: [PATCH 7/7] Cleanup DoublyLinkedList: fix API, simplify code, expand
 tests (#1260)

* Cleanup DoublyLinkedList and expand test coverage

- Add proper import for java.util.Iterator instead of inline qualified names
- Change addAt to throw IndexOutOfBoundsException instead of checked Exception
- Remove pointless local variable assignments in clear() and remove(Node)
- Move trav declaration to point of use in remove(Object)
- Replace inline java.util imports with proper imports in tests
- Expand test suite from 18 to 30 tests covering contains, indexOf,
  null handling, invalid indices, iterator, and forEach

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Disallow null elements in DoublyLinkedList

- Reject null in addFirst/addLast with IllegalArgumentException
- Simplify remove(Object) and indexOf(Object) by removing null branches
- Replace null-related tests with testNullRejection
- Remove null entries from randomized test data generators

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
---
 .../linkedlist/DoublyLinkedList.java          |  55 ++-----
 .../linkedlist/LinkedListTest.java            | 155 +++++++++++++++---
 2 files changed, 149 insertions(+), 61 deletions(-)

diff --git a/src/main/java/com/williamfiset/algorithms/datastructures/linkedlist/DoublyLinkedList.java b/src/main/java/com/williamfiset/algorithms/datastructures/linkedlist/DoublyLinkedList.java
index e8074bdef..3d5f02f2c 100644
--- a/src/main/java/com/williamfiset/algorithms/datastructures/linkedlist/DoublyLinkedList.java
+++ b/src/main/java/com/williamfiset/algorithms/datastructures/linkedlist/DoublyLinkedList.java
@@ -5,6 +5,8 @@
  */
 package com.williamfiset.algorithms.datastructures.linkedlist;
 
+import java.util.Iterator;
+
 public class DoublyLinkedList<T> implements Iterable<T> {
   private int size = 0;
   private Node<T> head = null;
@@ -36,7 +38,7 @@ public void clear() {
       trav.data = null;
       trav = next;
     }
-    head = tail = trav = null;
+    head = tail = null;
     size = 0;
   }
 
@@ -57,6 +59,7 @@ public void add(T elem) {
 
   // Add a node to the tail of the linked list, O(1)
   public void addLast(T elem) {
+    if (elem == null) throw new IllegalArgumentException("Null elements are not allowed");
     if (isEmpty()) {
       head = tail = new Node<T>(elem, null, null);
     } else {
@@ -68,6 +71,7 @@ public void addLast(T elem) {
 
   // Add an element to the beginning of this linked list, O(1)
   public void addFirst(T elem) {
+    if (elem == null) throw new IllegalArgumentException("Null elements are not allowed");
     if (isEmpty()) {
       head = tail = new Node<T>(elem, null, null);
     } else {
@@ -78,9 +82,9 @@ public void addFirst(T elem) {
   }
 
   // Add an element at a specified index
-  public void addAt(int index, T data) throws Exception {
+  public void addAt(int index, T data) {
     if (index < 0 || index > size) {
-      throw new Exception("Illegal Index");
+      throw new IndexOutOfBoundsException("Illegal index: " + index);
     }
     if (index == 0) {
       addFirst(data);
@@ -173,7 +177,7 @@ private T remove(Node<T> node) {
 
     // Memory cleanup
     node.data = null;
-    node = node.prev = node.next = null;
+    node.prev = node.next = null;
 
     --size;
 
@@ -207,23 +211,10 @@ public T removeAt(int index) {
 
   // Remove a particular value in the linked list, O(n)
   public boolean remove(Object obj) {
-    Node<T> trav = head;
-
-    // Support searching for null
-    if (obj == null) {
-      for (trav = head; trav != null; trav = trav.next) {
-        if (trav.data == null) {
-          remove(trav);
-          return true;
-        }
-      }
-      // Search for non null object
-    } else {
-      for (trav = head; trav != null; trav = trav.next) {
-        if (obj.equals(trav.data)) {
-          remove(trav);
-          return true;
-        }
+    for (Node<T> trav = head; trav != null; trav = trav.next) {
+      if (obj.equals(trav.data)) {
+        remove(trav);
+        return true;
       }
     }
     return false;
@@ -232,21 +223,9 @@ public boolean remove(Object obj) {
   // Find the index of a particular value in the linked list, O(n)
   public int indexOf(Object obj) {
     int index = 0;
-    Node<T> trav = head;
-
-    // Support searching for null
-    if (obj == null) {
-      for (; trav != null; trav = trav.next, index++) {
-        if (trav.data == null) {
-          return index;
-        }
-      }
-      // Search for non null object
-    } else {
-      for (; trav != null; trav = trav.next, index++) {
-        if (obj.equals(trav.data)) {
-          return index;
-        }
+    for (Node<T> trav = head; trav != null; trav = trav.next, index++) {
+      if (obj.equals(trav.data)) {
+        return index;
       }
     }
     return -1;
@@ -258,8 +237,8 @@ public boolean contains(Object obj) {
   }
 
   @Override
-  public java.util.Iterator<T> iterator() {
-    return new java.util.Iterator<T>() {
+  public Iterator<T> iterator() {
+    return new Iterator<T>() {
       private Node<T> trav = head;
 
       @Override
diff --git a/src/test/java/com/williamfiset/algorithms/datastructures/linkedlist/LinkedListTest.java b/src/test/java/com/williamfiset/algorithms/datastructures/linkedlist/LinkedListTest.java
index c921e6ca3..7ff785f38 100644
--- a/src/test/java/com/williamfiset/algorithms/datastructures/linkedlist/LinkedListTest.java
+++ b/src/test/java/com/williamfiset/algorithms/datastructures/linkedlist/LinkedListTest.java
@@ -5,13 +5,14 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Iterator;
+import java.util.LinkedList;
 import java.util.List;
 import org.junit.jupiter.api.*;
 
 public class LinkedListTest {
   private static final int LOOPS = 10000;
   private static final int TEST_SZ = 40;
-  private static final int NUM_NULLS = TEST_SZ / 5;
   private static final int MAX_RAND_NUM = 250;
 
   DoublyLinkedList<Integer> list;
@@ -27,44 +28,65 @@ public void testEmptyList() {
     assertThat(list.size()).isEqualTo(0);
   }
 
+  @Test
+  public void testNullRejection() {
+    assertThrows(IllegalArgumentException.class, () -> list.add(null));
+    assertThrows(IllegalArgumentException.class, () -> list.addFirst(null));
+    assertThrows(IllegalArgumentException.class, () -> list.addLast(null));
+  }
+
   @Test
   public void testRemoveFirstOfEmpty() {
-    assertThrows(Exception.class, () -> list.removeFirst());
+    assertThrows(RuntimeException.class, () -> list.removeFirst());
   }
 
   @Test
   public void testRemoveLastOfEmpty() {
-    assertThrows(Exception.class, () -> list.removeLast());
+    assertThrows(RuntimeException.class, () -> list.removeLast());
   }
 
   @Test
   public void testPeekFirstOfEmpty() {
-    assertThrows(Exception.class, () -> list.peekFirst());
+    assertThrows(RuntimeException.class, () -> list.peekFirst());
   }
 
   @Test
   public void testPeekLastOfEmpty() {
-    assertThrows(Exception.class, () -> list.peekLast());
+    assertThrows(RuntimeException.class, () -> list.peekLast());
   }
 
   @Test
   public void testAddFirst() {
     list.addFirst(3);
     assertThat(list.size()).isEqualTo(1);
+    assertThat(list.peekFirst()).isEqualTo(3);
     list.addFirst(5);
     assertThat(list.size()).isEqualTo(2);
+    assertThat(list.peekFirst()).isEqualTo(5);
   }
 
   @Test
   public void testAddLast() {
     list.addLast(3);
     assertThat(list.size()).isEqualTo(1);
+    assertThat(list.peekLast()).isEqualTo(3);
     list.addLast(5);
     assertThat(list.size()).isEqualTo(2);
+    assertThat(list.peekLast()).isEqualTo(5);
   }
 
   @Test
-  public void testAddAt() throws Exception {
+  public void testAdd() {
+    list.add(1);
+    list.add(2);
+    list.add(3);
+    assertThat(list.size()).isEqualTo(3);
+    assertThat(list.peekFirst()).isEqualTo(1);
+    assertThat(list.peekLast()).isEqualTo(3);
+  }
+
+  @Test
+  public void testAddAt() {
     list.addAt(0, 1);
     assertThat(list.size()).isEqualTo(1);
     list.addAt(1, 2);
@@ -77,6 +99,25 @@ public void testAddAt() throws Exception {
     assertThat(list.size()).isEqualTo(5);
   }
 
+  @Test
+  public void testAddAtValues() {
+    // Insert at various positions and verify order
+    list.addAt(0, 10);   // [10]
+    list.addAt(0, 20);   // [20, 10]
+    list.addAt(2, 30);   // [20, 10, 30]
+    list.addAt(1, 40);   // [20, 40, 10, 30]
+    assertThat(list.peekFirst()).isEqualTo(20);
+    assertThat(list.removeAt(1)).isEqualTo(40);
+    assertThat(list.removeAt(1)).isEqualTo(10);
+    assertThat(list.peekLast()).isEqualTo(30);
+  }
+
+  @Test
+  public void testAddAtInvalidIndex() {
+    assertThrows(IndexOutOfBoundsException.class, () -> list.addAt(-1, 1));
+    assertThrows(IndexOutOfBoundsException.class, () -> list.addAt(1, 1));
+  }
+
   @Test
   public void testRemoveFirst() {
     list.addFirst(3);
@@ -161,6 +202,15 @@ public void testRemoving() {
     assertThat(strs.size()).isEqualTo(0);
   }
 
+  @Test
+  public void testRemoveNonExistent() {
+    list.add(1);
+    list.add(2);
+    list.add(3);
+    assertThat(list.remove((Object) 99)).isFalse();
+    assertThat(list.size()).isEqualTo(3);
+  }
+
   @Test
   public void testRemoveAt() {
     list.add(1);
@@ -176,6 +226,34 @@ public void testRemoveAt() {
     assertThat(list.size()).isEqualTo(0);
   }
 
+  @Test
+  public void testRemoveAtInvalidIndex() {
+    list.add(1);
+    list.add(2);
+    assertThrows(IllegalArgumentException.class, () -> list.removeAt(-1));
+    assertThrows(IllegalArgumentException.class, () -> list.removeAt(2));
+  }
+
+  @Test
+  public void testContains() {
+    list.add(1);
+    list.add(2);
+    list.add(3);
+    assertThat(list.contains(2)).isTrue();
+    assertThat(list.contains(99)).isFalse();
+  }
+
+  @Test
+  public void testIndexOf() {
+    list.add(10);
+    list.add(20);
+    list.add(30);
+    assertThat(list.indexOf(10)).isEqualTo(0);
+    assertThat(list.indexOf(20)).isEqualTo(1);
+    assertThat(list.indexOf(30)).isEqualTo(2);
+    assertThat(list.indexOf(99)).isEqualTo(-1);
+  }
+
   @Test
   public void testClear() {
     list.add(22);
@@ -184,6 +262,7 @@ public void testClear() {
     assertThat(list.size()).isEqualTo(3);
     list.clear();
     assertThat(list.size()).isEqualTo(0);
+    assertThat(list.isEmpty()).isTrue();
     list.add(22);
     list.add(33);
     list.add(44);
@@ -192,9 +271,45 @@ public void testClear() {
     assertThat(list.size()).isEqualTo(0);
   }
 
+  @Test
+  public void testIterator() {
+    list.add(1);
+    list.add(2);
+    list.add(3);
+    Iterator<Integer> it = list.iterator();
+    assertThat(it.hasNext()).isTrue();
+    assertThat(it.next()).isEqualTo(1);
+    assertThat(it.next()).isEqualTo(2);
+    assertThat(it.next()).isEqualTo(3);
+    assertThat(it.hasNext()).isFalse();
+  }
+
+  @Test
+  public void testIteratorOnEmptyList() {
+    Iterator<Integer> it = list.iterator();
+    assertThat(it.hasNext()).isFalse();
+  }
+
+  @Test
+  public void testIteratorRemoveUnsupported() {
+    list.add(1);
+    Iterator<Integer> it = list.iterator();
+    assertThrows(UnsupportedOperationException.class, it::remove);
+  }
+
+  @Test
+  public void testForEach() {
+    list.add(10);
+    list.add(20);
+    list.add(30);
+    List<Integer> result = new ArrayList<>();
+    for (int v : list) result.add(v);
+    assertThat(result).containsExactly(10, 20, 30).inOrder();
+  }
+
   @Test
   public void testRandomizedRemoving() {
-    java.util.LinkedList<Integer> javaLinkedList = new java.util.LinkedList<>();
+    LinkedList<Integer> javaLinkedList = new LinkedList<>();
     for (int loops = 0; loops < LOOPS; loops++) {
 
       list.clear();
@@ -214,12 +329,8 @@ public void testRandomizedRemoving() {
         assertThat(javaLinkedList.remove(rm_val)).isEqualTo(list.remove(rm_val));
         assertThat(javaLinkedList.size()).isEqualTo(list.size());
 
-        java.util.Iterator<Integer> iter1 = javaLinkedList.iterator();
-        java.util.Iterator<Integer> iter2 = list.iterator();
-        while (iter1.hasNext()) assertThat(iter1.next()).isEqualTo(iter2.next());
-
-        iter1 = javaLinkedList.iterator();
-        iter2 = list.iterator();
+        Iterator<Integer> iter1 = javaLinkedList.iterator();
+        Iterator<Integer> iter2 = list.iterator();
         while (iter1.hasNext()) assertThat(iter1.next()).isEqualTo(iter2.next());
       }
 
@@ -238,8 +349,8 @@ public void testRandomizedRemoving() {
         assertThat(javaLinkedList.remove(rm_val)).isEqualTo(list.remove(rm_val));
         assertThat(javaLinkedList.size()).isEqualTo(list.size());
 
-        java.util.Iterator<Integer> iter1 = javaLinkedList.iterator();
-        java.util.Iterator<Integer> iter2 = list.iterator();
+        Iterator<Integer> iter1 = javaLinkedList.iterator();
+        Iterator<Integer> iter2 = list.iterator();
         while (iter1.hasNext()) assertThat(iter1.next()).isEqualTo(iter2.next());
       }
     }
@@ -247,7 +358,7 @@ public void testRandomizedRemoving() {
 
   @Test
   public void testRandomizedRemoveAt() {
-    java.util.LinkedList<Integer> javaLinkedList = new java.util.LinkedList<>();
+    LinkedList<Integer> javaLinkedList = new LinkedList<>();
 
     for (int loops = 0; loops < LOOPS; loops++) {
 
@@ -270,8 +381,8 @@ public void testRandomizedRemoveAt() {
         assertThat(num1).isEqualTo(num2);
         assertThat(javaLinkedList.size()).isEqualTo(list.size());
 
-        java.util.Iterator<Integer> iter1 = javaLinkedList.iterator();
-        java.util.Iterator<Integer> iter2 = list.iterator();
+        Iterator<Integer> iter1 = javaLinkedList.iterator();
+        Iterator<Integer> iter2 = list.iterator();
         while (iter1.hasNext()) assertThat(iter1.next()).isEqualTo(iter2.next());
       }
     }
@@ -279,7 +390,7 @@ public void testRandomizedRemoveAt() {
 
   @Test
   public void testRandomizedIndexOf() {
-    java.util.LinkedList<Integer> javaLinkedList = new java.util.LinkedList<>();
+    LinkedList<Integer> javaLinkedList = new LinkedList<>();
 
     for (int loops = 0; loops < LOOPS; loops++) {
 
@@ -303,8 +414,8 @@ public void testRandomizedIndexOf() {
         assertThat(index1).isEqualTo(index2);
         assertThat(javaLinkedList.size()).isEqualTo(list.size());
 
-        java.util.Iterator<Integer> iter1 = javaLinkedList.iterator();
-        java.util.Iterator<Integer> iter2 = list.iterator();
+        Iterator<Integer> iter1 = javaLinkedList.iterator();
+        Iterator<Integer> iter2 = list.iterator();
         while (iter1.hasNext()) assertThat(iter1.next()).isEqualTo(iter2.next());
       }
     }
@@ -329,7 +440,6 @@ public void testToString() {
   static List<Integer> genRandList(int sz) {
     List<Integer> lst = new ArrayList<>(sz);
     for (int i = 0; i < sz; i++) lst.add((int) (Math.random() * MAX_RAND_NUM));
-    for (int i = 0; i < NUM_NULLS; i++) lst.add(null);
     Collections.shuffle(lst);
     return lst;
   }
@@ -338,7 +448,6 @@ static List<Integer> genRandList(int sz) {
   static List<Integer> genUniqueRandList(int sz) {
     List<Integer> lst = new ArrayList<>(sz);
     for (int i = 0; i < sz; i++) lst.add(i);
-    for (int i = 0; i < NUM_NULLS; i++) lst.add(null);
     Collections.shuffle(lst);
     return lst;
   }