diff --git a/src/tree/vieter_tree.c b/src/tree/vieter_tree.c index 8a00088..b066af1 100644 --- a/src/tree/vieter_tree.c +++ b/src/tree/vieter_tree.c @@ -48,7 +48,7 @@ vieter_tree_error vieter_tree_remove(void **out, vieter_tree *tree, return vieter_tree_not_present; } - vieter_tree_error res = vieter_tree_node_remove(out, &tree->root, key); + vieter_tree_error res = vieter_tree_node_remove(out, tree->root, key); if (res != vieter_tree_ok) { return res; diff --git a/src/tree/vieter_tree_balancing.c b/src/tree/vieter_tree_balancing.c index 6e7dcd9..710203b 100644 --- a/src/tree/vieter_tree_balancing.c +++ b/src/tree/vieter_tree_balancing.c @@ -21,21 +21,14 @@ bool vieter_tree_node_validate(vieter_tree_node *node, vieter_tree_node_get(node->children[0], vieter_tree_node_black)) && (node->children[1] == NULL || vieter_tree_node_get(node->children[1], vieter_tree_node_black))); - bool right_child_flag_set = - node->parent == NULL || - (vieter_tree_node_get(node, vieter_tree_node_right) == - (node->parent->children[1] == node)); - return correctly_colored_children && right_child_flag_set && + return correctly_colored_children && vieter_tree_node_validate(node->children[0], passed_black_nodes, expected_black_nodes) && vieter_tree_node_validate(node->children[1], passed_black_nodes, expected_black_nodes); } -/* - * This function should be rewritten to use tree rotations instead. - */ vieter_tree_node *vieter_tree_node_balance(vieter_tree_node *node) { vieter_tree_node *parent = node->parent; vieter_tree_node *grand_parent = parent->parent; @@ -135,21 +128,11 @@ vieter_tree_node *vieter_tree_node_rotate(vieter_tree_node *old_root, bool dir) { vieter_tree_node *new_root = old_root->children[1 - dir]; - if (old_root->parent != NULL) { - vieter_tree_node_set_child( - old_root->parent, new_root, - vieter_tree_node_get(old_root, vieter_tree_node_right)); - } else { - new_root->parent = NULL; - } - // Right rotation if (dir) { vieter_tree_node_set_child(old_root, new_root->children[1], false); vieter_tree_node_set_child(new_root, old_root, true); - } - // Left rotation - else { + } else { vieter_tree_node_set_child(old_root, new_root->children[0], true); vieter_tree_node_set_child(new_root, old_root, false); } @@ -157,32 +140,7 @@ vieter_tree_node *vieter_tree_node_rotate(vieter_tree_node *old_root, return new_root; } -/* - * This function is currently implemented by very literally following the - * Wikipedia pseudocode. It's honestly not too bad, and I couldn't be bothered - * to properly research how to implement red-black removal (how is this so hard - * to find?) - * - * https://en.wikipedia.org/wiki/Red%E2%80%93black_tree#Removal_of_a_black_non-root_leaf - */ -vieter_tree_node *vieter_tree_node_remove_balanced(vieter_tree_node *node) { - vieter_tree_node *out; - - if (node->parent == NULL) { - out = node->children[0] != NULL ? node->children[0] : node->children[1]; - - vieter_tree_node_free(node); - - // This only happens when the root was the only element in the tree - if (out == NULL) - return out; - - vieter_tree_node_set(out, vieter_tree_node_black, true); - out->parent = NULL; - - return out; - } - +void vieter_tree_node_remove_balanced(vieter_tree_node *node) { // A red node can only have 0 or 2 children. The node we receive only has // one child at most, so we know if it's red that it doesn't have any // children. A black node that has a single (right) child can be replaced by @@ -190,8 +148,6 @@ vieter_tree_node *vieter_tree_node_remove_balanced(vieter_tree_node *node) { // replaced by its right child (even if it's NULL). if (!vieter_tree_node_get(node, vieter_tree_node_black) || node->children[1] != NULL) { - out = node->children[1]; - vieter_tree_node_set_child( node->parent, node->children[1], vieter_tree_node_get(node, vieter_tree_node_right)); @@ -201,13 +157,12 @@ vieter_tree_node *vieter_tree_node_remove_balanced(vieter_tree_node *node) { vieter_tree_node_free(node); - return out; + return; } // The complicated case is when we want to remove a black leaf // https://en.wikipedia.org/wiki/Red%E2%80%93black_tree#Removal_of_a_black_non-root_leaf - out = node; vieter_tree_node *parent = node->parent; vieter_tree_node *sibling, *close_nephew, *distant_nephew; bool dir = vieter_tree_node_get(node, vieter_tree_node_right); @@ -242,14 +197,13 @@ vieter_tree_node *vieter_tree_node_remove_balanced(vieter_tree_node *node) { // Case 2 vieter_tree_node_set(sibling, vieter_tree_node_black, false); node = parent; - out = node; } while ((parent = node->parent) != NULL); // Case 1 - return out; + return; case3: - out = vieter_tree_node_rotate(parent, dir); + vieter_tree_node_rotate(parent, dir); vieter_tree_node_set(parent, vieter_tree_node_black, false); vieter_tree_node_set(sibling, vieter_tree_node_black, true); sibling = close_nephew; @@ -268,8 +222,7 @@ case3: case4: vieter_tree_node_set(sibling, vieter_tree_node_black, false); vieter_tree_node_set(parent, vieter_tree_node_black, true); - - return out; + return; case5: vieter_tree_node_rotate(sibling, 1 - dir); @@ -279,11 +232,9 @@ case5: sibling = close_nephew; case6: - out = vieter_tree_node_rotate(parent, dir); + vieter_tree_node_rotate(parent, dir); vieter_tree_node_set(sibling, vieter_tree_node_black, vieter_tree_node_get(parent, vieter_tree_node_black)); vieter_tree_node_set(parent, vieter_tree_node_black, true); vieter_tree_node_set(distant_nephew, vieter_tree_node_black, true); - - return out; } diff --git a/src/tree/vieter_tree_balancing.h b/src/tree/vieter_tree_balancing.h index e5dced0..befc035 100644 --- a/src/tree/vieter_tree_balancing.h +++ b/src/tree/vieter_tree_balancing.h @@ -3,26 +3,6 @@ #include "vieter_tree_node.h" -/* - * Check whether the given tree is a valid red-black tree. - * - * @param node root of the (sub)tree - * @param passed_black_nodes how many black nodes the recursion has already seen - * at this point. This should be initialized as 0 for the topmost call. - * @param expected_black_nodes the correct amount of black nodes to expect when - * a NULL child is encountered. - * @return true if the tree is valid, false otherwise - */ -bool vieter_tree_node_validate(vieter_tree_node *node, - uint64_t passed_black_nodes, - uint64_t expected_black_nodes); - -/* - * Balance a path of 3 nodes into a complete binary tree, with a red root and - * black children. - */ -vieter_tree_node *vieter_tree_node_balance(vieter_tree_node *node); - /* * Ensure the tree remains a valid red-black tree after having inserting the * node. @@ -32,15 +12,9 @@ void vieter_tree_node_balance_after_insert(vieter_tree_node *node); /* * Remove the given node, ensuring the tree remains a valid red-black tree. * - * @param node node to remove. This should have at most a single child. If node - * isn't the root, this should be the right child, otherwise it can be either. - * @return root of the subtree that this function operated on. This can - * sometimes be the new root of the entire tree. + * @param node node to remove. This should have at most a single child, namely + * the right one. */ -vieter_tree_node *vieter_tree_node_remove_balanced(vieter_tree_node *node); +void vieter_tree_node_remove_balanced(vieter_tree_node *node); -/* - * Perform a tree rotation of the subtree with the given root. - */ -vieter_tree_node *vieter_tree_node_rotate(vieter_tree_node *old_root, bool dir); #endif diff --git a/src/tree/vieter_tree_node.c b/src/tree/vieter_tree_node.c index 57aea05..3f09f3e 100644 --- a/src/tree/vieter_tree_node.c +++ b/src/tree/vieter_tree_node.c @@ -109,17 +109,16 @@ vieter_tree_error vieter_tree_node_search(void **out, vieter_tree_node *root, return vieter_tree_ok; } -vieter_tree_error -vieter_tree_node_remove(void **out, vieter_tree_node **root_ptr, uint64_t key) { +vieter_tree_error vieter_tree_node_remove(void **out, vieter_tree_node *root, + uint64_t key) { vieter_tree_node *target; - vieter_tree_error res = vieter_tree_node_search_node(&target, *root_ptr, key); + vieter_tree_error res = vieter_tree_node_search_node(&target, root, key); if (res != vieter_tree_ok) { return res; } *out = target->data; - vieter_tree_node *possible_new_root; if (target->children[0] != NULL && target->children[1] != NULL) { vieter_tree_node *replacement = target->children[1]; @@ -131,15 +130,9 @@ vieter_tree_node_remove(void **out, vieter_tree_node **root_ptr, uint64_t key) { target->key = replacement->key; target->data = replacement->data; - possible_new_root = vieter_tree_node_remove_balanced(replacement); + vieter_tree_node_remove_balanced(replacement); } else { - possible_new_root = vieter_tree_node_remove_balanced(target); - } - - if (possible_new_root == NULL) { - *root_ptr = NULL; - } else if (possible_new_root->parent == NULL) { - *root_ptr = possible_new_root; + vieter_tree_node_remove_balanced(target); } return vieter_tree_ok; diff --git a/src/tree/vieter_tree_node.h b/src/tree/vieter_tree_node.h index fabee6b..f164704 100644 --- a/src/tree/vieter_tree_node.h +++ b/src/tree/vieter_tree_node.h @@ -33,7 +33,7 @@ vieter_tree_error vieter_tree_node_search_node(vieter_tree_node **out, vieter_tr vieter_tree_error vieter_tree_node_search(void **out, vieter_tree_node *root, uint64_t key); -vieter_tree_error vieter_tree_node_remove(void **out, vieter_tree_node **root_ptr, uint64_t key); +vieter_tree_error vieter_tree_node_remove(void **out, vieter_tree_node *root, uint64_t key); void vieter_tree_node_replace_with_child(vieter_tree_node *to_replace, vieter_tree_node *replacement); diff --git a/test/tree/test_balancing.c b/test/tree/test_balancing.c deleted file mode 100644 index cfda9d3..0000000 --- a/test/tree/test_balancing.c +++ /dev/null @@ -1,71 +0,0 @@ -#include "acutest.h" -#include "vieter_tree_internal.h" -#include "vieter_tree_node.h" -#include "vieter_tree_balancing.h" - -// This uses the examples from wikipedia -// https://en.wikipedia.org/wiki/Tree_rotation - -void test_rotate_right() { - vieter_tree_node *a = vieter_tree_node_init(); - vieter_tree_node *b = vieter_tree_node_init(); - vieter_tree_node *p = vieter_tree_node_init(); - - vieter_tree_node_set_child(p, a, false); - vieter_tree_node_set_child(p, b, true); - - vieter_tree_node *c = vieter_tree_node_init(); - vieter_tree_node *q = vieter_tree_node_init(); - - vieter_tree_node_set_child(q, p, false); - vieter_tree_node_set_child(q, c, true); - - vieter_tree_node *new_root = vieter_tree_node_rotate(q, true); - - TEST_CHECK(new_root == p); - TEST_CHECK(new_root->children[0] == a); - TEST_CHECK(new_root->children[1] == q); - TEST_CHECK(new_root->children[1]->children[0] == b); - TEST_CHECK(new_root->children[1]->children[1] == c); - - vieter_tree_node_free(a); - vieter_tree_node_free(b); - vieter_tree_node_free(p); - vieter_tree_node_free(c); - vieter_tree_node_free(q); -} - -void test_rotate_left() { - vieter_tree_node *b = vieter_tree_node_init(); - vieter_tree_node *c = vieter_tree_node_init(); - vieter_tree_node *q = vieter_tree_node_init(); - - vieter_tree_node_set_child(q, b, false); - vieter_tree_node_set_child(q, c, true); - - vieter_tree_node *a = vieter_tree_node_init(); - vieter_tree_node *p = vieter_tree_node_init(); - - vieter_tree_node_set_child(p, a, false); - vieter_tree_node_set_child(p, q, true); - - vieter_tree_node *new_root = vieter_tree_node_rotate(p, false); - - TEST_CHECK(new_root == q); - TEST_CHECK(new_root->children[0] == p); - TEST_CHECK(new_root->children[1] == c); - TEST_CHECK(new_root->children[0]->children[0] == a); - TEST_CHECK(new_root->children[0]->children[1] == b); - - vieter_tree_node_free(a); - vieter_tree_node_free(b); - vieter_tree_node_free(p); - vieter_tree_node_free(c); - vieter_tree_node_free(q); -} - -TEST_LIST = { - {"tree tree rotate right", test_rotate_right}, - {"tree tree rotate left", test_rotate_left}, - {NULL, NULL} -}; diff --git a/test/tree/test_tree.c b/test/tree/test_tree.c index 2ab6870..257ec66 100644 --- a/test/tree/test_tree.c +++ b/test/tree/test_tree.c @@ -42,7 +42,7 @@ void test_remove() { void *out; - for (uint64_t i = 0; i < 250; i++) { + for (uint64_t i = 0; i < 25; i++) { TEST_CHECK(vieter_tree_search(&out, tree, i) == vieter_tree_ok); TEST_CHECK(vieter_tree_remove(&out, tree, i) == vieter_tree_ok); TEST_CHECK(vieter_tree_validate(tree));