1

This program is a binary search tree of string to store information of students with the following details such as id, name and CGPA. And using unique id of a student to search, delete and insert different data. But in deletion when a subtree is involved the tree gets separated, I don't know what I'm doing wrong.

#include <iostream>
using namespace std;

struct BstNode
{
    string iD;
    string name;
    float cgpa;
    BstNode *left;
    BstNode *right;
};
BstNode *root;
BstNode *GetNewNode(string iD0, string n0, float cg0)
{
    BstNode *NewNode = new BstNode();
    NewNode->iD = iD0;
    NewNode->name = n0;                                         
    NewNode->cgpa = cg0;
    NewNode->left = NULL;
    NewNode->right = NULL;
    return NewNode;
}

void PreOrder(BstNode *root)
{
    if (root == NULL)
    {
        return;
    }
    cout << "ID: " << root->iD << "\nNAME: " << root->name << "\nCGPA: " << root->cgpa << endl;
    PreOrder(root->left);
    PreOrder(root->right);
}
void InOrder(BstNode *root)
{
    if (root == NULL)
    {
        return;
    }
    InOrder(root->left);
    cout << "ID: " << root->iD << "\nNAME: " << root->name << "\nCGPA: " << root->cgpa << endl;
    InOrder(root->right);
}
void PostOrder(BstNode *root)
{
    if (root == NULL)
    {
        return;
    }
    PostOrder(root->left);
    PostOrder(root->right);
    cout << "ID: " << root->iD << "\nNAME: " << root->name << "\nCGPA: " << root->cgpa << endl;
    cout << endl;
}
BstNode *Insert(BstNode *root, string iD2, string n2, float cg2)
{
    if (root == NULL)
    {
        root = GetNewNode(iD2, n2, cg2);
    }
    else if (iD2 <= root->iD)
    {
        root->left = Insert(root->left, iD2, n2, cg2);
    }
    else
    {
        root->right = Insert(root->right, iD2, n2, cg2);
    }
    return root;
}

BstNode *Rectify(BstNode *root, string id1, string n, float cg)
{
    root->iD = id1;
    root->name = n;
    root->cgpa = cg;
    return root;
}
struct BstNode *FindMin(BstNode *root)
{
    struct BstNode* current = root;
 
    /* loop down to find the leftmost leaf */
    while (current && current->left != NULL)
        current = current->left;
 
    return current;
}

