CODEWITHSUNDEEP
c++functionreturn-type
Homayoun Ghojavand
Homayoun Ghojavand

Reputation: 1

When to return from a function?

Sorry in advance if the question sounds naive. I am writing a simple bool function to check if all digits of a number are same. The following works, however the one after, gives me an error of

17:1: warning: control reaches end of non-void function [-Wreturn-type]

What I am doing wrong with the second one?

Working:

# include <iostream>
# include <string>

using namespace std;

bool samedigits (int number){
    bool result = true;
    std::string snumber = to_string (number);
    for (int i=0; i < snumber.size(); i++){
        if (snumber[i] != snumber[0]){
            result = result and false;
            break;
        }   
    }
    return result; 
}

int main()
{
    cout << samedigits (666);
    return 0;
}

Non working:

# include <iostream>
# include <string>

using namespace std;

bool samedigits (int number){
    
    std::string snumber = to_string (number);
    for (int i=0; i < snumber.size(); i++){
        if (snumber[i] != snumber[0]){
            return false;
            break;
        }   
        return true;
    }
    
}

int main()
{
    cout << samedigits (666);
    return 0;
}

Upvotes: 0

Views: 100

Answers (3)

JaMiT
JaMiT

Reputation: 16873

The second version of your function (combined with some information obtained via comments) indicates a misunderstanding of what return does. The second version would work (here is the misunderstanding) if a return statement were to simply store the indicated value for use when the function eventually ends. However, this is not how return works. A return statement stores the indicated value for use when the function ends and immediately ends the function. In the second version of your function, the for statement might as well be an if and the break is never executed as it comes right after a return.

To demonstrate, let's do a code walk-through for a call to samedigits(123).

bool samedigits (int number){

As we enter the function, number is set to 123.

std::string snumber = to_string (number);

The string snumber is set to "123".

for (int i=0; i < snumber.size(); i++){

The loop initializes by setting i to 0 then checks if 0 < 3 (the size of snumber is 3). This is true, so we enter the loop. Note that the result of entering the loop depends only on snumber not being empty.

    if (snumber[i] != snumber[0]){

We check to see if snumber[0] does not equal snumber[0]. This is a bit trivial, but the computer is willing to do it. The result, of course, is false (independent of what the input was – this part might be more interesting if the loop started at 1 instead of 0). So skip to the statement after the if.

    return true;

The function immediately ends, returning true.

And that's it. There is no second iteration of the loop. No other code is executed during this function call. Since to_string never returns an empty string, the second version of your function is functionally equivalent to the following:

bool samedigits (int /*number*/){
    return true;
    // Execution ends with the return statement, so nothing after
    // this comment ever executes.
    std::cout << "This is dead code. It will never execute.\n";
    std::cout << "You will never see this output.\n";
}

To fix the second version, you need to return inside the loop only when the if condition is true. Move return true; to be after the loop so that the loop can iterate multiple times. You do not want to end the function and tell the caller that all the digits are the same (which is what return true; does) until after you've checked all the digits. (If your loop finds a mismatch, execution will reach the return false; inside the loop. At that point, the function ends, so code after the loop has no effect on that function call.)

A smaller (cosmetic) fix is to get rid of the break. Suppose the loop did iterate enough times to find a mismatch. Execution would go into the if statement body and encounter return false. At that point, not only is the loop stopped, but the function as a whole ends, before the break statement is seen. The break statement is dead code, meaning code that can never be executed. In order to get to the break, execution has to go through a return. Execution may arrive at a return statement, but it never goes through one.

Also, be sure to thank your compiler for finding this error, as it does point to a bug in your code. It's not the exact bug the compiler reported, but then again, compilers are not exactly the best thinkers in the world. :)

Upvotes: 0

cigien
cigien

Reputation: 60228

Your algorithm is incorrect, you are only checking if the first character is equal to itself, and returning true for every input. Instead, you should move the return true outside the loop, so that you check all the characters first.

Unfortunately, the warning is a false positive. The compiler fails to realize that std::to_string is guaranteed to return a non-empty string when passed an int. This means that the for loop body will be entered, and the function will return a value.

Upvotes: 2

Guillaume Racicot
Guillaume Racicot

Reputation: 41770

The compiler is right. There is a code path in your second snippet that won't return.

for (int i=0; i < snumber.size(); i++){

Here, the std::string size function can return 0 according to its contract. When it does happen, then your function won't enter the loop. After that, you exit the function without returning.

Upvotes: 0

Related Questions