14

I cannot figure out why this error is happening: error: control may reach end of non-void function

Here is the code:

bool search(int value, int values[], int n) {

    if (n < 1) {
        return false;
    }   

    for (int i = 0; i < n; i++) {
        if (values[i] == value) {
            return true;
            break;
        }
        else { 
            return false;
        }
    }    
}

I understand that the error means that the function may reach the end without returning anything, but I cannot figure out how that might happen.

Matthew Champion
  • 400
  • 7
  • 18
cspearsall
  • 149
  • 1
  • 1
  • 3
  • 5
    The complier does not analyze that "deeply" the "sense" of the code in a function. Just add `return false;` at the end of the function. This will even make the first `if` unnecessary. – Kiril Kirov Mar 14 '14 at 16:24
  • 2
    possible duplicate of [What does "control reaches end of non-void function" mean?](http://stackoverflow.com/questions/6171500/what-does-control-reaches-end-of-non-void-function-mean) – fvu Mar 14 '14 at 16:34
  • 5
    BTW, it looks like your loop only checks `values[0]`. – Joseph Quinsey Mar 14 '14 at 16:34
  • 1
    I know I'm late to the party here and your homework's already done or you've given up on it years ago, but for completeness' sake I'd like to point out: 1) There's absolutely no need to break after return true; return exits the function so breaking out of the loop is redundant. 2) if (condition) return true; else return false; means you'll only test the condition one time. Simply put, i will never reach a value higher than 0. – Andrej Jun 20 '17 at 11:12

4 Answers4

14

You are getting this error because if your for loop breaks due to breaking condition i < n; then it don't find any return statement after for loop (see the below, I mentioned in code as comment). 

for (int i = 0; i < n; i++){
    if (values[i] == value){
        return true;
        break;
    }
    else{ 
        return false;
    }
}
  // here you should add either return true or false     
}

If for loop break due to i >= n then control comes to the position where I commented and there is no return statement present. Hence you are getting an error "reach end of non-void function in C".

Additionally, remove break after return statement. if return executes then break never get chance to execute and break loop. 

   return true;  -- it returns from here. 
    break;  -- " remove it it can't executes after return "

Check your compiler should give you a warning - 'unreachable code'.

Grijesh Chauhan
  • 57,103
  • 20
  • 141
  • 208
6

That compiler warning is not correct. Anyway, there is a bigger problem with your code:

bool search(int value, int values[], int n) {

    if (n < 1) {
        return false;
    }   

    for (int i = 0; i < n; i++) {
        if (values[i] == value) {
            return true;
            break;
        }
        else {            // !
            return false; // ! <-- Here is the mistake.
        }                 // !
    }    
}

This code only checks values[0] == value and then always returns. It's happening because of that else {return false;}.

You should write it this way:

bool search(int value, int values[], int n) {

    if (n < 1) {
        return false;
    }   

    for (int i = 0; i < n; i++) {
        if (values[i] == value) {
            return true;
            // break;  <- BTW, it's redundant.
        }
    }    
    return false;
}

Now, function checks entire values array and then returns false if there was no matches. But if it found a match, it will instantly return true witout checking other elements.

Also, compiler will not emit a warning for this code.

HolyBlackCat
  • 78,603
  • 9
  • 131
  • 207
3

Your code is equivalent to

return (n > 0 && values [0] == value);

Either you are in the habit of writing very simple things in an excessively complicated way, or that code doesn't do what you want it to do.

gnasher729
  • 51,477
  • 5
  • 75
  • 98
1

Some folks will probably hate this, but....

bool search(int value, int values[], int n) {

   if (n < 1) {
      return false;
   }   

   bool ret = false;
   for (int i = 0; i < n; i++) {
      if (values[i] == value) {
         ret = true;
         break;
      }
   }    
   return ret;
}
Jiminion
  • 5,080
  • 1
  • 31
  • 54