Print

Please login or register.
1 Hour 1 Day 1 Week 1 Month Forever
Login with username, password and session length

[Rgamer]

Another super dumb question here:
Why is this not working? I spent about 40 minutes looking it over, and couldn't figure out why it wasn't working.

Spoiler (click to show/hide)

#include <iostream>

int main()
{
    using namespace std;

    int nX;
    cout << "Please enter the first number: " << endl;
    cin >> nX;

    int nY;
    cout << "Please enter the second number: " << endl;
    cin >> nY;

    int nSelection;

tryAgain:
    do
    {
        cout << "Please make a selection:" << endl;
        cout << "1:Addition" << endl;
        cout << "2:Subtraction" << endl;
        cout << "3:Multiplication" << endl;
        cout << "4:Division" << endl;
        cin >> nSelection;
    }while (nSelection != 1 && nSelection != 2 && nSelection != 3 && nSelection != 4);


    switch(nSelection)
    {
        case'1':
            return nX + nY;
        break;//prevents it from this and all below(called fallthrough)
        case'2':
            return nX - nY;
        break;//Prevents fallthrough
        case'3':
            return nX * nY;
        break;//Prevents fallthrough
        case'4':
            return nX / nY;
        break;//prevents fallthrough
        default:
            cout << "Error" << endl;
            goto tryAgain;
    }


    return 0;
}

Thanks to anyone who manages to find the issue.

[Levi]

I notice that nSelection is an integer, but you are comparing it to char's in the switch statement.

[alfie275]

cin is outputting the integer they type, rather than the char, so compare it with an integer.
Also you used a goto.

[Starver]

Also you used a goto.

And given the code already appears to pre-validate the input before leaving the input and leading onto the case, I wouldn't even do that.  Either take it on trust (for the time being, that's being a simple bit of code) or replace the catch-all with a "die"-like exception raiser to alert you (or the end user) that something horrible's happened to the code.  You shouldn't ever encounter it (prior to you having made it more complex and forgetting to keep it consistent), but if it outputs Error XYZ in Operator Selection routine: an inexplicable selection of '<whatever>'! and quits the program, you have something of a guide when you're trying to work out why...


(BTW, I would have suggested the opposite, for the original problem, almost.  But to the same end...  Change the expected input integer to be an expected input character.  That way you don't have to rewrite more than necessary when you've decided you want "A", "B", "C", etc, instead of/in addition to "1", "2", "3", etc...  Or even literally "+", "-", "*" and "/", but I would aim at seeing this becoming more generalised and working against an "I expect <foo> for <Foo>, <bar> for <Bar>..." processing function, if you use a similar mechanism in more than one place in the code.  Think ahead!)

[eerr]

"while (nSelection != 1 && nSelection != 2 && nSelection != 3 && nSelection != 4);"

I was about to say order of operations, but this is just messed up.
You need more brackets, and you need the true/false equals operator (==)

Try

while (
(nSelection == 1) &&
(nSelection == 2) &&
(nSelection == 3) &&
(nSelection == 4)
)


also, note = vs == as seriously problematic, for debugging.

[Levi]

What is wrong with != in this situation?  Looks fine to me?

[Virex]

"while (nSelection != 1 && nSelection != 2 && nSelection != 3 && nSelection != 4);"

I was about to say order of operations, but this is just messed up.
You need more brackets, and you need the true/false equals operator (==)

Try

while (
(nSelection == 1) &&
(nSelection == 2) &&
(nSelection == 3) &&
(nSelection == 4)
)


also, note = vs == as seriously problematic, for debugging.

Wait, what kind of mathematical object is equal to multiple integers at the same time?

[Starver]

and you need the true/false equals operator (==)

You sure?

I read that do{}while loop as being intended as "keep going while there is no match".

(Ninjaed, on this point, while writing the below...)

And thus (yes, parenthesise, because I can never remember the order of precedence of logical-Ands and Ors versus (in)equality tests) the test of Anding together the not-equals will give a whole-scope Trueness if all of them don't equal.

I'd have done that differently, actually, but mostly in ways easier (or only possible) in other dialects of computer language, but maybe not relevant to this one.

How about we just do away with a separate "ask" loop and the Case block with that goto and do an eternal do{}while(true) loop within which is an "ask" and then the Case block.  The valid answers in the Case block break out of the loop, but the invalid one gives its complaint to the user and then gets looped back.

Or, prior to entry to the loop, there's a "continue = true", and each test in the Case sets "continue = false" ready for the "while(continue)" but.  Except for the final trap.  That complains to the user, and leaves "continue" alone.

But there are so many different loop-types available.  And ways of exploiting them.  If you don't want a separate "continue", set the "cSelection" character (already, as noted, ready for later expansion possibilities) as null prior to entering the loop.  Make the default statement re-set it to null, and make the while-continuation reliant upon it being so set.  (Hitting on a correct option will leave it alone, and the loop exits.  With the advantage of having the valid option available for any other purpose you might have, for example adding it to a history of the options chosen...)

