Stuck in an infinite loop when validating user input as an int

Question

So I'm displaying a menu using a do while loop as shown below, and I want the user to be prompted with the menu until they make a valid selection- by entering the digits 1, 2, 3 or 4. I then want to use a switch case statement for identifying the users choice and executing the relevant block of code. When it comes to input validation though, how can I account for the user entering a letter instead of a numerical digit? The below code successfully continues to the next iteration to reprompt the user when they enter a letter, except that it enters a continuous loop.

int selection;

do{
    cout << "Which option would you like to select? (select 1, 2, or 3)";
    cout << "1: Option 1" << endl;
    cout << "2: Option 2" << endl;
    cout << "3: Option 2" << endl;

    if(!(cin >> selection)){
        cout << "Please select an integer from 1-4." << endl;
        cin.clear()
    }

}while(selection != (1) or (2) or (3) or (4));

I've tried inserting the code below using istringstream to stream the users response from a string into an int inside the while loop as an alternative method to try solving the problem but to no avail.

string temp;
cin >> temp;
clearInputBuffer();
istringstream is(temp);
is >> selection;

UPDATED CODE - still getting an infinite loop (only when the user enters an alphabetic
character; integers behaveas expected)

int selection;

do{
    cout << "Which option would you like to select? (select 1, 2, or 3)";
    cout << "1: Option 1" << endl;
    cout << "2: Option 2" << endl;
    cout << "3: Option 2" << endl;

    if(std::cin >> selection){
       cout << "Enter the new price: ";
    }

    else if(!std::cin.eof()){
       cout << "Please select an integer from 1-4." << endl;
       cin.clear();
    }


    }while(selection != 1 && selection != 2 && selection != 3 && selection != 4);

I bet you don't know what `selection != (1) or (2) or (3) or (4)` means — Kiril Kirov, Dec 17 '14 at 15:11

Armen Tsirunyan · Answer 1 · 2014-12-17T16:06:57.963

5

while(selection != (1) or (2) or (3) or (4));

is syntactically valid although you most probably wanted

while(selection != 1 && selection != 2 && selection != 3 && selection != 4);

Your original expression is equivalent to

while((selection != 1) || (2) || (3) || (4))

(2), (3), and (4) are evaluated to true, which makes your loop infinite, because anything || true is true.

If anyone should be wondering, yes, C++ allows writing and instead of && and or instead of ||, and not instead of !, etc. You have to "Disable Language Extensions" to see this on MSVC.

[update]

Another problem is that in case of a non-integer input, the variable selection stays uninitialized. In the else clase, give it a value of -1, for example.

edited Dec 17 '14 at 16:06

answered Dec 17 '14 at 15:12

Armen Tsirunyan

130,161
59
324
434

Ah yes, thanks @Armen Tsirunyan- that mistake looks incredibly obvious now that it's been pointed out to me. I have updated my code above to reflect these, and some other changes, however I am still stuck in an infinite loop when the user enters an alphabetic character – David Greaves Dec 17 '14 at 15:54
Well the behaviour is still exactly the same as before the update, however I can see how my while condition was flawed- so I am sure that atleast part of the update will help, though this is not reflected in the output of the code as yet. – David Greaves Dec 17 '14 at 16:14

Dietmar Kühl · Accepted Answer · 2014-12-17T15:26:15.070

The obvious approach is to actually check that the input was successful and if that is not the case deal with the error, e.g., write an error message, clear the stream, ignore the character or line, and try again:

if (std::cin >> selection) {
    // do something with the good selection
}
else if (!std::cin.eof()) {
    std::cin.clear();
    std::cout << "invalid character ('" << char(std::cin.get()) << "') ignored\n";
}

Your code checks the stream and clears it but it doesn't extract the offending character. By the time you get to the check whether the selection is in range things are already bad and will stay that way.

You'd than go on and check the range. Your approach doesn't quite work as the logic or operator evaluates each individual element. One way is to check if the entered value is member of a specific rnage, e.g., using

int const valid[] = { 1, 2, 3, 4 };
if (std::end() == std::find(std::begin(valid), std::end(valid), selection)) {
    std::cout << "chosen invalid selection (" << selection << ")\n";
}

The alternative of checking each selection individually may be viable for a small number of selection but isn't really viable when the range of options grows bigger. Admittedly, once you have a bigger range of options you'd actually put the key and the operation together anyway:

std::unordered_map<int, std::function<void()>> actions;
bool done = false;

// set up different operations for the respective actions, e.g.:
actions.insert(std::make_pair(1, [](){ std::cout << "hello, world\n"; }));
actions.insert(std::make_pair(2, [&](){ done = true; std::cout << "goodbye\n"; }))

int selection;
if (std::cin >> selection) {
    auto it = actions.find(selection);
    if (it != actions.end()) {
        (it->second)();
    }
    else {
        std::cout << "unknown action selection " << selection << '\n';
    }
}

score 0 · Answer 3 · answered Dec 17 '14 at 15:13

0

Try this

while(selection != (1) and selection != (2) and selection != (3) and selection != (4));

answered Dec 17 '14 at 15:13

geft

615
1
6
18

score 0 · Answer 4 · answered Dec 17 '14 at 15:13

selection != (1) or (2) or (3) or (4)

Non-zero integers will evaluate to true so this is the equivalent of:

(selection != (1)) or true or true or true

which will always evaluate to true.

The way to fix this is to compare each individually

while(selection != 1 && selection != 2 && selection != 3 && selection != 4)

score 0 · Answer 5 · answered Dec 17 '14 at 15:16

0

(2), (3, (4) are always true, that's why you are stuck in an infinite cicle. try:

while(selection != 1 && selection != 2 && selection != 3 and selection != 4);

answered Dec 17 '14 at 15:16

Jérôme · Answer 6 · 2014-12-17T16:07:24.057

0

A Shorter version:

while (selection <= 4 && selection >= 1)

edited Dec 17 '14 at 16:07

answered Dec 17 '14 at 15:17

Jérôme

8,016
4
29
35

Which is wrong, because the expression is always true. – Jesper Dec 17 '14 at 15:49

score 0 · Answer 7 · edited May 23 '17 at 10:27

I got it to work, the update provided in the original post was missing one extra line of code which I identified from sifting through other questions regarding validating input in a while loop. I think @Dietmar Kühl tried to illustrate something similar in his suggestion, however my compiler didn't like the code and I didn't quite understand it so I wasn't sure how to make it work. I'll be playing around with your suggestion regarding validating against a range though Dietmar so thank you for your input.

Thanks also to everyone who contributed to this thread, especially the 99% of you that did so without sounding condescending :-) There's always one though. The thread from which I identified my missing statement can be found here, and the line added to the code is identified with a comment in the code below. Thanks again!

int selection;

do{
    cout << "Which option would you like to select? (select 1, 2, or 3)";
    cout << "1: Option 1" << endl;
    cout << "2: Option 2" << endl;
    cout << "3: Option 2" << endl;

    if(std::cin >> selection){
        cout << "Enter the new price: ";
    }

    else if(!std::cin.eof()){
        cout << "Please select an integer from 1-4." << endl;
        cin.clear();
        cin.ignore(10000,'\n'); // this line eliminated the infinite loop issue
    }


}while(selection != 1 && selection != 2 && selection != 3 && selection != 4);

Stuck in an infinite loop when validating user input as an int

7 Answers7