Merge pull request #82 from PatrykOlejniczak/master

Fix RedBlackTree impl/test #81

Author: aalhour

DataStructures/Trees/BinarySearchTreeNode.cs

@@ -1,5 +1,4 @@
-
-namespace DataStructures.Trees
+namespace DataStructures.Trees
 {
     /// <summary>
     /// The binary search tree node.
@@ -61,6 +60,11 @@ public virtual bool HasLeftChild
             get { return (this.LeftChild != null); }
         }
 
+        /// <summary>
+        ///     Check if this node has only one child and whether it is the right child.
+        /// </summary>
+        public virtual bool HasOnlyRightChild => !this.HasLeftChild && this.HasRightChild;
+
         /// <summary>
         /// Checks whether this node has right child.
         /// </summary>
@@ -69,6 +73,11 @@ public virtual bool HasRightChild
             get { return (this.RightChild != null); }
         }
 
+        /// <summary>
+        ///     Check if this node has only one child and whether it is the left child.
+        /// </summary>
+        public virtual bool HasOnlyLeftChild => !this.HasRightChild && this.HasLeftChild;
+
         /// <summary>
         /// Checks whether this node is the left child of it's parent.
         /// </summary>

DataStructures/Trees/RedBlackTree.cs

@@ -1,8 +1,7 @@
-using System;
+using DataStructures.Common;
+using System;
 using System.Collections.Generic;
 
-using DataStructures.Common;
-
 namespace DataStructures.Trees
 {
     /// <summary>
@@ -14,7 +13,6 @@ public enum RedBlackTreeColors
         Black = 1
     };
 
-
     /// <summary>
     /// Red-Black Tree Data Structure.
     /// </summary>
@@ -29,6 +27,10 @@ public class RedBlackTree<TKey> : BinarySearchTree<TKey> where TKey : IComparabl
             internal set { base.Root = value; }
         }
 
+        private bool IsRoot(RedBlackTreeNode<TKey> node)
+        {
+            return node == this.Root;
+        }
 
         /// <summary>
         /// CONSTRUCTOR.