[Valid_Dark]

I'm on my phone so I can't go into detail, but != will work how he used it I'm pretty sure. will go into detail soon.

[MaximumZero]

Maybe I'm just a n00b (and I totally am,) but the only thing I saw that was really wrong with it was the tryAgain and goto tryAgain. If he ditched them, it should be fine, no? Maybe encase the whole damned thing in a do/while to prevent screwups.

[Starver]

the only thing I saw that was really wrong with it was the tryAgain and goto tryAgain. If he ditched them, it should be fine, no?

While not exactly good style, I don't see anything wrong with it, per se.  Also, because the "while(not this and not that and not the other)" only lets through (or should let through, see below) an expected value, the goto will never even be used, so could just be removed and it would work just the same (or not-work just the same, if other errors are present).[/quote]

Maybe encase the whole damned thing in a do/while to prevent screwups.

Yup.  See my post if you like reading mini-walls of text with multiple possible answers...

The definite problem seems to be the mismatching (either make it all character input/testing or make it all integer input/testing), and because I still can't remember the precedence order, to be safe we need to parenthesise the multi-check line (or do away with it, as we've just both said, and have instead something like the "while(continue)" or "while(cSelecton == null)" termination method in that whole-darned-thing loop).

[Valid_Dark]

ok, the reason != works in this case is because it is one of the relational operators,
the relational operators are != , == , > , < , =< , and =>, these all return a boolian value.
(I know the person that originally asked for help probably knows this, but there seemed to be some confusion about it, so I thought i'd clear that up)

Code: [Select]

I notice that nSelection is an integer, but you are comparing it to char's in the switch statement.what he said is true, in order to compare them to int values remove the ' ' from the switch cases.
example, you did  "case'1':" should be "case 1:"
but in this case it doesn't really matter, because i'm pretty sure the ascii of 1-9 is 1-9.


that could be written way better than a do while loop IMHO
but even if you want it as a do while loop

the cin should be inside of it, or else it would only work the first time. then when it goes back to it to try it again the value that was used as nselector the first time would be kept every other time it ran.
so you should either include the cin inside the loop, and/or reset the nselector value for each rerun of the program.
-this I think is the main problem you're having / the answer you're looking for.

your default doesn't have a break, it doesn't really need one in this case but it's good practice to always include them in the switch cases.

as others said, it's a bad Idea to use goto functions, it's much better to include the whole thing in another kind of loop,
if you're comfortable using do...while loops you should be more than comfortable using most of the other kinds of loops, so you can figure out a way for it to go back to the middle point of the program without using the goto function.


-got ninja'd

[Mephisto]

The program is returning the result, not printing it out. After fixing the int->char comparison, the program functions just fine. Just... there's no output. The requested value is the program's return value. Change the returns within the switch statement to "cout << " and you can actually see what the results are.

[Valid_Dark]

the only thing I saw that was really wrong with it was the tryAgain and goto tryAgain. If he ditched them, it should be fine, no?

While not exactly good style, I don't see anything wrong with it, per se.  Also, because the "while(not this and not that and not the other)" only lets through (or should let through, see below) an expected value, the goto will never even be used, so could just be removed and it would work just the same (or not-work just the same, if other errors are present).

Maybe encase the whole damned thing in a do/while to prevent screwups.

Yup.  See my post if you like reading mini-walls of text with multiple possible answers...

The definite problem seems to be the mismatching (either make it all character input/testing or make it all integer input/testing), and because I still can't remember the precedence order, to be safe we need to parenthesise the multi-check line (or do away with it, as we've just both said, and have instead something like the "while(continue)" or "while(cSelecton == null)" termination method in that whole-darned-thing loop).
[/quote]

he's not just doing a while loop, he's doing a do...while loop.

do...while  syntax
do
{statements}
while (expression);

NOT A while loop
while loop syntax
while (expression)
{statements}

I orgininally thought the issue was with the while line as well, it just looks funky,
"while this is not 1 and not 2 and not 3 and not 4"
 could be written better no doubt, but that is not the problem.


and I feel stupid for not even noticing he used return instead of cout.
return is used for closing the function / program. not for printing to the console or any other output stream.

[Rgamer]

The program is returning the result, not printing it out. After fixing the int->char comparison, the program functions just fine. Just... there's no output. The requested value is the program's return value. Change the returns within the switch statement to "cout << " and you can actually see what the results are.

Seeing as this is running in a console window, I can see it. However, I can't believe I didn't realize that. Thanks.

and I feel stupid for not even noticing he used return instead of cout.

It's alright. I feel stupid for using return instead of cout.

While not exactly good style, I don't see anything wrong with it, per se.  Also, because the "while(not this and not that and not the other)" only lets through (or should let through, see below) an expected value, the goto will never even be used, so could just be removed and it would work just the same (or not-work just the same, if other errors are present).

That goto was the result of a misunderstanding of how do...while loops work. I didn't realize that the do...while loop wouldn't let through other values, so I thought that was the only way to manage it.(That I currently know of.)