Simple Calculator Problem (C++)

[Vorde]

Hello all.

I'm teaching myself C++ (my first programming language), and as an exploration I decided to see if I could make a simple four-function calculator. The thing is I'm having trouble. I managed to successfully write a program that would input two integers and return all four functions of the two (their sum, difference, quotient and product) but I couldn't get it to just give me one at a time (a problem with my if clauses I think).

So I started from scratch and wrote this:

Now I get a return value of 0, regardless of the input integers and the chosen function. I'm new to C++, so could anyone tell me what I'm doing wrong?

Thank you.

int chooseOperator(int OP, int num1, int num2)
{

	if (OP == '+')
		return add(num1, num2);
	if (OP == '-')
		return subtract(num1, num2);
	if (OP == '*')
		return multiply(num1, num2);
	if (OP == '/')
		return divide(num1, num2);
	else 
		return 0;
}



int main()
{
	using namespace std;
cout << "Thank you, will you please enter your first integer:" << endl;
		int num1;
			cin >> num1;
cout << "Thank you, will you please enter your second integer:" << endl;
		int num2;
			cin >> num2;
cout << "Will you please choose an operator:" << endl;
		int O;
			cin >> O;
cout << chooseOperator(O, num1, num2);

		cin.clear();
		cin.ignore(255, '\n');
		cin.get();

return 0;
}

(I didn't copy and paste the trivial stuff like the function definitions but I'm not worried that's the problem)

 

[I like Serena]

Hi Vorde!

You have:

int O;
cin >> O;

This will read an 'integer' from standard input.
That is, spaces and new lines are skipped, and digits are expected.

When you enter for instance '+', that can not be converted to an integer.
This would leave your input stream in an invalid state, and your integer in an uninitialized state.

It seems you've already found that the stream state is invalid, since you added cin.clear() afterward.

 

[Vorde]

Ah! Thank you so much! Switching it to a char command fixed the whole thing.

I am indebted.

 

[Borek]

Now that you got this part right, change your if statements to a switch/case:

switch(OP)
{
	case '+': return add(num1, num2);
	case '-': return subtract(num1, num2);
	case '*': return return multiply(num1, num2);
	case '/': return divide(num1, num2);
	default: return 0;
}

Note: in a way this is a shortcut - each case should be ended with a break. But it doesn't matter in this particular situation, as return guarantees code execution stops.

 

[Vorde]

I don't know this shortcut yet. Seems super useful though, does it do exactly what it seems like it's doing?

 

[uart]

does it do exactly what it seems like it's doing?

No, not quite.

Most languages have something like a switch/case type control structure that allows one individual case to be executed out of a range of choices. C/C++ is a bit unusual however, in that the default behavior is to "drop through" (and hence execute all subsequent cases) rather than just execute the selected case. To overcome this you need to add a "break" command at the end of each case (that is unless you actually want it to "drop through" of course).

Boreks point was that in this particular instance, the fact that each case clause consists of a "return" means that you don't have to worry about "drop through".

 

[Borek]

Compare:

switch(OP)
{
	case '+': return add(num1, num2);
	case '-': return subtract(num1, num2);
	case '*': return return multiply(num1, num2);
	case '/': return divide(num1, num2);
	default: return 0;
}

and

switch(OP)
{
	case '+': answ = add(num1, num2);
		  break;
	case '-': answ = subtract(num1, num2);
		  break;
	case '*': answ = multiply(num1, num2);
		  break;
	case '/': answ = divide(num1, num2);
		  break;
	default: answ = 0;
}
return answ;

They both do the same - one stops the execution leaving the function (which is a shortcut I was referring to), the other by using break commands (which stops execution of switch and jumps to the end of the list as expected). Note that in the latter case the break command is not needed after the last case entry (but putting it there is not an error, just doesn't make much sense).

 

[Vorde]

I think I understand and it definitely seems to be shorthand, but it seems to me that using the more general 'if' clause let's you be more versatile with your conditions. I was making some changes (adding a decimal-to-binary function) and it was useful to be able to change the requirements of one of the if statements (adding a &&) but not the others, whereas I don't think what I did would be as easy using the 'case' system.

 

[Borek]

Both if and case/switch are valid tools, sometimes it is better to use one, sometimes it is better to use the other. I believe switch/case is faster when there are many values, for sure code is more readable. Note you can always do something like

	case '/': if (num2 != 0) answ = num1/num2;
			else answ = MAXINT;
		  break;

if you want to add a logical condition.