Merge pull request #91 from UBC-MDS/fix-delete_search_enhancement

Fix delete search enhancement, also added time and space complexity in readme

Author: javiermtzo99

README.md

@@ -16,6 +16,15 @@
     - Keys in the right subtree of a node are larger than the node’s key.
     - Duplicate keys are not allowed.
 
+    Time Complexity:
+    Search, Insert, Delete:
+    - Average Case: O(log n) => This occurs when the BST is balanced.
+    - Worst Case: O(n) => This occurs when the BST is skewed.
+
+    Space Complexity:
+    - Space for the BST: O(n).
+    - Recursive Operations: O(h), where h is the height of the tree.
+
     **Methods**
     - **`insert(key)`**:
         - Inserts a specified key into the Binary Search Tree (BST) while maintaining BST properties.

src/datastructpy/non_linear/trees/binary_search_tree.py

@@ -94,6 +94,11 @@ def search(self, key):
             - The Node object containing the specified key if found.
             - None if the key does not exist or the tree is empty.
 
+        Raises
+        ------
+        TypeError
+            If the key is not an integer or is None.
+
         Examples
         --------
         >>> bst = BinarySearchTree()
@@ -105,6 +110,11 @@ def search(self, key):
         >>> bst.search(20) is None
         True
         """
+        if key is None:
+            raise TypeError("None values are not allowed in the BST.")
+        if not isinstance(key, int):
+            raise TypeError("Only integers are allowed in the BST.")
+
         current = self.root
         while current is not None:
             if key == current.key:
@@ -128,19 +138,34 @@ def delete(self, key):
         key : int
             The value to delete from the BST.
 
+        Returns
+        -------
+        bool
+            - True if the key was found and deleted.
+            - False if the key was not found.
+
+        Raises
+        ------
+        TypeError
+            If the key is not an integer or is None.
+
         Examples
         --------
         >>> bst = BinarySearchTree()
         >>> bst.insert(10)
         >>> bst.insert(5)
         >>> bst.insert(15)
         >>> bst.delete(10)
-        >>> bst.root.key
-        15
+        True
+        >>> bst.delete(20)
+        False
         """
+        if key is None:
+            raise TypeError("None values are not allowed in the BST.")
+        if not isinstance(key, int):
+            raise TypeError("Only integers are allowed in the BST.")
+
         def _delete(node, key):
-            if node is None:
-                return None
             if key < node.key:
                 node.left = _delete(node.left, key)
             elif key > node.key:
@@ -157,7 +182,10 @@ def _delete(node, key):
                 node.right = _delete(node.right, min_larger_node.key)
             return node
 
+        if self.search(key) is None:
+            return False
         self.root = _delete(self.root, key)
+        return True
 
     @staticmethod
     def list_to_tree(elements):

tests/non-linear/trees/binary_search_tree/test_delete.py

@@ -18,43 +18,84 @@ def bst():
 def test_delete_empty_tree():
     """
     Tests the delete operation on an empty tree.
-    Deleting a key from an empty BST should not cause any errors.
+    Deleting a key from an empty BST should return False.
     """
     bst = BinarySearchTree()
-    bst.delete(10)
-    assert bst.root is None, "Deleting from an empty tree should do nothing."
+    assert not bst.delete(10), "Deleting from an empty tree should return False."
 
 def test_delete_leaf_node(bst):
     """
     Tests the deletion of a leaf node (node with no children).
     """