BstNode *Delete(BstNode *root, string data)
{
    if (root == NULL)
        return root;
    else
    {
        if (data < root->iD)
            root->left = Delete(root->left, data);

        else if (data > root->iD)
            root->right = Delete(root->right, data);
        else
        {
            if (root->left == NULL and root->right == NULL)
                return NULL;

            else if (root->left == NULL)
            {
               struct BstNode *temp = root->right;
                delete(root);
                return temp;
            }
            else if (root->right == NULL)
            {
               struct BstNode *temp = root->left;
                delete(root);
                return temp;
            }
            else
            {
               struct BstNode *temp = FindMin(root->right);
                root->iD = temp->iD;
                root->name = temp->name;
                root->cgpa = temp->cgpa;
                root->right = Delete(root->right, temp->iD);
            }
        }
        return root;
    }
}

    BstNode *Search(BstNode * root, string iDs)
    {
        cout << "Finding " << endl;
        if (root == NULL)
        {
            cout << "No such info!" << endl;
            return root;
        }
        else if (root->iD == iDs)
        {
            cout << "Found 0 " << endl;
            cout << "ID: " << root->iD << endl;
            cout << "NAME: " << root->name << endl;
            cout << "CGPA: " << root->cgpa << endl;
            bool done = false;
            while (!done)
            {
                cout << "Would you like to update this student's information? [y/n]\n";
                string x;
                cin >> x;
                if (x == "y")
                {
                    cout << "The following fields are updatable:\n";
                    cout << "1. Student ID\n";
                    cout << "2. Student Name\n";
                    cout << "3. Student CGPA\n";
                    cout << "4. Remove Student\n";
                    cout << "Enter your choice:\n";
                    bool done1 = false;
                    while (!done1)
                    {
                        int ch;
                        cin >> ch;
                        if (ch == 1)
                        {
                            cout << "Please enter new ID:   \n";
                            string nwid;
                            cin >> nwid;
                            cout << "============================" << endl;
                            cout << "Updating  \n";
                            cout << "============================" << endl;
                            root = Rectify(root, nwid, root->name, root->cgpa);
                            cout << "============================" << endl;
                            cout << "Updated Successfully!  \n";
                            cout << "============================" << endl;
                            cout << "============================" << endl;
                            cout << root->iD << endl;
                            cout << root->name << endl;
                            cout << root->cgpa << endl;
                            cout << "============================" << endl;
                            done1 = true;
                        }
                        else if (ch == 2)
                        {
                            cout << "Please enter new name:   \n";
                            string nwname;
                            cin >> nwname;
                            cout << "============================" << endl;
                            cout << "Updating  \n";
                            cout << "============================" << endl;
                            root = Rectify(root, root->iD, nwname, root->cgpa);
                            cout << "============================" << endl;
                            cout << "Updated Successfully!  \n";
                            cout << "============================" << endl;
                            cout << "============================" << endl;
                            cout << root->iD << endl;
                            cout << root->name << endl;
                            cout << root->cgpa << endl;
                            cout << "============================" << endl;
                            done1 = true;
                        }
                        else if (ch == 3)
                        {
                            cout << "Please enter new CGPA:   \n";
                            float nwcg;
                            cin >> nwcg;
                            cout << "============================" << endl;
                            cout << "Updating  \n";
                            cout << "============================" << endl;
                            root = Rectify(root, root->iD, root->name, nwcg);
                            cout << "============================" << endl;
                            cout << "Updated Successfully!  \n";
                            cout << "============================" << endl;
                            cout << "============================" << endl;
                            cout << root->iD << endl;
                            cout << root->name << endl;
                            cout << root->cgpa << endl;
                            cout << "============================" << endl;
                            done1 = true;
                        }
                        else if (ch == 4)
                        {
                            cout << "Removing\n";
                           root = Delete(root, root->iD);
                            cout << "Removed!\n";
                            return root;
                            done1 = true;
                        }
                        else
                        {
                            cout << "Wrong input try again!  \n";
                        }
                    }
                    done = true;
                }
                else if (x == "n")
                {
                    done = true;
                }
                else
                {
                    cout << "Wrong input try again!  \n";
                }
            }

            return root;
        }
        else if (iDs <= root->iD)
        {
            return Search(root->left, iDs);
        }
        else
        {
            return Search(root->right, iDs);
        }
    }

    int main()
    {
        root = NULL;
        root = Insert(root, "21-44631-1", "Rafiul", 3.75);
        root = Insert(root, "21-44531-1", "Haque", 4.0);
        root = Insert(root, "21-44431-1", "Rafsun", 3.78);
        root = Insert(root, "21-44331-1", "Rafi", 3.5);
        root = Insert(root, "21-44231-1", "ABC", 4.0);
        root = Insert(root, "21-44611-1", "AC", 4.0);
        cout << "Please enter the ID you want to search: ";
        string s;
        cin >> s;
        cout << endl;
        Search(root, s);
        cout << "Reading tree in preorder : \n";
        PreOrder(root);
        cout<<"============================"<< endl;
        cout << "\nReading tree in inorder : \n";
        InOrder(root);
        cout<<"============================"<< endl;
        cout << "\nReading tree in postorder : \n";
        PostOrder(root);
        cout << "============================" << endl;
    }
  • 1
    Off-topic: If this is excercise, OK, otherwise be aware that `std::map` already is a BST, implemented as efficient red-black-tree... And: about [`using namespace std`](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice)... – Aconcagua May 05 '22 at 07:15
  • 1
    Sit down with a debugger to step through the code statement by statement while monitoring variables and their values. As you step through the code, draw out the tree you create and modify using pencil and paper. When you make a change in the tree, erase and redraw nodes and links on the paper. Sooner or later you will come to a point where your drawing just doesn't make sense anymore, and you need to take a closer look at the code that caused it. – Some programmer dude May 05 '22 at 07:16
  • 1
    This is C++ – `GetNewNode` should rather be a constructor. And you should prefer C++ *keywords* (`nullptr`) over old (obsolete) C *macros* (`NULL`). – Aconcagua May 05 '22 at 07:17
  • 1
    Belaying the items having nothing to do with your problem, there are two red flags I see. 1. the isolated node (e.g. no children) deletion leaks memory. You return NULL, but never deleted the actual node. 2. Your subtree relocation logic in the case of a two-child victim is flawed from inception because you're doing field-by-field transference, and orphaning the actual node thereafter. Also, once again, you don't delete the actual victim node. – WhozCraig May 05 '22 at 07:24
  • You do not need that initial check for both children being null; if left is null then you return right – if that's null as well, fine, you return null as you would have with that initial check anyway... Same would apply for the right child – even though after first check it is clear that it isn't any more... – Aconcagua May 05 '22 at 07:32
  • Side note: In case of both children existing finding minimum and then deleting it requires you to iterate the tree twice. Sub-optimal... – Aconcagua May 05 '22 at 07:34
  • I'd rather recommend an iterative approach; recursive call is not the very last thing you do, so you cannot tail-call-optimise, i.e. stack grows with every recursion. I'd first look for the node to delete, then continue iterating towards the minimum while at the same time remembering the *parent* of the minimum, so an additional pointer. Then you can simply adjust the parent's left child to the minimum's right one without having to iterate again. – Aconcagua May 05 '22 at 07:39
  • Alternatives: Let `findMin` return the parent as well (`std::pair` as return value or an output paremeter) or let the function have an additional handler function parameter, defaulting to nullptr, that could do the adjustment right when the child is found. – Aconcagua May 05 '22 at 07:42

