Strange behaviour when reading in int from STDIN

Question

Suppose we have a menu which presents the user with some options:

Welcome:
1) Do something
2) Do something else
3) Do something cool
4) Quit

The user can press 1 - 4 and then the enter key. The program performs this operation and then presents the menu back to the user. An invalid option should just display the menu again.

I have the following main() method:

int main()
{
    while (true)
        switch (menu())
        {
            case 1:
                doSomething();
                break;
            case 2:
                doSomethingElse();
                break;
            case 3:
                doSomethingCool();
                break;
            case 4:
                return 0;
            default:
                continue;
        }
}

and the follwing menu():

int menu()
{
  cout << "Welcome:" << endl
       << "1: Do something" << endl
       << "2: Do something else" << endl
       << "3: Do something cool" << endl
       << "4: Quit" << endl;

  int result = 0;
  scanf("%d", &result);
  return result;
}

Entering numerical types works great. Entering 1 - 4 causes the program to perform the desired action, and afterwards the menu is displayed again. Entering a number outside this range such as -1 or 12 will display the menu again as expected.

However, entering something like 'q' will simply cause the menu to display over and over again infinitely, without even stopping to get the user input.

I don't understand how this could possibly be happening. Clearly, menu() is being called as the menu is displayed over and over again, however scanf() is part of menu(), so I don't understand how the program gets into this error state where the user is not prompted for their input.

I originally had cin >> result which did exactly the same thing.

Edit: There appears to be a related question, however the original source code has disappeared from pastebin and one of the answers links to an article which apparently once explained why this is happening, but is now a dead link. Maybe someone can reply with why this is happening rather than linking? :)

Edit: Using this example, here is how I solved the problem:

int getNumericalInput()
{
    string input = "";
    int result;

    while (true)
    {
        getline(cin, input);

        stringstream sStr(input);
        if (sStr >> result)
            return result;

        cout << "Invalid Input. Try again: ";
    }
}

and I simply replaced

int result = 0;
scanf("%d", &result);

with

int result = getNumericalInput();

Answer 1

When you try to convert the non-numeric input to a number, it fails and (the important part) leaves that data in the input buffer, so the next time you try to read an int, it's still there waiting, and fails again -- and again, and again, forever.

There are two basic ways to avoid this. The one I prefer is to read a string of data, then convert from that to a number and take the appropriate action. Typically you'll use std::getline to read all the data up to the new-line, and then attempt to convert it. Since it will read whatever data is waiting, you'll never get junk "stuck" in the input.

The alternative is (especially if a conversion fails) to use std::ignore to read data from the input up to (typically) the next new-line.

Answer 2

1) Say this to yourself 1000 times, or until you fall asleep:

     I will never ever ever use I/O functions without checking the return value.

2) Repeat the above 50 times.

3) Re-read your code: Are you checking the result of scanf? What happens when scanf cannot convert the input into the desired format? How would you go about learning such information if you didn't know it? (Four letters come to mind.)

_{I would also question why you'd use scanf rather than the more appropriate iostreams operation, but that would suffer from exactly the same problem.}

Answer 3

You need to verify if the read succeeded. Hint: it did not. Always test after reading that you successfully read the input:

if (std::cin >> result) { ... }
if (scanf("%d", result) == 1) { ... }

In C++ the failed state is sticky and stays around until it gets clear()ed. As long as the stream is in failed state it won't do anything useful. In either case, you want to ignore() the bad character or fgetc() it. Note, that failure may be due to having reached the end of the stream in which case eof() is set or EOF is returned for iostream or stdio, respectively.