Harden binary child assignment

Fix set_child_at index errors and clean up parent/hash references
when replacing or clearing children. This preserves swap semantics and
avoids stale lookups.

Author: evolve75

lib/tree/binarytree.rb

@@ -203,12 +203,29 @@ def inordered_each
     #
     # @raise [ArgumentError] If the index is out of limits.
     def set_child_at(child, at_index)
-      raise ArgumentError 'A binary tree cannot have more than two children.'\
-                          unless (0..1).include? at_index
+      raise ArgumentError, 'A binary tree cannot have more than two children.'\
+                           unless (0..1).include? at_index
 
-      @children[at_index]        = child
-      @children_hash[child.name] = child if child # Assign the name mapping
-      child.parent               = self if child
+      old_child = @children[at_index]
+      if old_child && old_child != child
+        still_present = @children.each_with_index.any? do |existing, idx|
+          idx != at_index && existing.equal?(old_child)
+        end
+
+        unless still_present
+          @children_hash.delete(old_child.name)
+          old_child.set_as_root!
+        end
+      end
+
+      if child
+        child.parent&.remove!(child) unless child.parent == self
+        @children[at_index]        = child
+        @children_hash[child.name] = child # Assign the name mapping
+        child.parent               = self
+      else
+        @children[at_index] = nil
+      end
       child
     end
 

test/test_binarytree.rb

@@ -229,6 +229,9 @@ def test_left_child_equals
       assert_nil(@root.left_child, 'The left child should now be nil')
       assert_nil(@root.first_child, 'The first child is now nil')
       assert_equal('B Child at Right', @root.last_child.name, 'The last child should now be the right child')
+      assert(@left_child1.root?, 'The old left child should now be a root')
+      assert_nil(@left_child1.parent, 'The old left child should not have a parent')
+      assert_nil(@root['A Child at Left'], 'Lookup by old left name should be nil')
     end
 
     # Test right_child= method.
@@ -249,6 +252,15 @@ def test_right_child_equals
       assert_nil(@root.right_child, 'The right child should now be nil')
       assert_equal('A Child at Left', @root.first_child.name, 'The first child should now be the left child')
       assert_nil(@root.last_child, 'The first child is now nil')
+      assert(@right_child1.root?, 'The old right child should now be a root')
+      assert_nil(@right_child1.parent, 'The old right child should not have a parent')
+      assert_nil(@root['B Child at Right'], 'Lookup by old right name should be nil')
+    end
+
+    # Test invalid index error for set_child_at.
+    def test_set_child_at_invalid_index
+      error = assert_raise(ArgumentError) { @root.send(:set_child_at, @left_child1, 2) }
+      assert_match(/cannot have more than two children/i, error.message)
     end
 
     # Test isLeft_child? method.