control reaches end of non-void function

[lancevo3]

I get the warning control reaches end of non-void function and I'm not seeing what causes it.

double ExprTree::evaluate(TNode* p) const
        {
        if(p->data == "+" || p->data == "-" || p->data == "*" || p->data
                ==  "/")
                {
                double op1 = evaluate(p->left);
                double op2 = evaluate(p->right);
                if(p->data == "+")
                        return op1 + op2;
                if(p->data == "-")
                        return op1 - op2;
                if(p->data == "*")
                        return op1 * op2;
                if(p->data == "/")
                        return op1 / op2;
                }
        else if(isalpha(p->data[0]))
                return getVariable(p->data);
        else
                return atof((p->data).c_str());
        }

[sfuo]

Is this where it says the error is? Or can you post all your code if it isn't too long.

[Tom Gunn]

if(p->data == "+")
                        return op1 + op2;
                if(p->data == "-")
                        return op1 - op2;
                if(p->data == "*")
                        return op1 * op2;
                if(p->data == "/")
                        return op1 / op2;

If none of these cases are true, there is no default return value, and execution will fall off the end of the function without returning anything.

[sfuo]

If none of those cases are true then it goes to the else if below.

[Tom Gunn]

If none of those cases are true then it goes to the else if below.

If statements do not fall through like switch cases.

#include <iostream>

int main()
{
    if (true)
    {
        std::cout << "Body of outer if\n";

        if (false)
        {
            std::cout << "Body of inner if\n";
        }
        else if (false)
        {
            std::cout << "Body of inner else if\n";
        }
    }
    else
    {
        std::cout << "Body of outer else\n";
    }
}

[sfuo]

I never said it worked in that kind of structure the way he has it set up it works like a switch. Read what he posted not what you snipped.2

[lancevo3]

I figured it out I just turned it into a cascading if and it solved the warning.

[Tom Gunn]

Read what he posted not what you snipped.

I mean no offense, but you are wrong. Please study the code lancevo3 posted more carefully. I will remove the unnecessary stuff and reformat the code to a more common style to help you see the error:

double evaluate(TNode* p)
{
    if(outer condition 1)
    {
        if(inner condition 1)
            return;
        if(inner condition 2)
            return;
        if(inner condition 3)
            return;
        if(inner condition 4)
            return;

        // no return!
    }
    else if(outer condition 2)
        return;
    else
        return;
}

If you cannot see the error now, I recommend studying if statements until you can. This is basic knowledge that a C++ programmer must have if he wants to write bug free code.

[siddhant3s]

Consider the parenthesised version of the OP's code:

double ExprTree::evaluate(TNode* p) const
{		//this is the first if
        if(	p->data == "+" ||
			p->data == "-" ||
			p->data == "*" ||
			p->data ==  "/"		)
                {
		            double op1 = evaluate(p->left);
		            double op2 = evaluate(p->right);
		            
					if(p->data == "+")			//if number 1.1
		                    return op1 + op2;
		            if(p->data == "-")			//if number 1.2
		                    return op1 - op2;
		            if(p->data == "*")			//if number 1.3
		                    return op1 * op2;
		            if(p->data == "/")			//if number 1.4
		                    return op1 / op2;
                }
        else
				{
					if(isalpha(p->data[0]))		// if number 2
		                return getVariable(p->data);
        			else
                		return atof((p->data).c_str());

        		}
}

Thinking logically, we all know that this function will return a value [1].
But the compiler cannot determine that. This is because the compiler is unable to realise that when the condition of the first if is true then at least one of the four conditions of if statements(if number 1.1 to 1.4) will definitely be true. The compiler cannot logically relate the original condition and the distributed condition. So, according to the compiler, there can arise situations when all of the internal if's (if number 1.1 to 1.4) be false at the same time. Hence, (according to the compiler) there may arise situations when the function returns no value, thus the warning.

[1]: If it is not immediately clear that this function will definitely return something, you should perhaps check the structure of the if lader thus formed. If the if_1 is true, at least one of the internal if's ( if_1.1 to if_1.4) will have to be true. If the if_1 is false, the else part gets executed. That means the if_2 is checked, which if found to be true, returns a value. If the if_2 is false, then too it returns a value.

[sfuo]

I understand what you are trying to say with the inner and outer conditions and you would run into an error there but the outer condition is that the input has to be one of the inner.

I wrote a quick example to double check myself and found it worked how I expected.

#include <iostream>

using namespace std;

int funct(int input)
{
	if( input == 1 || input == 2 || input == 3 || input == 4 )
	{
		cout << "range of 1-4" << endl;
		if( input == 1 )
			return 1;
		if( input == 2 )
			return 2;
		if( input == 3 )
			return 3;
		if( input == 4 )
			return 4;		
	}
	else if( input == 5 )
	{
		cout << "is 5" << endl;
		return 5;
	}
	else
	{
		cout << "else" << endl;
		return input;
	}
}

int main()
{
	int input;
	
	cin >> input;
	
	cout << funct(input) << endl;
	
	system("PAUSE");
	return 0;
}

His code would not work if it was

if( input != alpha || input != number ) (meaning a symbol)
{
if( + ) return add
if( - ) return subtract
if( * ) return multiply
if( / ) return divide
//here would be an error because there is no return for other symbols however the other examples have cut out all that by making the "outer" if statement only take in the inner ifs
}
else if ( input == alpha )
return getVariable(input)
else
//all other cases (numbers) in his example it would take in anything including other symbols like '%' '@' which would return 0
return atof(input)

It is all dependent on what conditions you use because in the case where you put it in the more common style yes there would be an error because you don't know what outer condition 1 is. But if you look at an example with real code if outer condition 1 is all of the inner conditions (1-4) then it works because you put return values for every case possible within that if statement.

[Tom Gunn]

It is all dependent on what conditions you use because in the case where you put it in the more common style yes there would be an error because you don't know what outer condition 1 is. But if you look at an example with real code if outer condition 1 is all of the inner conditions (1-4) then it works because you put return values for every case possible within that if statement.

Yes, so you understand the error now? Code with intertwined dependencies like this is bad code that needs to be fixed. If a maintenance programmer cannot glance at a simple algorithm and know that it is right without doing an in depth analysis, there is a serious problem.

[sfuo]

Yeah OK it is bad for a large scale project and one that will be changed and maintained. For this example it doesn't really matter but what you are trying to say is it is bad practice and you should do it with proper structure.

There is a huge difference between wrong and a better and cleaner way. In this case no ways were wrong but I can see where you are coming from you just came across unclear at the start.