@@ -233,8 +235,8 @@ protected virtual void _adjustTreeAfterInsertion(RedBlackTreeNode<TKey> currentN
                 // This is the simplest step: Basically recolor, and bubble up to see if more work is needed.
                 if (_safeCheckIsRed(_safeGetSibling(currentNode.Parent)))
                 {
-                    // If it has a sibling and it is red, then then it has a parent
-                    currentNode.Parent.Color = RedBlackTreeColors.Red;
+                    // If it has a sibling and it is black, then then it has a parent
+                    currentNode.Parent.Color = RedBlackTreeColors.Black;
 
                     // Color sibling of parent as black
                     _safeUpdateColor(_safeGetSibling(currentNode.Parent), RedBlackTreeColors.Black);
@@ -455,80 +457,109 @@ protected override bool _remove(BSTNode<TKey> nodeToDelete)
         }
 
         /// <summary>
-        /// The internal remove helper.
-        /// Separated from the overriden version to avoid casting the objects from BSTNode to RedBlackTreeNode.
-        /// This is called from the overriden _remove(BSTNode nodeToDelete) helper.
+        ///     The internal remove helper.
+        ///     Separated from the overriden version to avoid casting the objects from BSTNode to RedBlackTreeNode.
+        ///     This is called from the overriden _remove(BSTNode nodeToDelete) helper.
         /// </summary>
         protected bool _remove(RedBlackTreeNode<TKey> nodeToDelete)
         {
             if (nodeToDelete == null)
-                return false;
-
-            // Temporary nodes
-            RedBlackTreeNode<TKey> node1, node2;
-
-            // If nodeToDelete has either one child or no children at all
-            if (!nodeToDelete.HasLeftChild || !nodeToDelete.HasRightChild)
             {
-                node1 = nodeToDelete;
-            }
-            else
-            {
-                // nodeToDelete has two children
-                node1 = (RedBlackTreeNode<TKey>)base._findNextLarger(nodeToDelete);
+                return false;
             }
 
-            // Left child case
-            if (node1.HasLeftChild)
+            if (IsRoot(nodeToDelete) && !nodeToDelete.HasChildren)
             {
-                node2 = node1.LeftChild;
+                Root = null;
             }
             else
             {
-                node2 = node1.RightChild;
-            }
-
-            // If node2 is not null, copy parent references
-            if (node2 != null)
-                node2.Parent = node1.Parent;
+                // X is the node we will replace with the nodeToDelete in the tree once we remove it.
+                RedBlackTreeNode<TKey> x;
 
-            if (node1.Parent != null)
-            {
-                if (node1.IsLeftChild)
+                if (!nodeToDelete.HasChildren)
                 {
-                    node1.Parent.LeftChild = node2;
+                    x = nodeToDelete;
+                    Transplant(nodeToDelete, null);
                 }
-                else
+                else if (nodeToDelete.HasOnlyRightChild)
                 {
-                    node1.Parent.RightChild = node2;
+                    x = nodeToDelete.RightChild;
+                    Transplant(nodeToDelete, nodeToDelete.RightChild);
                 }
-            }
-            else
-            {
-                Root = node2;
-                Root.Parent = null;
-            }
+                else if (nodeToDelete.HasOnlyLeftChild)
+                {
+                    x = nodeToDelete.LeftChild;
+                    Transplant(nodeToDelete, nodeToDelete.LeftChild);
+                }
+                else
+                {
+                    // Y is the node we will replace with the X in the tree once we move it to the nodeToDelete position.
+                    var y = (RedBlackTreeNode<TKey>)_findMinNode(nodeToDelete.RightChild);
+                    x = y.RightChild;
 
-            // Swap values
-            if (!node1.IsEqualTo(nodeToDelete))
-            {
-                nodeToDelete.Value = node1.Value;
-            }
+                    if (y.Parent == nodeToDelete)
+                    {
+                        if (x != null)
+                        {
+                            x.Parent = y;
+                        }
+                    }
+                    else
+                    {
+                        Transplant(y, y.RightChild);
+                        y.RightChild = nodeToDelete.RightChild;
+                        y.RightChild.Parent = y;
+                    }
 
-            // Adjust the Red-Black Tree rules
-            if (node1.Color == RedBlackTreeColors.Black && node2 != null)
-            {
-                _adjustTreeAfterRemoval(node2);
-                Root.Color = RedBlackTreeColors.Black;
+                    Transplant(nodeToDelete, y);
+                    y.LeftChild = nodeToDelete.LeftChild;
+                    y.LeftChild.Parent = y;
+                    y.Color = nodeToDelete.Color;
+
+                    if (Root == nodeToDelete)
+                    {
+                        Root = y;
+                        Root.Parent = null;
+                    }
+                }
+
+                if (nodeToDelete.Color == RedBlackTreeColors.Black)
+                {
+                    _adjustTreeAfterRemoval(x);
+                }
             }
 
-            // Decrement the count
             base._count--;
 
             return true;
         }
 
+        /// <summary>
+        ///     Insert one subtree in the place of the other in his parent.
+        /// </summary>
+        /// <param name="replaced">Subtree of node will be replaced by <param name="replacement">.</param></param>
+        /// <param name="replacement">Subtree replaces <param name="replaced">.</param></param>
+        private void Transplant(RedBlackTreeNode<TKey> replaced, RedBlackTreeNode<TKey> replacement)
+        {
+            if (replaced.Parent == null)
+            {
+                this.Root = replacement;
+            }
+            else if (replaced == replaced.Parent.LeftChild)
+            {
+                replaced.Parent.LeftChild = replacement;
+            }
+            else
+            {
+                replaced.Parent.RightChild = replacement;
+            }
 
+            if (replacement != null)
+            {
+                replacement.Parent = replaced.Parent;
+            }
+        }
         /*************************************************************************************************/
 
 
@@ -629,7 +660,5 @@ public override void RemoveMax()
             // Invoke the internal remove node method.
             this._remove(node);
         }
-
     }
-
 }