357mag

I'm trying to incorporate a little more error checking in my program. I have this menu that lists four possible choices:

1. Addition
2. Subtraction
3. Multiplication
4. Division

I want to add some code so that if the user enters a choice(a number)that's not on the menu, a message will display and the program will keep prompting him until he enters either a 1, 2, 3, or 4. But what I got isn't working. Even when I enter a correct choice, the program continues to loop instead of going on with the rest of the program. Don't know where I'm going wrong. Here is what I got:

teencoder

Where are the while's curley braces. Also you are using a binary or(|) you need a logical or(||).
See http://www.cplusplus.com/doc/tutorial/operators.html.

__________________
Geeks may not be cool now but in the long run they prosper.

DaWei

You might consider "choice < 1 || choice > 4". I'm pretty sure we've discussed this before: always check your input for success. What happens if I enter 'z'? You'll never pick up a correct entry, even if I subsequently make one, that's what.

__________________
Abstraction doesn't make it impossible to write bad code; it makes it possible to write superior code.
Contributor's Corner: Grumpy on C++ Exceptions DaWei on Pointers

francoissoft

DaWei has a point. You should check for errors on the iostream. You can do this by calling the method cin.bad() to determine if there is a problem with the input.

DaWei

Actually, a good return from cin.bad () does not mean there was no failure. Check the documentation for cin.good (), cin.fail (), and cin.bad ().

__________________
Abstraction doesn't make it impossible to write bad code; it makes it possible to write superior code.
Contributor's Corner: Grumpy on C++ Exceptions DaWei on Pointers

357mag

My program still isn't working correctly. I did add braces for the while, and I did change the binary OR to the logical OR:


No matter what I enter, whether its a 1, 2, 3, 4(all correct choices) the program says "Invalid choice..."

357mag

I even tried adding parentheses around everything but it still doesn't work.

357mag

I think I'm using the wrong operator. I got to use Logical AND(&&).

lectricpharaoh

See how the test conditions for your while and if statements (red) are the same? That's one problem; it's redundant.
This is your second problem. Using the or operator as you've done (whether logical or bitwise doesn't matter) will always yield a result of true. This means the code in blue will always be executed.

See, you say you want it to execute the while code block if response does not equal one, or if response does not equal two (or three, or four). Let's say response did equal one; it therefore could not be two, meaning the second test would be true. Combining a true value with either a false or true will yield true.

In fact, by using what is called 'short-circuit evaluation', the third and fourth cases will never be evaluated, and the second will only be evaluated sometimes. Short-circuit evaluation is where the program can determine the total result of a combined expression that uses logical or/and. For example, if you have A || B || C, and A is true, there is no need to evaluate B or C (since regardless of their values, the net result will be true). Likewise, for A && B && C, if A is false, the result must be false, so there's no sense evaluating B or C.

__________________
And once again, Probability proves itself willing to sneak into a back alley and service Drama as would a copper-piece harlot.
- Vaarsuvius, Order of the Stick

DaWei

Your code is going to be redundant, anyway, because you're going to have to test again to decide whether to add, subtract, etc. Why don't you use a switch? The default case would then represent improper input.

__________________
Abstraction doesn't make it impossible to write bad code; it makes it possible to write superior code.
Contributor's Corner: Grumpy on C++ Exceptions DaWei on Pointers