0
Node* search(Node* &root, int data) {
    if (root == NULL) {
        cout << "key not found\n";
        return NULL;
    }
    if (root -> key == data)
        return root;
    else if(root-> key > data)
        return search(root -> left, data);
    else
        return search(root -> right,data);
}

Node* findMinVal(Node* &root){
    Node* current = root;
    while (current -> left != NULL)
        current = current -> left;
    return current;
}

void deleteNode(Node* &root, int key) {
    if (root == NULL)
        return;
    Node* node = search(root, key);
    if (node -> left == NULL) {
        Node* temp = node;
        node = node -> right;
        free(temp);
    }
    else if (node -> right == NULL) { 
        Node* temp;
        node = node -> left;
        free(temp);
    }
    else {
        Node* temp = findMinVal(node -> right);
        node -> key = temp -> key;
        free(temp);
    }
}

example tree:

Inorder traversal of the given tree:

20 30 40 50 60 70 80 

output and commands :

Delete 20
Inorder traversal of the modified tree 
0 30 40 50 60 70 80 
Robert
  • 7,394
  • 40
  • 45
  • 64
rv7
  • 37
  • 5
  • Concerning `void deleteNode()`: `Node *node` is a local variable. E.g. `node = node -> right;` doesn't replace the `root`, or `node->left` or `node->right` of the parent node. It just modifies the local pointer `node`. (This is the most prominent I found in a first glance.) – Scheff's Cat Nov 05 '19 at 16:04
  • Thanks for the quick reply. but how can i rectify this issue of local variable – rv7 Nov 05 '19 at 16:07
  • You don't need the pointer but you need the reference of the pointer. I just thought about how to change the `search()`. Returning the reference to pointer is dangerous because you cannot return a useful reference if `search()` didn't found something. – Scheff's Cat Nov 05 '19 at 16:09
  • A pointer to pointer could help (`Node**`). It still can return a `nullptr` if `search()` didn't find the key. Otherwise, you get the pointer to pointer which has to be updated. – Scheff's Cat Nov 05 '19 at 16:12
  • okay but will this also rectify my main problem where instead of deleting the node . it assigns a random or 0 value – rv7 Nov 05 '19 at 16:14
  • okay i got that u want me to change return type from node* to node* * – rv7 Nov 05 '19 at 16:16
  • I guess so. If I understood it right (by debugging by eyes), you `free()` the node but leave it in the tree (by not overriding it). That's U.B. and good for any strange effects. – Scheff's Cat Nov 05 '19 at 16:17
  • thanks i ll try . btw what is U.B. ? – rv7 Nov 05 '19 at 16:18
  • I would propose another solution: Make `deleteNode()` recursive as well (similar like `search()` and use a reference to pointer (as you already did) but for the current node pointer. I once wrote a sample for balanced removal that's even more complicated but, may be, it helps: [SO: c++ delete specific node with two children from binary search tree](https://stackoverflow.com/a/48826108/7478597) – Scheff's Cat Nov 05 '19 at 16:19
  • U.B. -> Undefined Behavior (what C and C++ programmers fear/hate most). ;-) (A granted crash would be a mercy.) [**U.B.**](https://stackoverflow.com/a/4105123/1505939) – Scheff's Cat Nov 05 '19 at 16:20
  • i have a doubt . if i store the result of search like this Node* node = search() , node will have the address of node returned from search . so why does it matter if node is a local variable we are changing everything on address level – rv7 Nov 05 '19 at 16:29
  • Because, with `node = node->left;` or `node = node->right;`, you override the local variable but not the original pointer (which might be either `root` or `left` or `right` from another node). Additionally, you make a copy of `node` before -> `Node* temp = node;` and later `free(temp);`. Now, you have a dangling pointer somewhere in the tree... – Scheff's Cat Nov 05 '19 at 16:32
  • so by this Node* node = search() ; it doesn't mean that i m copying address of node from orignal tree that i got from search in the node variable ?? i am just copying its content ?? – rv7 Nov 05 '19 at 16:35
  • Remember: a pointer is a value itself. A variable with a pointer is a variable with a value which is an address of something. Copying a pointer variable into another (by assignment), you have two variables with that value (both the address of same something). If you now change one of the variables, it doesn't change something, nor does it change the other variable. This is in general what I tried to explain... – Scheff's Cat Nov 05 '19 at 16:36
  • what do you mean by dangling pointer ?? – rv7 Nov 05 '19 at 16:36
  • as you said both have address of same something so why editing one of them can't change the other ?? – rv7 Nov 05 '19 at 16:38
  • A dangling pointer is a pointer with an address which has been released (e.g. with `free()` or `delete`). The pointer seems to point to something but actually something does not exist anymore. So, the pointer is dangling (sometimes called wild pointer as well). Accessing the pointee is U.B. (It could crash your program or it could seem to work or it could cause any other strange effect ... like e.g. providing a 0 where it's not expected.) – Scheff's Cat Nov 05 '19 at 16:39
  • Because a pointer variable doesn't contain "something" but the _address_ of "something". Modifying the variable is changing the _address_ (e.g. assign another address). This doesn't change "something", nor does it change another variable which (incidentally or intentionally) has the same _address_ of "something". – Scheff's Cat Nov 05 '19 at 16:40
  • okay thanks i got this now . i am actually changing address of a local variable . and doing nothing else – rv7 Nov 05 '19 at 16:43
  • Nope. The local variable `node` contains the address of your found node. Then, you assign the address of another node to this local var. `node`. But the address of found node is still stored either in `root` or in `left` or `right` of its parent node. These are not affected by assigning anything to local variable `node`. Even more worse: Before assigning `node`, you made a copy of the pointer `Node *temp = node;` Then you `free(temp);`. But still the address of the free'd node is stored either in `root` or in `left` or `right` of its parent node. So, this one just has been made dangling. – Scheff's Cat Nov 05 '19 at 16:48
  • okay i got it now thanks for the help – rv7 Nov 05 '19 at 17:25

0 Answers0