c icon indicating copy to clipboard operation
c copied to clipboard

Binary Search Tree: Test root node with value == 0 is allowed

Open annoj opened this issue 3 years ago • 2 comments

In my attempt to solve the Binary Search Tree exercise, I first came up wit a solution that would have overwritte it's root node when building a tree if the value of the root node was 0. This behaviour was not cought by the provided tests, but by @siebenschlaefer, who did a great job providing mentoring and feedback.

I would suggest to add a test for the tree to allow zero as the value of the root node to the testsuite. I have already written such a test for the C track and am happy to open a PR to add it.

in case you deem this an issue applicable to all language tracks, I'm happy to also open a PR in https://github.com/exercism/problem-specifications.

The test I propose to add is as follows:

static void test_data_can_have_zero_as_first_node(void)
{
    int tree_data[] = { 0, 0, 1, 2, 3 };
    node_t *tree = build_tree(tree_data, ARRAY_SIZE(tree_data));

    TEST_ASSERT_NOT_NULL(tree);
    TEST_ASSERT_EQUAL_INT(0, tree->data);
    TEST_ASSERT_NOT_NULL(tree->left);
    TEST_ASSERT_NOT_NULL(tree->right);

    TEST_ASSERT_EQUAL_INT(0, tree->left->data);
    TEST_ASSERT_NULL(tree->left->left);
    TEST_ASSERT_NULL(tree->left->right);

    TEST_ASSERT_EQUAL_INT(1, tree->right->data);
    TEST_ASSERT_NULL(tree->right->left);
    TEST_ASSERT_NOT_NULL(tree->right->right);

    TEST_ASSERT_EQUAL_INT(2, tree->right->right->data);
    TEST_ASSERT_NULL(tree->right->right->left);
    TEST_ASSERT_NOT_NULL(tree->right->right->right);

    TEST_ASSERT_EQUAL_INT(3, tree->right->right->right->data);
    TEST_ASSERT_NULL(tree->right->right->right->left);
    TEST_ASSERT_NULL(tree->right->right->right->right);

    free_tree(tree);
}

annoj avatar Aug 07 '22 01:08 annoj

@annoj sorry for the delay in responding. Can you help me understand what's special about 0 as the first node? How did your implementation fail?

ryanplusplus avatar Aug 10 '22 00:08 ryanplusplus

@ryanplusplus in my first implementation I used the following implementation for build_tree():

static void _insert_node(node_t* tree, int value)
{
    if (!tree->data) {
        tree->data = value;

        return;
    }

    if (value <= tree->data) {
        if (!tree->left) {
            tree->left = calloc(1, sizeof(node_t));
            tree->left->data = value;
        } else {
            _insert_node(tree->left, value);
        }
    } else {
        if (!tree->right) {
            tree->right = calloc(1, sizeof(node_t));
            tree->right->data = value;
        } else {
            _insert_node(tree->right, value);
        }
    }
}

node_t *build_tree(int *tree_data, size_t tree_data_len)
{
    node_t* tree = calloc(1, sizeof(node_t));

    for (size_t i = 0; i < tree_data_len; i++) {
        _insert_node(tree, tree_data[i]);
    }

    return tree;
}

In the first line of _insert_node() I would check whether tree->data is not null (which is obviously a mistake). This was not cought by any of the tests.

annoj avatar Aug 13 '22 15:08 annoj