fix: SplayTree.remove is broken

nghiahoang · nghiahoang · commit f4fb2fc0eb04 · 2020-06-25T13:17:01.000+07:00
diff --git a/Splay Tree/SplayTree.playground/Contents.swift b/Splay Tree/SplayTree.playground/Contents.swift
@@ -5,29 +5,10 @@
 print("Hello, Swift 4!")
 #endif
 
-let splayTree = SplayTree(value: 1)
-splayTree.insert(value: 2)
-splayTree.insert(value: 10)
-splayTree.insert(value: 6)
-
-splayTree.remove(value: 10)
-splayTree.remove(value: 6)
-
-splayTree.insert(value: 55)
-splayTree.insert(value: 559)
-splayTree.remove(value: 2)
-splayTree.remove(value: 1)
-splayTree.remove(value: 55)
-splayTree.remove(value: 559)
-
-splayTree.insert(value: 1843000)
-splayTree.insert(value: 1238)
-splayTree.insert(value: -1)
-splayTree.insert(value: 87)
-
-splayTree.minimum()
-splayTree.maximum()
-
-
-
-
+var tree = SplayTree<Int>(value: 0)
+tree.insert(value: 2)
+tree.insert(value: 3)
+tree.insert(value: 4)
+tree.insert(value: 7)
+_ = tree.search(value: 2)
+tree.remove(value: 2)
diff --git a/Splay Tree/SplayTree.playground/Sources/SplayTree.swift b/Splay Tree/SplayTree.playground/Sources/SplayTree.swift
@@ -289,120 +289,42 @@ extension Node {
      - Node     Resulting from the deletion and the splaying of the removed node
      
      */
-    public func remove(value: T) -> Node<T>? {
-        let replacement: Node<T>?
+    fileprivate func remove(value: T) -> Node<T>? {
+        guard let target = search(value: value) else { return self }
         
-        if let v = self.value, v == value {
+        if let left = target.left, let right = target.right {
+            let largestOfLeftChild = left.maximum()
+            left.parent = nil
+            right.parent = nil
             
-            var parentToSplay: Node<T>?
-            if let left = left {
-                if let right = right {
-                    
-                    replacement = removeNodeWithTwoChildren(left, right)
-                    
-                    if let replacement = replacement,
-                        let replacementParent = replacement.parent,
-                        replacementParent.value != self.value {
-                        
-                        parentToSplay = replacement.parent
-                        
-                    } else if self.parent != nil {
-                        parentToSplay = self.parent
-                    } else {
-                        parentToSplay = replacement
-                    }
-                    
-                } else {
-                    // This node only has a left child. The left child replaces the node.
-                    replacement = left
-                    if self.parent != nil {
-                        parentToSplay = self.parent
-                    } else {
-                        parentToSplay = replacement
-                    }
-                }
-            } else if let right = right {
-                // This node only has a right child. The right child replaces the node.
-                replacement = right
-                if self.parent != nil {
-                    parentToSplay = self.parent
-                } else {
-                    parentToSplay = replacement
-                }
-            } else {
-                // This node has no children. We just disconnect it from its parent.
-                replacement = nil
-                parentToSplay = parent
-            }
-            
-            reconnectParentTo(node: replacement)
+            SplayOperation.splay(node: largestOfLeftChild)
+            largestOfLeftChild.right = right
             
-            // performs the splay operation
-            if let parentToSplay = parentToSplay {
-                SplayOperation.splay(node: parentToSplay)
-            }
+            return largestOfLeftChild
             
-            // The current node is no longer part of the tree, so clean it up.
-            parent = nil
-            left = nil
-            right = nil
+        } else if let left = target.left {
+            replace(node: target, with: left)
+            return left
             
-            return parentToSplay
+        } else if let right = target.right {
+            replace(node: target, with: right)
+            return right
             
-        } else if let v = self.value, value < v {
-            if left != nil {
-                return left!.remove(value: value)
-            } else {
-                let node = self
-                SplayOperation.splay(node: node)
-                return node
-                
-            }
         } else {
-            if right != nil {
-                return right?.remove(value: value)
-            } else {
-                let node = self
-                SplayOperation.splay(node: node)
-                return node
-                
-            }
+            return nil
         }
     }
     
-    private func removeNodeWithTwoChildren(_ left: Node, _ right: Node) -> Node {
-        // This node has two children. It must be replaced by the smallest
-        // child that is larger than this node's value, which is the leftmost
-        // descendent of the right child.
-        let successor = right.minimum()
+    private func replace(node: Node<T>, with newNode: Node<T>?) {
+        guard let sourceParent = sourceNode.parent else { return }
         
-        // Connect our left child with the new node.
-        successor.left = left
-        left.parent = successor
-        
-        // Connect our right child with the new node. If the right child does
-        // not have any left children of its own, then the in-order successor
-        // *is* the right child.
-        if right !== successor {
-            successor.right = right
-            right.parent = successor
+        if sourceNode.isLeftChild {
+            sourceParent.left = newNode
         } else {
-            successor.right = nil
+            sourceParent.right = newNode
         }
         
-        // And finally, connect the successor node to our parent.
-        return successor
-    }
-    
-    private func reconnectParentTo(node: Node?) {
-        if let parent = parent {
-            if isLeftChild {
-                parent.left = node
-            } else {
-                parent.right = node
-            }
-        }
-        node?.parent = parent
+        newNode?.parent = sourceParent
     }
 }
 
diff --git a/Splay Tree/SplayTree.swift b/Splay Tree/SplayTree.swift
@@ -44,7 +44,7 @@ public enum SplayOperation {
         - Returns
             - Operation     Case zigZag - zigZig - zig
      */
-    public static func operation<T: Comparable>(forNode node: Node<T>) -> SplayOperation {
+    public static func operation<T>(forNode node: Node<T>) -> SplayOperation {
         
         if let parent = node.parent, let _ = parent.parent {
             if (node.isLeftChild && parent.isRightChild) || (node.isRightChild && parent.isLeftChild) {
@@ -289,120 +289,42 @@ extension Node {
             - Node     Resulting from the deletion and the splaying of the removed node
      
      */
-    public func remove(value: T) -> Node<T>? {
-        let replacement: Node<T>?
+    fileprivate func remove(value: T) -> Node<T>? {
+        guard let target = search(value: value) else { return self }
         
-        if let v = self.value, v == value {
+        if let left = target.left, let right = target.right {
+            let largestOfLeftChild = left.maximum()
+            left.parent = nil
+            right.parent = nil
             
-            var parentToSplay: Node<T>?
-            if let left = left {
-                if let right = right {
-                    
-                    replacement = removeNodeWithTwoChildren(left, right)
-                    
-                    if let replacement = replacement,
-                        let replacementParent = replacement.parent,
-                        replacementParent.value != self.value {
-                        
-                        parentToSplay = replacement.parent
-                        
-                    } else if self.parent != nil {
-                        parentToSplay = self.parent
-                    } else {
-                        parentToSplay = replacement
-                    }
-                    
-                } else {
-                    // This node only has a left child. The left child replaces the node.
-                    replacement = left
-                    if self.parent != nil {
-                        parentToSplay = self.parent
-                    } else {
-                        parentToSplay = replacement
-                    }
-                }
-            } else if let right = right {
-                // This node only has a right child. The right child replaces the node.
-                replacement = right
-                if self.parent != nil {
-                    parentToSplay = self.parent
-                } else {
-                    parentToSplay = replacement
-                }
-            } else {
-                // This node has no children. We just disconnect it from its parent.
-                replacement = nil
-                parentToSplay = parent
-            }
+            SplayOperation.splay(node: largestOfLeftChild)
+            largestOfLeftChild.right = right
             
-            reconnectParentTo(node: replacement)
+            return largestOfLeftChild
             
-            // performs the splay operation
-            if let parentToSplay = parentToSplay {
-                SplayOperation.splay(node: parentToSplay)
-            }
+        } else if let left = target.left {
+            replace(node: target, with: left)
+            return left
             
-            // The current node is no longer part of the tree, so clean it up.
-            parent = nil
-            left = nil
-            right = nil
-
-            return parentToSplay
+        } else if let right = target.right {
+            replace(node: target, with: right)
+            return right
             
-        } else if let v = self.value, value < v {
-            if left != nil {
-                return left!.remove(value: value)
-            } else {
-                let node = self
-                SplayOperation.splay(node: node)
-                return node
-                
-            }
         } else {
-            if right != nil {
-                return right?.remove(value: value)
-            } else {
-                let node = self
-                SplayOperation.splay(node: node)
-                return node
-                
-            }
+            return nil
         }
     }
     
-    private func removeNodeWithTwoChildren(_ left: Node, _ right: Node) -> Node {
-        // This node has two children. It must be replaced by the smallest
-        // child that is larger than this node's value, which is the leftmost
-        // descendent of the right child.
-        let successor = right.minimum()
+    private func replace(node: Node<T>, with newNode: Node<T>?) {
+        guard let sourceParent = sourceNode.parent else { return }
         
-        // Connect our left child with the new node.
-        successor.left = left
-        left.parent = successor
-        
-        // Connect our right child with the new node. If the right child does
-        // not have any left children of its own, then the in-order successor
-        // *is* the right child.
-        if right !== successor {
-            successor.right = right
-            right.parent = successor
+        if sourceNode.isLeftChild {
+            sourceParent.left = newNode
         } else {
-            successor.right = nil
+            sourceParent.right = newNode
         }
         
-        // And finally, connect the successor node to our parent.
-        return successor
-    }
-    
-    private func reconnectParentTo(node: Node?) {
-        if let parent = parent {
-            if isLeftChild {
-                parent.left = node
-            } else {
-                parent.right = node
-            }
-        }
-        node?.parent = parent
+        newNode?.parent = sourceParent
     }
 }
 
diff --git a/Splay Tree/readme.md b/Splay Tree/readme.md
@@ -102,8 +102,11 @@ To insert a value:
 
 To delete a value: 
 
-- Delete it as in a binary search tree 
-- Splay the parent of the removed node to the root 
+- Perform search for the value, after performed search function if the tree contains the value, it'll be the root of the new tree.
+- If the tree has only left child, change left child to the root of the tree, remove the old root node
+- If the tree has only right child, change right child to the root of the tree, remove the old root node
+- Else, the tree has both two children, set parent of two children to nil, so that they're two new trees (left-tree and right-tree).
+- Splay the minimum node of right-tree (or minimum node of left-tree), then set left-tree as left child of new root of right-tree (or set right-tree as right child of new root of left-tree), return right-tree
 
 ### Search 
 
