0

I am using Recursion to search an element in my Binary Search Tree but my code stops working if the element is not present in the BST.

void tree::searching(node *root,int key)
{
    if(root->key==key||root==NULL)
    {
       cout<<"Congratulation Element found in the BST"<<"\n";
       return;
    } else {
        if(key<root->key)
        {
           searching(root->left,key);
        } else {
           searching(root->right,key);
        }
    }
}
1
  • Your check for a null-pointer root is wrong. Commented Oct 19, 2018 at 17:27

3 Answers 3

2

You're dereferencing a NULL pointer here:

if(root->key==key||root==NULL)
{
    cout<<"Congratulation Element found in the BST"<<"\n";
    return;
}

The || operator evauates the left side first, and if it is value then it evaluates the right side. So you dereference root before checking if it is NULL.

Do the NULL check first and return if you find a NULL pointer:

void tree::searching(node *root,int key)
{
    if (root == nullptr) {
        return;
    }

    if(root->key==key) {
        cout<<"Congratulation Element found in the BST"<<"\n";
    } else if(key<root->key)
        searching(root->left,key);
    } else {
        searching(root->right,key);
    }
}
Sign up to request clarification or add additional context in comments.

5 Comments

I think, it's worth returning from successful branch as well, as in your case, recursion will just keep happening until root == nullptr will happen.
@JohnCvelth An explicit return is superfluous in this case since the program will flow right to the end of the function after the element is found.
@JohnCvelth On a key match the other else-if and else are not executed, and there is no code after that. In short, it already does return immediately after the match.
Ticked. Fixes both problems (the null pointer dereference and the broken logic announcing a find on every non-matching terminating leaf node). The only thing I would change is the requirement of both operator < and operator == for key comparisons. if (a < b) ... else if (b < a) ... else matched One operator. Regardless, nice.
@WhozCraig, oops, yes. I was inattentive.
2

The problem

If root would be nullptr in this statement:

    if(root->key==key||root==NULL)

you would first dereference a null pointer with root->key, which is UB, before checking if it's NULL.

The solution

Do it the other way round:

    if(root==nullptr||root->key==key)

In this case, if root is NULL, the the if clause is immediately executed. Only if root would be not NULL, would the pointer be dereferenced.

Note: you tell that the element was found even if the element is not found (i.e. root reached a nullptr, without ever having encountered the correct key). Consider having distinct case for nullptr (means that it was not found), and equality (means that it was found).

Comments

0

You are printing too early. If the programs go to a leaf it will print because the expression in the fist if will be evaluated to true.

void tree::searching(node *root,int key)
{
    if (root == nullptr)
    {
        return;
    }
    if(root->key==key)
    {
        cout<<"Congratulation Element found in the BST"<<"\n";
        return;
    }
    else
    {
        if(key<root->key)
        {
            searching(root->left,key);
        }
        else
        {
            searching(root->right,key);
        }
    }

 }

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.