Implemented BinaryTree_Remove() and wrote a test for it

src/BinaryTree/BinaryTree.c

@@ -13,7 +13,8 @@ typedef struct BinaryTreeWalker_s {
 	enum BinaryTreeNodePosition position;
 } BinaryTreeWalker;
 
-static size_t _TreeNodeSize(BinaryTree* tree) {
+static size_t _TreeNodeSize(BinaryTree* tree)
+{
 	return sizeof(BinaryTreeNode) + tree->value_size;
 }
 
@@ -22,7 +23,8 @@ static inline size_t _DepthOf(BinaryTreeNode* node)
 	return node == NULL ? 0 : node->depth;
 }
 
-static BinaryTreeNode* _CreateNode(BinaryTree* tree, void* value) {
+static BinaryTreeNode* _CreateNode(BinaryTree* tree, void* value)
+{
 	BinaryTreeNode* node = Allocator_Allocate(tree->allocator, _TreeNodeSize(tree));
 	if (node != NULL) {
 		memcpy(node->value, value, tree->value_size);
@@ -34,7 +36,8 @@ static BinaryTreeNode* _CreateNode(BinaryTree* tree, void* value) {
 	return node;
 }
 
-int BinaryTree_Create(BinaryTree* target, BinaryTreeComparator compare, void* xdata, size_t value_size, allocator_t* allocator) {
+int BinaryTree_Create(BinaryTree* target, BinaryTreeComparator compare, void* xdata, size_t value_size, allocator_t* allocator)
+{
 	target->root = NULL;
 	target->compare = compare;
 	target->xdata = xdata;
@@ -47,7 +50,8 @@ int BinaryTree_Create(BinaryTree* target, BinaryTreeComparator compare, void* xd
 	return EXIT_SUCCESS;
 }
 
-static void _UpdateDepth(BinaryTreeNode* node) {
+static void _UpdateDepth(BinaryTreeNode* node)
+{
 	size_t depth_left = 0, depth_right = 0;
 
 	if (node->left != NULL) {
@@ -66,58 +70,167 @@ static void _UpdateDepth(BinaryTreeNode* node) {
 	node->depth += 1;
 }
 
-static void _UpdateDepths(DynamicArray* parents) {
+static void _UpdateDepths(DynamicArray* parents)
+{
 	for (size_t i = parents->reserved - 1; i != SIZE_MAX; i--) {
 		BinaryTreeWalker* walker = DynamicArray_GetPointer(parents, i);
 		_UpdateDepth(walker->node);
 	}
 }
 
-int _Insert(BinaryTree* tree, BinaryTreeNode* node) {
+static int _BinaryTree_ValueWalk(BinaryTree* tree, void* value)
+{
 	BinaryTreeNode** parent = &tree->root;
 	tree->walker_stack.reserved = 0;
 
-	if (*parent == NULL) {
+	while ((*parent) != NULL) {
-		*parent = node;
-	}
-
-	while ((*parent) != node) {
 		BinaryTreeWalker* walker;
 		DynamicArray_AppendEmpty(&tree->walker_stack, (void**) &walker);
 		// Can't be NULL because the size was just increased
 
 		walker->node = *parent;
 
-		int comparison = tree->compare(node->value, (*parent)->value, tree->xdata);
+		int comparison = tree->compare(value, (*parent)->value, tree->xdata);
 
 		if (comparison == 0) {
 			// equal
-			return EXIT_FAILURE;
+			walker->position = BINARYTREENODEPOSITION_SELF;
+			return EXIT_SUCCESS;
 		} else if (comparison < 0) {
-			// less than
-			if ((*parent)->left == NULL) {
-				(*parent)->left = node;
-			}
-
 			parent = &(*parent)->left;
 			walker->position = BINARYTREENODEPOSITION_LEFT;
 		} else {
-			// greater than
-			if ((*parent)->right == NULL) {
-				(*parent)->right = node;
-			}
-
 			parent = &(*parent)->right;
 			walker->position = BINARYTREENODEPOSITION_RIGHT;
 		}
 	}
 
-	_UpdateDepths(&tree->walker_stack);
+	return ENOTFOUND;
+}
+
+static int _BinaryTree_Insert(BinaryTree* tree, BinaryTreeNode* node)
+{
+	int walk_code = _BinaryTree_ValueWalk(tree, node->value);
+
+	if (walk_code != ENOTFOUND) {
+		return EEXIST;
+	}
+
+	BinaryTreeWalker* bottom_walker = DynamicArray_GetPointer(&tree->walker_stack, tree->walker_stack.reserved - 1);
+	if (bottom_walker == NULL) {
+		// root node is empty
+		tree->root = node;
+		return EXIT_SUCCESS;
+	} else {
+		switch (bottom_walker->position) {
+			case BINARYTREENODEPOSITION_SELF:
+				// unreachable code
+				return EXIT_FAILURE;
+
+			case BINARYTREENODEPOSITION_LEFT:
+				bottom_walker->node->left = node;
+				break;
+
+			case BINARYTREENODEPOSITION_RIGHT:
+				bottom_walker->node->right = node;
+				break;
+		}
+	}
+
+        _UpdateDepths(&tree->walker_stack);
 	_UpdateDepth(node);
 
 	return EXIT_SUCCESS;
 }
 
+static int _BinaryTree_DestroyNode(BinaryTree* tree, BinaryTreeNode* node)
+{
+	Allocator_Free(tree->allocator, node, sizeof(*node) + tree->value_size);
+
+	return EXIT_SUCCESS;
+}
+
+static BinaryTreeNode** _BinaryTreeNode_SmallestNodeLocation(BinaryTreeNode* node)
+{
+	if (node->left == NULL) {
+		return NULL;
+	}
+	
+	BinaryTreeNode** smallest_location = &node->left;
+	do {
+		smallest_location = &(*smallest_location)->left;
+	} while (smallest_location != NULL);
+
+	return smallest_location;
+}
+
+static int _BinaryTreeNode_UnlinkNode(BinaryTreeNode** nodep)
+{
+	size_t node_depth = _DepthOf(*nodep);
+	if (node_depth == 1) {
+		// leaf node without children
+		*nodep = NULL;
+	} else {
+		if ((*nodep)->left == NULL) {
+			// only right node active
+			*nodep = (*nodep)->right;
+		} else if ((*nodep)->right == NULL) {
+			// only left node active
+			*nodep = (*nodep)->left;
+		} else {
+			// both right and left node active
+			BinaryTreeNode** smallest_subnode = _BinaryTreeNode_SmallestNodeLocation(*nodep);
+			BinaryTreeNode* right_branch = (*nodep)->right;
+			*nodep = *smallest_subnode;
+			*smallest_subnode = (*smallest_subnode)->right;
+			(*nodep)->right = right_branch;
+		}
+	}
+
+	return EXIT_SUCCESS;
+}
+
+static int _BinaryTree_Remove(BinaryTree* self, void* value)
+{
+	int walk_code = _BinaryTree_ValueWalk(self, value);
+
+	if (walk_code == ENOTFOUND) {
+		return ENOTFOUND;
+	}
+
+	BinaryTreeWalker* bottom_walker = DynamicArray_GetPointer(&self->walker_stack, self->walker_stack.reserved - 2);
+
+	BinaryTreeNode* preserved_pointer;
+	if (bottom_walker == NULL) {
+		// root node
+		preserved_pointer = self->root;
+		_BinaryTreeNode_UnlinkNode(&self->root);
+		self->walker_stack.reserved = 0;
+	} else {
+		BinaryTreeNode* parent_node = bottom_walker->node;
+		switch (bottom_walker->position) {
+			case BINARYTREENODEPOSITION_SELF:
+				return EXIT_FAILURE;
+
+			case BINARYTREENODEPOSITION_LEFT:
+				preserved_pointer = parent_node->left;
+				_BinaryTreeNode_UnlinkNode(&parent_node->left);
+				break;
+
+			case BINARYTREENODEPOSITION_RIGHT:
+				preserved_pointer = parent_node->right;
+				_BinaryTreeNode_UnlinkNode(&parent_node->right);
+				break;
+                }
+
+		bottom_walker->position = BINARYTREENODEPOSITION_SELF;
+        }
+
+	_BinaryTree_DestroyNode(self, preserved_pointer);
+
+	return EXIT_SUCCESS;
+}
+
 int _GrowParentStack(BinaryTree* tree)
 {
 	size_t depth = _DepthOf(tree->root);
@@ -187,8 +300,22 @@ static void _Rebalance(BinaryTree* tree)
 				break;
 
 			case BINARYTREENODEPOSITION_SELF:
-				// not gonna happen
+				if (i == 0) {
+					nodep = &tree->root;
+				} else {
+					BinaryTreeWalker* parent_walker = DynamicArray_GetPointer(
+						&tree->walker_stack,
+						i - 1
+					);
+					if (parent_walker->position == BINARYTREENODEPOSITION_LEFT) {
+						nodep = &parent_walker->node->left;
+					} else {
+						// right, self is not allowed
+						nodep = &parent_walker->node->right;
+					}
+				}
 				break;
+
 		}
 
 		_RebalanceNode(nodep);
@@ -198,14 +325,8 @@ static void _Rebalance(BinaryTree* tree)
 	_RebalanceRoot(tree);
 }
 
-static int _BinaryTree_DestroyNode(BinaryTree* tree, BinaryTreeNode* node)
+int BinaryTree_Insert(BinaryTree* tree, void* value)
 {
-	Allocator_Free(tree->allocator, node, sizeof(*node) + tree->value_size);
-
-	return EXIT_SUCCESS;
-}
-
-int BinaryTree_Insert(BinaryTree* tree, void* value) {
 	if (tree == NULL) return EDESTADDRREQ;
 	if (value == NULL) return EINVAL;
 
@@ -216,7 +337,7 @@ int BinaryTree_Insert(BinaryTree* tree, void* value) {
 	BinaryTreeNode* new_node = _CreateNode(tree, value);
 	if (new_node == NULL) return ENOMEM;
 
-	if (_Insert(tree, new_node)) {
+	if (_BinaryTree_Insert(tree, new_node)) {
 		_BinaryTree_DestroyNode(tree, new_node);
 		return EXIT_FAILURE;
 	}
@@ -225,6 +346,21 @@ int BinaryTree_Insert(BinaryTree* tree, void* value) {
 	return EXIT_SUCCESS;
 }
 
+int BinaryTree_Remove(BinaryTree* self, void* value)
+{
+	if (self == NULL) return EDESTADDRREQ;
+	if (value == NULL) return EINVAL;
+
+	int remove_code = _BinaryTree_Remove(self, value);
+	if (remove_code) {
+		return remove_code;
+	}
+
+	_Rebalance(self);
+
+	return EXIT_SUCCESS;
+}
+
 int BinaryTree_ForEachPostOrder(BinaryTree* tree, BinaryTreeNodeFunction action, void* context)
 {
 	tree->walker_stack.reserved = 0;

src/BinaryTree/BinaryTree.h

@@ -13,7 +13,7 @@ typedef struct BinaryTreeNode_s {
 	char value[];
 } BinaryTreeNode;
 
-typedef int (*BinaryTreeComparator) (void* this, void* other, void* xdata);
+typedef int (*BinaryTreeComparator) (void* self, void* other, void* xdata);
 
 typedef struct BinaryTree_s {
 	BinaryTreeNode* root;

src/errorcodes.h

@@ -42,6 +42,11 @@
 #define ENOMEM 12
 #endif
 
+#ifndef EEXIST
+// File/Resource exists already
+#define EEXIST 17
+#endif
+
 #ifndef ECANCELED
 // Not enough memory
 #define ECANCELED 132

tests/BinaryTree.test.c

@@ -1,5 +1,6 @@
 #include <assert.h>
 #include <stdlib.h>
+#include <stdio.h>
 
 #include "../src/BinaryTree/BinaryTree.h"
 
@@ -35,6 +36,15 @@ void testLifetime(void)
 	return;
 }
 
+int _PrintDoubleNode(void* context, BinaryTreeNode* node)
+{
+	(void) context;
+	for (size_t i = 1; i != node->depth; i++) putchar('\t');
+	printf("%.0f\n", *(double*) node->value);
+
+	return EXIT_SUCCESS;
+}
+
 void testInsert(void)
 {
 	allocator_t allocator;
@@ -54,6 +64,76 @@ void testInsert(void)
 	assert(EXIT_SUCCESS == 	BinaryTree_Insert(&tree, &value));
 	assert(tree.root != NULL);
 
+	value = 3;
+	assert(EXIT_SUCCESS == 	BinaryTree_Insert(&tree, &value));
+	assert(*(double*) tree.root->left->value == value);
+
+	value = 2;
+	assert(EXIT_SUCCESS == 	BinaryTree_Insert(&tree, &value));
+	assert(*(double*) tree.root->value == 3);
+	assert(*(double*) tree.root->left->value == 2);
+	assert(*(double*) tree.root->right->value == 5);
+
+	value = 4;
+	assert(EXIT_SUCCESS == 	BinaryTree_Insert(&tree, &value));
+	assert(*(double*) tree.root->value == 3);
+	assert(*(double*) tree.root->left->value == 2);
+	assert(*(double*) tree.root->right->value == 5);
+	assert(*(double*) tree.root->right->left->value == 4);
+
+	value = 7;
+	assert(EXIT_SUCCESS == 	BinaryTree_Insert(&tree, &value));
+	assert(*(double*) tree.root->value == 3);
+	assert(*(double*) tree.root->left->value == 2);
+	assert(*(double*) tree.root->right->value == 5);
+	assert(*(double*) tree.root->right->left->value == 4);
+	assert(*(double*) tree.root->right->right->value == 7);
+
+	value = 6;
+	assert(EXIT_SUCCESS == 	BinaryTree_Insert(&tree, &value));
+	assert(*(double*) tree.root->value == 5);
+	assert(*(double*) tree.root->left->value == 3);
+	assert(*(double*) tree.root->right->value == 7);
+	assert(*(double*) tree.root->right->left->value == 6);
+	assert(*(double*) tree.root->left->left->value == 2);
+	assert(*(double*) tree.root->left->right->value == 4);
+
+	value = 8;
+	assert(EXIT_SUCCESS == 	BinaryTree_Insert(&tree, &value));
+	assert(*(double*) tree.root->value == 5);
+	assert(*(double*) tree.root->left->value == 3);
+	assert(*(double*) tree.root->right->value == 7);
+	assert(*(double*) tree.root->right->left->value == 6);
+	assert(*(double*) tree.root->right->right->value == 8);
+	assert(*(double*) tree.root->left->left->value == 2);
+	assert(*(double*) tree.root->left->right->value == 4);
+
+
+	BinaryTree_Destroy(&tree);
+		assert(allocator.reserved == 0);
+
+	return;
+}
+
+void testRemoval(void)
+{
+	allocator_t allocator;
+	Allocator_CreateSystemAllocator(&allocator);
+	BinaryTree tree;
+
+	assert(EXIT_SUCCESS == BinaryTree_Create(
+		&tree,
+		(BinaryTreeComparator) doublecomparator,
+		(void*) 0xDEADBEEF,
+		sizeof(double),
+		&allocator)
+	);
+
+	double value = 5;
+
+	assert(EXIT_SUCCESS == 	BinaryTree_Insert(&tree, &value));
+	assert(tree.root != NULL);
+
 	value = 3;
 	assert(EXIT_SUCCESS == 	BinaryTree_Insert(&tree, &value));
 
@@ -63,19 +143,14 @@ void testInsert(void)
 	value = 4;
 	assert(EXIT_SUCCESS == 	BinaryTree_Insert(&tree, &value));
 
+	value = 5;
+	assert(EXIT_FAILURE == 	BinaryTree_Insert(&tree, &value));
 
-	value = 7;
+	assert(EXIT_SUCCESS == BinaryTree_Remove(&tree, &value));
-	assert(EXIT_SUCCESS == 	BinaryTree_Insert(&tree, &value));
+	assert(EXIT_SUCCESS == BinaryTree_Insert(&tree, &value));
-
-	value = 6;
-	assert(EXIT_SUCCESS == 	BinaryTree_Insert(&tree, &value));
-
-	value = 8;
-	assert(EXIT_SUCCESS == 	BinaryTree_Insert(&tree, &value));
-
 
 	BinaryTree_Destroy(&tree);
-		assert(allocator.reserved == 0);
+	assert(allocator.reserved == 0);
 
 	return;
 }
@@ -117,10 +192,36 @@ void testEqual(void)
 	return;
 }
 
+void testMany(void)
+{
+	allocator_t allocator;
+	Allocator_CreateSystemAllocator(&allocator);
+	BinaryTree tree;
+
+	assert(EXIT_SUCCESS == BinaryTree_Create(
+		&tree,
+		(BinaryTreeComparator) doublecomparator,
+		(void*) 0xDEADBEEF,
+		sizeof(double),
+		&allocator)
+	);
+
+	for (double value = 0; value < 300; value++) {
+		assert(EXIT_SUCCESS == 	BinaryTree_Insert(&tree, &value));
+	}
+
+	BinaryTree_Destroy(&tree);
+	assert(allocator.reserved == 0);
+
+	return;
+}
+
 int main()
 {
 	testLifetime();
 	testInsert();
 	testEqual();
+	testMany();
+	testRemoval();
 	return 0;
 }