If/Else statement returns

Question

This section of code keeps throwing an error, "not all control paths return a value". I'm not entirely sure how to rewrite this so that I can fix the error. Other than the returns, this is what I need this section of code to do. Should I create a new variable that I declare within the if/else statement and then have the return for that variable right before the ending bracket?

node * LList::search(int srchKey)
{
    node * p = head;
    while (p != NULL)
    {
        if (p->key == srchKey)
        {
            return p;
        }
        else
        {
            return NULL;
        }
        p = p->next;
    }
}

Think: what if the loop is never entered? Also, I can't see when `p = p->next;` would ever run. That should give dead code warnings. — Carcigenicate, Apr 26 '18 at 23:56
`p = p->next;` will never be executed. Either the `if` or `else` block executes, and both of them return from the function, which ends the loop. — Barmar, Apr 26 '18 at 23:57
@Barmar Thank you for pointing that out as well. Our professor included an example in the PowerPoint slides that I was similar to this one. So as long as there is an if/else, the other statement is useless? — draikaina8503, Apr 27 '18 at 00:06
Not in general. Only in this case because both of them return from the function. — Barmar, Apr 27 '18 at 00:22

score 3 · Answer 1 · edited Apr 27 '18 at 01:23

If head is NULL, search() will exit without reaching any return statement. The return value will be indeterminate. That is what the compiler is complaining about.

If head is not NULL, search() will check only the first node and then return a value, it will not search the whole list. Because of that, the return NULL; statement should not be inside the loop at all. It will exit search() as soon as it encounters an element that doesn't match, rather than continuing to the next element in the list.

This is similar to the problem I describe in Searching array reports "not found" even though it's found.

You should wait until the loop ends before doing return NULL;. If you get there, it means the item being searched for really wasn't found.

node * LList::search(int srchKey)
{
    node * p = head;
    while (p != NULL)
    {
        if (p->key == srchKey)
        {
            return p;
        }
        p = p->next;
    }
    return NULL;
}

Christian.M · Answer 2 · 2018-04-27T00:02:41.970

1

You need another return statement after the while loop. One possible code path is that the program never enters the while loop. A simple

return NULL;

should fix it. EDIT: Also, your loop will only ever make one pass through. I would remove the (if p == NULL) block; it is useless. I see your intent, but the way to implement it is how I described above.

edited Apr 27 '18 at 00:02

answered Apr 26 '18 at 23:55

Christian.M

173
8

This is a perfect answer addressing his warning, but it could be even better if you added a bit about how he can only ever make one loop iteration! – scohe001 Apr 26 '18 at 23:57
Btw, you can suggest an edit if you see something like that. Saves me the trouble of doing it myself! :) – Christian.M Apr 27 '18 at 00:03
2

At my rep if I make an edit it goes through immediately. So especially for something as big as that, in my experience suggesting in a comment usually works out better :) – scohe001 Apr 27 '18 at 00:30

Shadi · Answer 3 · 2018-04-27T00:57:16.473

1

when while condition is false, then you do not have return.

Add a return outside while in case you do not enter the loop.

Moreover, p = p->next; will never be executed because you exit the loop before reaching it because of if-else.

Regarding the logic, It seems to me that you do not need the else part at all. try to omit it and place return NULL before the end of the routine.

edited Apr 27 '18 at 00:57

answered Apr 26 '18 at 23:57

Shadi

1,701
2
14
27

If/Else statement returns

3 Answers3