-    bst.delete(5)
-    assert bst.search(5) is None, "Leaf node 5 should be deleted."
+    assert bst.delete(5), "Leaf node 5 should be successfully deleted."
+    assert bst.search(5) is None, "Leaf node 5 should no longer be in the tree."
+
+def test_delete_non_existent_key(bst):
+    """
+    Tests deleting a key that does not exist in the BST.
+    Should return False without modifying the tree.
+    """
+    assert not bst.delete(100), "Deleting a non-existent key should return False."
+
+def test_delete_non_integer(bst):
+    """
+    Tests deleting a key that isn't an integer (string).
+    Should raise a TypeError.
+    """
+    with pytest.raises(TypeError):
+        bst.delete("100")
+
+def test_delete_none_key(bst):
+    """
+    Tests deleting a None key.
+    Should raise a TypeError.
+    """
+    with pytest.raises(TypeError):
+        bst.delete(None)
+
+def test_delete_none_node(bst):
+    """
+    Tests deleting a key that does not exist in the BST.
+    Should return False without modifying the tree.
+    """
+    bst = BinarySearchTree()
+    assert not bst.delete(100), "Deleting a non-existent key should return False."        
+
+def test_delete_node_with_only_left_child():
+    """
+    Tests deleting a node that has only a left child.
+    Ensures it reaches the `elif node.right is None: return node.left` condition.
+    """
+    bst = BinarySearchTree()
+    bst.insert(10)
+    bst.insert(5)  # Left child only
+    assert bst.delete(10), "Node 10 should be successfully deleted."
+    assert bst.root.key == 5, "Node 5 should replace node 10."
 
 def test_delete_traverse_right_subtree(bst):
     """
     Tests the deletion of a node located in the right subtree.
     """
-    bst.delete(30)
-    assert bst.search(30) is None, "Node 30 should be deleted."
+    assert bst.delete(30), "Node 30 should be successfully deleted."
+    assert bst.search(30) is None, "Node 30 should no longer be in the tree."
     assert bst.root.right.key == 35, "Node 35 should replace node 30 in the right subtree."
 
 def test_delete_node_with_one_child(bst):
     """
     Tests the deletion of a node with only one child.
     The child node should replace the deleted node.
     """
-    bst.delete(10)
-    assert bst.search(10) is None, "Node 10 should be deleted."
+    assert bst.delete(10), "Node 10 should be successfully deleted."
+    assert bst.search(10) is None, "Node 10 should no longer be in the tree."
     assert bst.root.left.key == 15, "Node 15 should replace node 10."
 
 def test_delete_node_with_two_children(bst):
     """
     Tests the deletion of a node that has two children.
     The node should be replaced by its in-order successor.
     """
-    bst.delete(20)
-    assert bst.search(20) is None, "Node 20 should be deleted."
+    assert bst.delete(20), "Node 20 should be successfully deleted."
+    assert bst.search(20) is None, "Node 20 should no longer be in the tree."
     assert bst.root.key == 25, "Root should now be replaced by in-order successor 25."
 
 def test_delete_root_node():
@@ -64,14 +105,14 @@ def test_delete_root_node():
     """
     bst = BinarySearchTree()
     bst.insert(10)
-    bst.delete(10)
-    assert bst.search(10) is None, "Root node 10 should be deleted."
+    assert bst.delete(10), "Root node 10 should be successfully deleted."
+    assert bst.search(10) is None, "Root node 10 should no longer be in the tree."
 
 def test_delete_complex_tree(bst):
     """
     Tests the deletion of a node in a complex tree structure.
     Ensures the BST maintains correct structure after deletion.
     """
-    bst.delete(20)  # Deleting root with two children
-    assert bst.search(20) is None, "Root 20 should be deleted."
+    assert bst.delete(20), "Root 20 should be successfully deleted."
+    assert bst.search(20) is None, "Root 20 should no longer be in the tree."
     assert bst.root.key == 25, "Root should now be replaced by in-order successor 25."

tests/non-linear/trees/binary_search_tree/test_search.py

@@ -43,9 +43,14 @@ def test_search_empty_tree():
     empty_bst = BinarySearchTree()
     assert empty_bst.search(10) is None  # Should return None
 
-def test_search_invalid_algorithm(bst):
+def test_search_invalid_key():
     """
-    Tests if search() handles invalid algorithm names properly.
+    Tests if search() raises TypeError for invalid key types.
     """
-    with pytest.raises(TypeError):  # Since 'algorithm' is removed, this should raise a TypeError
-        bst.search(10, algorithm='invalid_algorithm')
+    bst = BinarySearchTree.list_to_tree([10, 5, 15])
+    with pytest.raises(TypeError):
+        bst.search(None)
+    with pytest.raises(TypeError):
+        bst.search("string")
+    with pytest.raises(TypeError):
+        bst.search(10.5)