0

I am implementing a binary search tree in C ++, but the code has an error, when deleting a node with 2 children the resulting tree is not well connected, the method to delete a node with 0 or 1 child works well. This code is based in this answer: https://stackoverflow.com/a/31698320/11053027 .

Edit: For example if a add the values in this order: 17, 12, 25, and then I try delete 17 that is the root, it is deleted, so when I try to show all the elements are: 12, 25, with 25 as root. But if now I try to delete 12 it is not deleted, the problem is caused when I first deleted 17. No error message is shown.

Can you help me please. Thanks in advance. Here is the code:

    Node<E> *getSuccesor(Node<E> &node) const {
       auto *Succesor = node.right;
       while (Succesor != nullptr) {
          if (Succesor->left == nullptr) return Succesor;
          Succesor = Succesor->left;
       }
       return Succesor;
    }

    void remove(Node<E> &node) {
       if (&node == nullptr) return;
       int nChildren = numChildren(node);
       Node<E> *child;

       if (nChildren == 2) {
        child = getSuccesor(node);
        remove(*child);
        child->parent = node.parent;
        child->left = node.left;
        child->right = node.right;
        if (&node == root)
            root = child;
        else {
            if (&node == node.parent->right)
                node.parent->right = child;
            else node.parent->left = child;
        }
    } else {
        child = (node.left != nullptr ? node.left : node.right);

        if (child != nullptr)
            child->parent = node.parent;

        if (&node == root)
            root = child;
        else {
            if (&node == node.parent->left)
                node.parent->left = child;
            else node.parent->right = child;
        }
    }
    Size--;
 }
Alexander
  • 25
  • 4
  • 1
    What's the exact error message? Care to elaborate in your [question](https://stackoverflow.com/questions/18348620/call-function-with-arguments-without-parenthesis-in-c) please. Also make your code a [mcve]. – πάντα ῥεῖ Sep 07 '20 at 23:17

1 Answers1

0

It's hard to be certain, since you haven't given us a minimal complete example, but I think the problem is here:

if (nChildren == 2) {
  child = getSuccesor(node);
  remove(*child);
  child->parent = node.parent;
  child->left = node.left;
  child->right = node.right;

So now child knows its children. But they don't know that child is now their parent. Their parent pointers still point to the defunct node (in this case, '17'), which messes up the logic in the next removal.

After those lines, add these:

if(child->left)
  child->left->parent = child;
if(child->right)
  child->right->parent = child;

EDIT: The same bug is also in the other branch of the if-else conditional, but I'm sure you can see how to fix it there too.

Beta
  • 96,650
  • 16
  • 149
  • 150
  • Thank you, this worked, just one last question, to completely remove that node at the end of the function do I have to do: delete node.element; ? And do nothing with node.left, node.right and node.parent, right? – Alexander Sep 08 '20 at 15:26
  • @Alexander: [I don't know what `element` is, and I don't know how you constructed the nodes in the first place](https://stackoverflow.com/help/minimal-reproducible-example), but yes, you must dispose of that node somehow. – Beta Sep 08 '20 at 16:39