While loop carries on forever (C++)

Question

I'm still quite new to c++ and just experimenting with the language.

I have recently created a 'tictactoe' game.

I have created this simple looking function to get the users board position (from 1 to 9). It seems to work fine but I found a weird bug so to call it;

Whenever I enter a 12 digit number or higher the loop just carries on forever printing 'Invalid position!' and on to the next line 'Choose your position 1-9: '.

I am using visual studio to write code. Is it something with the code or is it perfectly fine? I'm eager to find out to learn from it.

Here's the function:

int getUserPosition()
{
    int position;
    while (true)
    {
        cout << " Choose your position 1-9: " << endl;
        cin >> position;
        if (position < 1 or position > 9)
        {
            cout << "Invalid position!" << endl;
        }
        else if (board[position - 1] == 'X')
        {
            cout << "This position has been taken!" << endl;
        }
        else
        {
            return position;
            break;
        }
    }
}

You should not be "experimenting" with C++. That does not tend to end well. Learn the basics from a good book first. — Baum mit Augen, May 08 '18 at 23:05
Test the stream state after `cin >> position;` You have no idea whether or not you read good data. If you read bad data, you want to discard the bad data and `clear` the error flags before continuing. Something like `if (cin >> position) { do stuff with position } else { clean up bad input }` — user4581301, May 08 '18 at 23:07
I have bought few books; I prefer to learn from project and from the mistakes I make in the process. Simply reading a book and copying down code within doesn't do the trick for me. — SzczureX, May 08 '18 at 23:07
Don't use `>>` for user input. It has the same problems as `scanf`. Also, code after `return` is unreachable. — melpomene, May 08 '18 at 23:08
The better books don't have you copying down code. They pose problems based on the material covered them to make you write the code and test your understanding of the material. — user4581301, May 08 '18 at 23:09
@einpoklum Not sure if I agree tbh. People at that level experimenting usually ends in "Hey, I can return without a return statement!" and similar fun. They cannot even know if their experiment worked because chances are they hit some sort of UB. — Baum mit Augen, May 08 '18 at 23:16
@SzczureX *"I prefer to learn from project and from the mistakes"* What I'm getting at is mainly that there are a lot of mistakes that appear to work anyway, but are still Undefined Behavior. Some of those mistakes are very subtle. Also, chances are you'll teach yourself bad practices, as the good way to do things is not always trivial to find through trial and error. — Baum mit Augen, May 08 '18 at 23:19
Have you used a debugger to investigate what's happening? If you prefer hands-on learning, a debugger is a necessity. — Stephen Newell, May 08 '18 at 23:25
If you are inputting a 12-digit number, that will be too large for a 32-bit signed int to hold. Which means that position is probably being populated by a garbage value. — tdk001, May 08 '18 at 23:37
Possible duplicate of [Unexpected behavior from cin when overflowing int](https://stackoverflow.com/questions/35898958/unexpected-behavior-from-cin-when-overflowing-int) — Silvio Mayolo, May 09 '18 at 04:00

score 3 · Accepted Answer · answered May 21 '18 at 16:37

I finally understand the actual behavior here. This is not invoking undefined behavior but rather defined behavior you don't want.

cin >> position;

This tried to read your 12 digit number, which didn't fit into an int, so the read failed. Because it failed on a format error, the 12 digit number remained in the buffer, so the next pass around the loop tried to read it again.

Always use cin::getline() when you intend to read keyboard input. The error/cleanup logic in cin is not designed to resync keyboard input, but rather to resync input piped from a generator that can't read your error messages. In the future, when you try to use cin with a pipe, the best solution is to check cin::fail() and bail the program if it ever gets set.

While loop carries on forever (C++)

1 Answers1