From 4b89ec2ee136f8ac069527b99f090ccfcad1ed9b Mon Sep 17 00:00:00 2001 From: Brett Date: Thu, 12 Jan 2023 14:09:34 -0500 Subject: [PATCH] Two children delete is still broken. Single / no child delete works fine. --- build_and_run_debug.sh | 4 ++ include/blt/std/binary_tree.h | 95 +++++++++++++++++------------------ src/tests/binary_trees.h | 5 +- 3 files changed, 54 insertions(+), 50 deletions(-) create mode 100755 build_and_run_debug.sh diff --git a/build_and_run_debug.sh b/build_and_run_debug.sh new file mode 100755 index 0000000..79d5e50 --- /dev/null +++ b/build_and_run_debug.sh @@ -0,0 +1,4 @@ +#!/bin/bash +cd cmake-build-debug +cmake -DBUILD_TESTS=ON -G Ninja ../ && ninja -j 16 && ./BLT_TESTS +cd .. diff --git a/include/blt/std/binary_tree.h b/include/blt/std/binary_tree.h index 8625965..a7c7d6d 100644 --- a/include/blt/std/binary_tree.h +++ b/include/blt/std/binary_tree.h @@ -134,7 +134,6 @@ namespace blt { } return root; } - public: node_binary_search_tree() = default; @@ -153,58 +152,56 @@ namespace blt { void remove(const T& element) { BST_node* parent = nullptr; BST_node* elementNode = search(&parent, element); - - BST_node*& parentChildSide = parent->left; - if (parent->right == elementNode) - parentChildSide = parent->right; + if (parent == elementNode) + parent = nullptr; if (elementNode->left != nullptr && elementNode->right != nullptr) { - // root node special case: TODO: better way of doing this. - /*auto& leastNodeGreater = findMin(elementNode->right); - if (parent != elementNode) { - // move up the node and delete the old one. - parentChildSide->payload = leastNodeGreater->payload; - if (leastNodeGreater->parent->left == leastNodeGreater) - leastNodeGreater->parent->left = nullptr; - else - leastNodeGreater->parent->right = nullptr; - delete(leastNodeGreater); - } else { - leastNodeGreater->left = - }*/ - /* delete(m_root); - m_root = nullptr; - // reconstruct subtree. More efficient way of doing this... TODO - std::vector subNodes = inOrderTraverse(elementNode); - for (auto* node : subNodes) { - // insert will create a new node, we must delete old one to prevent memory leaks - if (node != elementNode) { - insert(node->payload); - delete (node); - } - } - } else { - - // reconstruct subtree. More efficient way of doing this... TODO - std::vector subNodes = inOrderTraverse(elementNode); - for (auto* node : subNodes) { - // insert will create a new node, we must delete old one to prevent memory leaks - if (node != elementNode) { - insert(parent, node->payload); - delete (node); - } - } - }*/ - } else { - parentChildSide = elementNode->left != nullptr ? elementNode->left : elementNode->right; - } - std::cout << elementNode << "\n"; - //delete (elementNode); + BST_node* inOrderSuccessor = elementNode->left != nullptr ? elementNode->left : elementNode->right; + BST_node* inOrderSuccessorParent = nullptr; + // go all the way to the left subtree + while (inOrderSuccessor->left != nullptr) { + inOrderSuccessorParent = inOrderSuccessor; + inOrderSuccessor = inOrderSuccessor->left; + } + // make sure we maintain the tree structure if our moving node has a subtree + BST_node* inOrderSuccessorReplacement = inOrderSuccessor->right != nullptr ? inOrderSuccessor->right : nullptr; + if (parent != nullptr) { + if (parent->right == elementNode) + parent->right = inOrderSuccessor; + else if (parent->left == elementNode) + parent->left = inOrderSuccessor; + else + throw binary_search_tree_error("Parent node doesn't own child!\n"); + } else + m_root = inOrderSuccessor; + // reconstruct the node's children + inOrderSuccessor->left = elementNode->left; + inOrderSuccessor->right = elementNode->right; + // delete the parent's reference to the moved node + if (inOrderSuccessorParent != nullptr){ + if (inOrderSuccessorParent->left == inOrderSuccessor) + inOrderSuccessorParent->left = inOrderSuccessorReplacement; + else if (inOrderSuccessorParent->right == inOrderSuccessor) + inOrderSuccessorParent->right = inOrderSuccessorReplacement; + else + throw binary_search_tree_error("Parent does not contain child!\n"); + } + } else { + auto replacementNode = elementNode->left != nullptr ? elementNode->left : elementNode->right; + if (parent == nullptr) + m_root = replacementNode; + else { + if (parent->right == elementNode) + parent->right = replacementNode; + else if (parent->left == elementNode) + parent->left = replacementNode; + else + throw binary_search_tree_error("Parent node doesn't contain element of search!\n"); + } + } + delete (elementNode); } - /*void remove(const T& element) { - remove(m_root, element); - }*/ inline std::vector inOrderTraverse() { return inOrderTraverse(m_root); diff --git a/src/tests/binary_trees.h b/src/tests/binary_trees.h index d17da41..60fd2b3 100644 --- a/src/tests/binary_trees.h +++ b/src/tests/binary_trees.h @@ -32,9 +32,12 @@ void binaryTreeTest(){ auto searchedNode = dataTree.search(10); std::cout << "10's children: "<< searchedNode->left->payload << ", " << searchedNode->right->payload << "\n"; - dataTree.remove(4); + dataTree.remove(10); printBinaryTree(dataTree); + + //searchedNode = dataTree.search(8); + //std::cout << "8's children: "<< searchedNode->left->payload << ", " << searchedNode->right->payload << "\n"; } \ No newline at end of file