What is wrong with this logic

1
2
3
4
5
6
7
8
9
10
11
12
13
14
int main()
{
	cout << "Enter 1 or 2: ";
	int user_int; 
	cin >> user_int;

	while(user_int != 1 || user_int != 2)
	{
		cout << "Try again: ";
		cin >> user_int;
	}

	return 0;
}

When I run the above program, if I enter a 1 or a 2, the loop continues to repeat even though it seems that it should not. As far as I know, the loop should continue while user_int is not equal to 1 or 2, but if I input 1, the loop should read, while 1 is not equal to 1 OR 1 is not equal to 2. If user_int is 1 or 2, the loop continues anyway. When I take out the second part of the expression and just test, while(user_int != 1), and I input 1, then the program works correctly, it only messes up when I include the , || user_int != 2. Why is that? I also thought that the OR operator used short circuit evaluation, meaning that if the first operand is true it doesn't check the second, because only one operand need be true for the logic to be true. If that is true, then why would adding the second operand mess it up?
Last edited on
In order for that condition to be true, it must be "not equal to 1" OR "not equal to 2". If you enter 1, the second condition is true. If you enter 2, the first condition is true.

Try using &&. That way it'll only continue if the number entered is neither 2 nor 1. If it is either one of those, you want the whole condition to be false and not enter the loop.
Logical or expressions evaluate to true when either or both of the conditions are true.

Considering short circuiting:
When user_int is 1, the left side is true, so the entire expression evaluates to true and the loop continues.
When user_int is 2, the left side is false and the right side is true, so the entire expression evaluates to true and the loop continues.
When user_int is 3, the left side is true, so the entire expression evaulates to true and the loop continues.

Your looping expression always evaluates to true no matter what user_input happens to be.

Try: while (user_int != 1 && user_int != 2)
or: while (!(user_int == 1 || user_int == 2))
Okay, I am very confused. You say that if one of the operands is true, then the loop statement executes, but I tried typing in 5 as the user_int value, and the loop still executes, so the statement: while(5 != 1 || 5 != 2), still executes the loop. Why is that? It executes the loop no matter what number I enter.
Okay, I am very confused. You say that if one of the operands is true, then the loop statement executes, but I tried typing in 5 as the user_int value, and the loop still executes, so the statement: while(5 != 1 || 5 != 2), still executes the loop.

Correct. 5 != 1 is true. 5 is not equal to 1 is a true statement, so the loop continues.

Okay but what if I input 1. 1 != 1 is not a true statement, so the loop should terminate, but it doesn't. (I'm not trying to be difficult, I just really don't understand this)
Okay but what if I input 1. 1 != 1 is not a true statement, so the loop should terminate, but it doesn't.


1 != 1 is false, but 1 != 2 is true. And since only one of those two expressions need to evaluate to true in 1 != 1 || 1 != 2 the loop continues.

For logical OR, given an expression x and an expression y that evaluate to a boolean value:

 X | Y | Result
---+---+--------
 T | T |   T
 F | T |   T
 T | F |   T
 F | F |   F


For your loop control expression to ever evaluate to false, both sides must evaluate to false and because of the way you've formulated the expression, that cannot happen.

Okay, I get it now. This is embarrassing. My thinking was that, since OR has short circuit evaluation, if 1 != 1 is false, the loop should terminate, but short circuit evaluation only occurs if the first operand is true. If false, it tests the second operand. Wow. That was so simple and I made so complicated. Sorry about that, thank you so much for the help!
Hi,

One more thing :+)

Even though there are only 2 options, you could use a switch with a default case.

Check out this in the tutorial - the switch is near the bottom of that page.

http://www.cplusplus.com/doc/tutorial/control/

The other thing is to put the whole switch inside a bool controlled while loop:

1
2
3
4
5
6
7
8
9
10
11
12
13
bool Quit = false;

while (!Quit) {
    showmenu(); // create this function to print the menu
    // get input
    switch (input) {
     // .... various cases with break after each one
     // one of the cases sets Quit to true

     default:  // catches bas input
    
   }
}


Also, I have a personal vendetta against statements like this
while (input != 1 && input != 2) {

IMO it's ugly, error prone and non scalable. Use switch for a reasonable number of options (< 15 say), otherwise use a std::map<std::string, std::function> where the first type is the option - could be an unsigned int or char ; the function is called when that option is selected.

Good Luck !!
Wow, that is awesome. I'm always looking for better ways to write code, but I thought it would be asking to much to request people showing me ways of writing things better. Thanks so much!
closed account (48T7M4Gy)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
#include <iostream>

int main()
{
    int user_int = 0;
    
    while(
         ( std::cout << "Please enter 1 or 2: " and !(std::cin >> user_int) )
         or
         ( user_int != 1 and user_int != 2 )
         )
    {
        std::cout << "Error: You must enter 1 or 2\n";
        std::cin.clear();
        std::cin.ignore(1000,'\n');
    }
    
    std::cout << "Number selected: " << user_int << '\n';
    
    return 0;
}
Topic archived. No new replies allowed.