C++

[Viz]

For some reason I cannot get this working properly, it keeps responding as a invalid value when you enter +,-,* or /. Any help? Also any suggestions on making this code more efficient would be appreciated!

int calMenu(){ // Main Calculator Function
	int x;
	int y;
	char calInput;

/*_____________[Does not work]_______________*/
	start:
	cout << "Please select operation: (+,-,*,/): "; //Does not work for some reason
	cin >> calInput;
	if (!calInput == '+'|'-'|'*'|'/')
	{
		cout << "You entered a incorrect operation." << endl;
		goto start;
	}
/*___________________________________*/

	cout << "Please enter your first number: ";
	cin >> x;
	cout << "Please enter your second number: ";
	cin >> y;


	switch (calInput){
	case '+': cout << x << " + " << y << " = " << x + y << endl;
		break;
	case '-': cout << x << " - " << y << " = " << x - y << endl;
		break;
	case '*': cout << x << " * " << y << " = " << x * y << endl;
		break;
	case '/': cout << x << " / " << y << " = " << x / y << endl;
}
	return 0;

edit:
I've chose to run with this and this seems to work the best:

cout << "(+)(-)(*)(/)" << endl;

	while (running)
	{

	cout << "Please choose an operation: ";
	cin >> calInput;

	if((calInput != '+') && (calInput != '-') && (calInput != '*') && (calInput != '/')){
	cout << "Please choose a valid operation." << endl;
        break;
	}
	

[DrLuke]

Don't use goto. It only causes problems. Instead, use a while loop, with a boolean variable like

bool entering = true;
while(entering)
{
    ...
    if(!calInput == '+'|'-'|'*'|'/')
         entering = false;
}

[MatheusMCardoso]

I think you should do this:

calInput != '+'|'-'|'*'|'/'

instead of this:

!calInput == '+'|'-'|'*'|'/'

I'm not sure if it makes a difference, but the way you're doing it doesn't make sense.

[MegaJohnny]

The other problem is with the if statement - I see you want to check whether it's any of those characters, but there's no shorthand like that, and using one pipe symbol causes it to use [url=http://www.learncpp.com/cpp-tutorial/38-bitwise-operators/]bitwise operations[/url] on the characters. So your program ORs together all the characters and tests whether callInput is equal to the result. Your if statement should be something like:

if (callInput != '+' && callInput != '-' && callInput != '*' && callInput != '/')

which reads "if callInput isn't equal to + AND callInput isn't equal to - ....."

[Johnny on Flame]

if(!calInput == '+'|'-'|'*'|'/')

This is wrong. You are doing 'or' for each of these chars, and then comparing them to calInput.
I'll propose this as a solution:

if(!strchr("+-*/", calInput))

strchr searches for the first occurance of a char inside a string. if it returns something, it means that calInput was inside "+-*/" and the return will be a pointer to where it is, otherwise the return is null, which works as a way to check it.

[QUOTE=DrDevil;44658480]Don't use goto. It only causes problems. Instead, use a while loop, with a boolean variable like

bool entering = true;
while(entering)
{
    ...
    if(!calInput == '+'|'-'|'*'|'/')
         entering = false;
}

[/QUOTE]

GOTO is a bad idea, but instead of using a boolean variable, you could just call break;

[Tamschi]

[QUOTE=DrDevil;44658480]Don't use goto. It only causes problems. Instead, use a while loop, with a boolean variable like

bool entering = true;
while(entering)
{
    ...
    if(!calInput == '+'|'-'|'*'|'/')
         entering = false;
}

[/QUOTE]

It's not [I]always[/I] terrible, in this case a loop or tailcall function would be better though.

I think there are two issues here: First is that [I]![/I] has precedence over [I]==[/I], so you invert calInput and then compare it to the part on the right.
There's a [I]!=[/I] operator to test for inequality.

The second is that [I]|[/I] is bitwise or and has higher precedence than [I]==[/I] and [I]!=[/I] too, so you get a single char on the right side that probably has none of the values you want...

You really have to compare them one-by-one or use a function that checks if the value you give it is in an array.

[Viz]

Thanks guys it's working flawlessly. Man I can't believe I had so much trouble with a simple calculator...:tinfoil:

[Adult]

[QUOTE=Viz;44661269]Thanks guys it's working flawlessly. Man I can't believe I had so much trouble with a simple calculator...:tinfoil:[/QUOTE]

trying to do multiple comparisons like this is a really, really common problem amongst new programmers. don't beat yourself up about it.

[Angus725]

Perhaps merging the if statement with the switch statement might be a better solution.

int calMenu(){ // Main Calculator Function
	int x = 0, y = 0;
	char calInput = 0;

    cout << "Please select operation: (+,-,*,/): ";
    cin >> calInput;

    cout << "Please enter your first number: ";
    cin >> x;
    cout << "Please enter your second number: ";
    cin >> y;

    while(1)
    {
        switch (calInput)
        {
            case '+': cout << x << " + " << y << " = " << x + y << endl;
                return 0;
            case '-': cout << x << " - " << y << " = " << x - y << endl;
                return 0;
            case '*': cout << x << " * " << y << " = " << x * y << endl;
                return 0;
            case '/': cout << x << " / " << y << " = " << x / y << endl;
                return 0;
            case default:
                cout << "You entered a incorrect operation." << endl;
                cout << "Please select operation: (+,-,*,/): ";
                cin >> calInput;
                break;
        }
    }
}

[Rofl my Waff]

[QUOTE=DrDevil;44658480]Don't use goto. It only causes problems. Instead, use a while loop, with a boolean variable like

bool entering = true;
while(entering)
{
    ...
    if(!calInput == '+'|'-'|'*'|'/')
         entering = false;
}

[/QUOTE]

Why is this being disagreed. There is literally no reason to use a goto over iteration here.

[supersnail11]

[QUOTE=Rofl my Waff;44670572]Why is this being disagreed. There is literally no reason to use a goto over iteration here.[/QUOTE]

Because he says never to use goto.

[Rofl my Waff]

I don't think a beginner should use goto ever. It teaches people who can't figure out the logic of where their code is going to skip over things.

[itsthejayden]

Next time, use this
[url]http://facepunch.com/showthread.php?t=1250528[/url]

[supersnail11]

[QUOTE=Rofl my Waff;44670788]I don't think a beginner should use goto ever. It teaches people who can't figure out the logic of where their code is going to skip over things.[/QUOTE]

But he said that you should [i]never[/i] use goto. Like there is no possible case that goto can be useful.

[Angus725]

I rarely, but do use goto statements for some very fancy loop exit conditions. However, I do have background in assembly programming...

[editline]28th April 2014[/editline]

[QUOTE=Rofl my Waff;44670572]Why is this being disagreed. There is literally no reason to use a goto over iteration here.[/QUOTE]

The loop is fine, but the logic doesn't work.

[Jookia]

[QUOTE=supersnail11;44670977]But he said that you should [i]never[/i] use goto. Like there is no possible case that goto can be useful.[/QUOTE]

I like to think that gotos highlight flaws in the language's ability to handle control flows.

[Tamschi]

[QUOTE=Jookia;44672176]I like to think that gotos highlight flaws in the language's ability to handle control flows.[/QUOTE]

It's nice in Java (off all things): You can label a block and then break out of it using

break label;