Search code examples
cdebuggingbinary-treebinary-search-tree

Why does my binary tree deletion remove entire left portion of tree?


I have an assignment in which i need to make an implementation of a binary search tree in C. In my attempt of implementing a delete function for the tree i have failed to make an implementation that doesn't delete the entire (or almost entire) tree, if the node to be deleted is on the left side. This is the function i have written:

btree_node *btree_remove(const int x, btree_node *root) {
  //if the value cant be found return null;
   if (root == NULL) return root;
  //search for the node to be deleted
   if (x < root->data) {
    root->left = btree_remove(x, root->left);
   }
   
   if (x > root->data){
    root->right = btree_remove(x, root->right);
   }
   else {
    //case when there is no child
     if ((root->left == NULL) && (root->right == NULL)) {
      free(root);
       return NULL;
     }
     //case when it has one child
     else {if (root->right == NULL){
        btree_node* temp = root->left;
        free(root);
        return temp;
      }
      if (root->left == NULL) {
        btree_node* temp = root->right;
        free(root);
        return temp;
      }
     }
      
      //case when it has two children
      if((root->left != NULL) && (root->right !=NULL)){
        btree_node* minNode = findMin(root->right);
        root->data = minNode->data;
        root->right = btree_remove(minNode->data, root->right);
        return root;
      }
   }
 }

A binary tree i have made using an insert function looks as such:

   10
  /  \
 5    17
/ \
2  NULL

When i then try to delete the node (5) the expected outcome is:

      10
     /  \
    2    17
   / \
NULL  NULL

The outcome using this function is:

      17
     /  \
    2    NULL
   / \
NULL  NULL

Solution

  • You need to have an if - else if - else structure. Currently, if x < root->data is true (and indeed, 5 < 17), then the first if will evaluate to true, root->left will be assigned to 5 (after the inner btree_remove is executed and evaluated) and then x > root->data will be false (because 5 > 5 is not true) and the else will be executed erroneously.

    What you actually need is to differentiate x < root->data from x > root->data from else and, like the conditions, the blocks should be mutually exclusive too:

       if (x < root->data) {
        root->left = btree_remove(x, root->left);
       }
       
       else if (x > root->data){
        root->right = btree_remove(x, root->right);
       }
       else {
        //case when there is no child
         if ((root->left == NULL) && (root->right == NULL)) {
          free(root);
           return NULL;
         }
         //case when it has one child
         else {if (root->right == NULL){
            btree_node* temp = root->left;
            free(root);
            return temp;
          }
          if (root->left == NULL) {
            btree_node* temp = root->right;
            free(root);
            return temp;
          }
         }