BST delete - delete "tmp" causes lose of the tree

Question

Attached my code for trying to delete a node in bst.

If I want to delete node 1, when specifying tmp = del in "if (del_node->l_ == NULL)", and remove tmp, then del is removed as well, and the tree data is lost. how can I solve this issue?

Example tree:

all data members and functions are declared public for simplicity.

  void BST::DeleteNode(int data) {
      BinaryTreeNode* &del_node = BST_Search(head_, data);
      if (!del_node->l_ && !del_node->r_)
      {
          delete del_node;
          del_node = nullptr;
          return;
      }
      if (del_node->l_ == NULL)
      {
          BinaryTreeNode* tmp = del_node;
          del_node = del_node->r_;
          tmp = nullptr;
          delete tmp;
          return;
      }
      if (del_node->r_ == NULL)
      {
          BinaryTreeNode* tmp = del_node;
          del_node = del_node->l_;
          delete tmp;
          return;
      }
      else
      {
          del_node->data_ = smallestRightSubTree(del_node->r_);
      }

  }

  int BST::smallestRightSubTree(BinaryTreeNode* rightroot)
  {
      // if rightroot has no more left childs
      if (rightroot && !rightroot->l_)
      {
          int tmpVal = rightroot->data_;
          BinaryTreeNode* tmp = rightroot;
          rightroot = rightroot->r_;
          delete tmp;
          return tmpVal;
      }
      return smallestRightSubTree(rightroot->l_);
  }
  int main()
  {
      BST bst;
      bst.BST_Insert(bst.head_, 3);
      bst.BST_Insert(bst.head_, 5);
      bst.BST_Insert(bst.head_, 1);
      bst.BST_Insert(bst.head_, 2);
      bst.DeleteNode(1);
      return 0;
  }

Thanks for help! EDIT: this is how tmp and del_node look like after the line "del_node = del_node->r_)" in the condition "if(del->l = null)"

void BST::BST_Insert(BinaryTreeNode*& head, int data) {
if (head == nullptr) {
    head = new BinaryTreeNode(data, nullptr, nullptr);
    return;
}
if (data > head->data_) {
    BST_Insert(head->r_, data);
}
else {
    BST_Insert(head->l_, data);
}

}

BinaryTreeNode* BST::BST_Search(BinaryTreeNode* root, int key) {
    if (root == nullptr || root->data_ == key)
        return root;
    if (key > root->data_)
        return BST_Search(root->r_, key);
    return BST_Search(root->l_, key);
}

`tmp = nullptr; delete tmp;` Does not free up any memory. it merely forgets the value of the pointer that was in tmp. Not sure what "tree data is lost" means. You should probably take a debugger to this and observe the method in action. Or make a proper [mre] so that we could see the problem. Or both. — , Nov 01 '20 at 21:24
[traditional note about debuggers](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) — , Nov 01 '20 at 21:30
Thanks for comment. Took away the "tmp=nullptr", the thing is, that the tree is constructed as I expect. however, when Im inside of the delete method, "del_node" gets the address of node 1, "tmp" gets the same address in memory as del_node. when del_node is promoted to point its right child, the data in del_node and tmp is deleted.. (maybe wrong use of the delete operator?) Attached screenshot from the debugger in the original post. THANKS — MorMordoch, Nov 01 '20 at 21:33
If after this piece of code del_node points off into the void which could be the stack then the suspicion would fall on `BST_Search` not doing quite the right thing and you haven't shown us what's in that. The demands for a [mre] are not just to inconvenience you, there's a point to it... — , Nov 01 '20 at 21:44
Sorry! Didn't notice. Added those functions as well. Still trying to understand what is not working.. Isn't it related to the face that I delete a pointer? maybe since they hold the same address, deleting one causes the deletion of the other? I have been debugging this for the last two hours, but for some reason, the del_node and tmp are becoming null after the line of "delete tmp" — MorMordoch, Nov 01 '20 at 21:54
What does the line `BinaryTreeNode* &del_node = BST_Search(head_, data);` do when initialized with the rvalue from `BinaryTreeNode* BST::BST_Search` ? This allocates a temporary on the stack and the reference is to that temporary (I believe this should emit a warning). You probably intended it to be a reference to the original variable holding the pointer. — , Nov 01 '20 at 22:20
So your search should be taking its argument as and returning a pointer reference as well. (not sure I'm making this very clear, it is somewhat complicated) — , Nov 01 '20 at 22:22
Maybe this would be easier with `**` instead of `*&`, at least with the double-star code the compiler does not let you mistake a one-star pointer for a two-star one. — , Nov 01 '20 at 22:41

score 0 · Answer 1 · answered Nov 02 '20 at 07:59

0

If your BST_Search returns a BinaryTreeNode* by value, what is the reference in BinaryTreeNode* &del_node = BST_Search(head_, data); actually referencing? It allocates a new temporary and references that. You probably wanted it to reference the variable that is holding the pointer in the tree so that you can modify the tree.

Your BST_Search would have to look like this:

BinaryTreeNode*& BST::BST_Search(BinaryTreeNode*& root, int key) {
    if (root == nullptr || root->data_ == key)
        return root;
    if (key > root->data_)
        return BST_Search(root->r_, key);
    return BST_Search(root->l_, key);
}

I can't check whether this actually works, because you didn't provide a self-contained compilable example. But something along these lines.

answered Nov 02 '20 at 07:59

Thanks, it works! I had been debugging this fir hours! Could you explain me why the version without the ref to pointer in search didnt work? I thought thats how you return an address.. and again, thank you! – MorMordoch Nov 02 '20 at 09:05
Imagine if you wanted to delete `root` (bst.root? not sure how you named it). You want to be able to change the value of the pointer inside the `root` variable. If you search for it using the old search, you get its value (pointer to the first node). Can you use this value to assign a different pointer value to the `root` variable? You would need to get a reference to the variable to be able to do that, not just its value. – Nov 02 '20 at 09:18
Alternative view: your `del_node` was a copy. When you change it, you are changing your copy only, not the original. You need to get a reference to the original somehow. – Nov 02 '20 at 09:19

BST delete - delete "tmp" causes lose of the tree

1 Answers1