Binary search tree lookup output

Question

I am attempting to implement a lookup function for a binary search tree. While it does return true if I lookup the root of the tree, when I lookup other entries that are in the tree it returns false. When I debugged it the function seemed to return 1 but would then continue running and then return 0 at the end. It was my understanding that the function should terminate as soon as it returns any value so I'm not sure why this is happening.

int lookup(int n,struct node* x)
{
    if (x->data==n)
    {
        return 1;
    }
    else if (x->left==NULL && x->right==NULL)
    {
        return 0;
    }
    else if (n>x->data && x->right!=NULL)
    {
        lookup(n,x->right);
    }
    else
    {
        lookup(n,x->left);
    }
    return 0;
}

You probably want to write `return (lookup(n,x->right));` instead of just `lookup(n,x->right);`. — Stef, Sep 14 '20 at 12:59
Sorry, @Stef . I didn't see your comment while typing. It turns out to be the same, I did not actually copy your input however.... Feel free to make your own explained answer and we can have a race for upvotes. ;-) — Yunnosch, Sep 14 '20 at 13:06
I think I've got it now. So the "return 1" was exiting out of the recursive call of lookup but since I wasn't returning that, the first call still reached "return 0"? Both very helpful thank you. — Sam, Sep 14 '20 at 13:11
sb2346 Please wait a little for Stefs possibly upcoming answer, before you decide on which one to accept. He deserves a little patience. — Yunnosch, Sep 14 '20 at 13:12
Hi @Yunnosch, no hard feelings, I do not plan on writing an answer. — Stef, Sep 14 '20 at 13:21

Yunnosch · Answer 1 · 2020-09-14T13:22:02.100

4

The value returned by your recursive lookup calls (i.e. lookup(n,x->right); and lookup(n,x->left);) is ignored, even if it is the correct one.
This is because at the end of the function (i.e. after returning from one of those calls) you unconditionally return 0;.

Replacing lookup(n,x->XXX); with return lookup(n,x->XXX); is what you want to do.

edited Sep 14 '20 at 13:22

answered Sep 14 '20 at 13:03

Yunnosch

26,130
9
42
54

In case Stef also has an explained answer here upvote that one please. – Yunnosch Sep 14 '20 at 13:10
I did already - and I like your way. If it still needs it feel free. Though I would prefer more of your proposals as comment. – Yunnosch Sep 14 '20 at 13:23

Vlad from Moscow · Accepted Answer · 2020-09-14T13:45:21.417

The function has incorrect behavior because it returns nothing in these if statements

    else if (n>x->data && x->right!=NULL)
    {
        lookup(n,x->right);
    }
    else
    {
        lookup(n,x->left);
    }

That is the control will be passed to the last return statement after the if-else statements

    return 0;

But if you will update the function like

int lookup(int n,struct node* x)
{
    if (x->data==n)
    {
        return 1;
    }
    else if (x->left==NULL && x->right==NULL)
    {
        return 0;
    }
    else if (n>x->data && x->right!=NULL)
    {
        return lookup(n,x->right);
    }
    else
    {
        return lookup(n,x->left);
    }
    return 0;
}

nevertheless the function can invoke undefined behavior because it does not check whether the pointer x is equal to NULL.

For example let's assume that x->data is not equal to n and n is less than x->data. Also let's assume that x->left is equal to NULL and x->right is not equal to NULL.

In this case the sub-statement of the first if statement

    if (x->data==n)
    {
        return 1;
    }

will not be executed.

And the sub-statement of the second if statement also will not be executed because x->right is not equal to NULL.

    else if (x->left==NULL && x->right==NULL)
    {
        return 0;
    }

The same problem will exists with the third if statement.

So the control will be passed inside the last else statement

    else
    {
        lookup(n,x->left);
    }

where the function is called with the null-pointer x->left. Thus in the second call of the function you will try to use the null-pointer to access memory

    if (x->data==n)
    {
        return 1;
    }

The function can be written the following way. Pay attention to that as the function does not change the tree itself its second parameter should be declared with the qualifier const.

int lookup( int n, const struct node *x )
{
    if ( x == NULL )
    {
        return 0;
    }
    else if ( n < x->data )
    {
        return( n, x->left );
    }
    else if ( x->data < n )
    {
        return ( n, x->right );
    }
    else
    {
        return 1;
    }
}

Also usually such a function is declared with the first parameter that specifies the pointer to the node (to the tree) and the second parameter specifies what is searched. That is like

int lookup( const struct node *x, int n );

Or even like

_Bool lookup( const struct node *x, int n );

CiaPan · Answer 3 · 2020-09-14T13:50:42.690

You do a recursive call but you do not return its result!

The simplest recursive version of your code would be:

int lookup(int n, struct node* x)
{
    if (x == NULL)
        return 0;

    if (n == x->data)
        return 1;

    if (n < x->data)
        return lookup(n, x->left);
    else
        return lookup(n, x->right);
}

As long as it didn't get out of the branch it tests the current node - when the value is found, 1 is returned, otherwise the lookup dives into a left or right branch. When the path ends (x == NULL) the value was not found, and 0 is returned.

The result propagates upwards on return from recursion.

The simplest iterative version:

int lookup(int n, struct node* x)
{
    while (x != NULL)
    {
        if (n == x->data)
            return 1;

        if (n < x->data)
            x = x->left;
        else
            x = x->right;
    }

    return 0;
}

It works similarly to the recursive one except that it does not dive into recursion and just moves the x pointer down the tree. If the value is found prior the branch end, 1 is returned. Otherwise the result is 0 when scan walks past a tree leaf.

Binary search tree lookup output

3 Answers3