1 Answers1

1

Deleting a BST node from a simple (e.g. non-self-balancing) implementation involves three things:

  1. Find the node you're deleting. More specifically, find the pointer in the tree (which may be the root pointer) that points to the node you want to delete.

  2. Once found, promote one of the children to occupy the vacancy about to be left by the node being deleted.

  3. Hang the opposite child (if there even is one) in the left (or right, depending on which child you promoted in (2), position possible of the promoted child.

Note that it is possible to cover all four node+children cases (no children, only a left child, only a right child, or both), using a single algorithm, especially when using a pointer-to-pointer to iterate the tree. It is done like this (liberally commented to try and explain what is going on):

BstNode *Delete(BstNode *root, string data)
{
    BstNode **pp = &root;

    // find the node we want to delete by id
    while (*pp)
    {
        if (data < (*pp)->iD)
            pp = &(*pp)->left;
        
        else if ((*pp)->iD < data)
            pp = &(*pp)->right;
        
        else // equivalent. fount it
            break;
    }

    // if this is non-null, we found the pointer in the
    //  tree that points to the node we want to delete.
    //  note that this could even be the root pointer
    //  where we started.
    if (*pp)
    {
        // remember the node. we're about to overwrite
        //  the tree pointer that is referring to it.
        BstNode *victim = *pp;

        // always promote the right node.
        *pp = victim->right;

        // now iterate down the opposite side of the
        //  note we just promoted, looking for the first
        //  null slot on which we can hang our sibling
        //  (if there even is one; it's ok if there isn't)
        while (*pp)
            pp = &(*pp)->left;

        // this is where we hang the sibling node
        *pp = victim->left;

        // the tree is fixed up and the node excised. we
        //  can delete it now.
        delete victim;
    }

    // return this. remember, root may have been pointing
    //  to the node we deleted, and if so, it has changed
    //  so we need to inform the caller by return result.
    return root;
}

Sans the overt comments explaining what is happening, it is a surprisingly small amount of code, especially when you consider what it is accomplishing.

WhozCraig
  • 65,258
  • 11
  • 75
  • 141
  • Interesting... I'm rather used to fetching children upwards as the question author obviously tried. Not verified, but I imagine this variant leading to less balanced trees, though. Well, sure, if *that* is an issue, then we'd swap to better tree algorithms (AVL, red/black, ...) anyway ;) – Aconcagua May 05 '22 at 10:35
  • @Aconcagua Oh, it *absolutely* leads to misbalanced trees. It is, however, the absolute-simplest algorithm I know of for keeping the ordering correct. Were I concerned about subtree order (in the 'depth' vernacular), I'd do what you and the OP described, but further it by rotating it into position about its respective subtree, then hang the sibling. The main reason, when learning deletion, I first teach the method above is, pictorially, the above algorithm is super easy to visualize and understand. I'd obviously do *none* of this in RL code, opting for a standard map instead. – WhozCraig May 05 '22 at 16:39