C++

Forum

 
Infinity Loop. What am I doing wrong here?

Sevvy (2)
Hello, new to programming. Watching videos while I create stuff to try and improve. So, here's my issue. When selecting New game, continue, and quit... everything works correctly. However, when I try to test it out using a deliberate error, it immediately spams the console with the error message and doesnt allow for another cin to correct this, even though it is written in the loop.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
 
   bool new_game {false};
    bool continue_game {false};
    int main_screen_choice;
    
    do {
        cout << "(1) New Game\t(2) Continue\t (3) Quit" << endl;
        cin >> main_screen_choice;
    
        if (main_screen_choice == 1) {new_game = true; 
            break;
            } else if (main_screen_choice == 2) {continue_game = true;
            break;
            } else if (main_screen_choice == 3) { 
            return 0;
            } else cout << "\nThat is not a valid selection, please try again.\n" << endl;
            }
while (continue_game == false && new_game == false);
dutch (2548)
You seem to be asking about how to handle non-integer (erroneous) input in your program. When attempting to read an integer, non-numeric input causes cin to go into an error state while leaving the input in the buffer. The error state must be cleared and the input buffer must be emptied.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
 
#include <iostream>
#include <limits>
using namespace std;

int main()
{
    int choice = 0;
    while (choice == 0)
    {
        cout << "1, 2, or 3: ";
        cin >> choice;
        if (!cin || choice < 1 || choice > 3)
        {
            cin.clear(); // reset error state
            // empty input buffer
            cin.ignore(numeric_limits<streamsize>::max(), '\n');
            cout << "Bad input. Try again.\n";
            choice = 0;
        }
    }
    cout << "Choice: " << choice << '\n';
}

Last edited on
Handy Andy (5051)
Hello Sevvy,

After reading your post and dutch's post this may help explain.

It all starts at line 7 with cin >> main_screen_choice;. This is known as formatted input. So "cin" looks at the variable that it is inputting to ad realizes it is an int, so it is expecting a whole number to be entered. Anything that is not a number will cause the "cin" to fail. And as dutch point out this need to be dealt with.

Because "cin" has failed it is possible that "main_screen_choice" will be given the value of (0) zero, unless your compiler is using pre-2011 standards, then the variable may contain the garbage value it has when defined.

Having said that it is a good practice to initialize all your variables when they are defined. If for no other reason to know that it does not contain a garbage value. This is more for numeric variables as a "std::string" is empty when defined and has no value that needs initialized.

The endless loop problem is because you entered something that is not a number and the value of "main_screen_choice" is either (0) zero or garbage. Either way the if statements will never be true and you end up at the while condition. Since the variables in the while condition have never changed this will evaluate to true and you start the loop over. This time "cin" is in a failed state and will not stop for any input, so you have the endless loop because nothing changes.

I made some changes to your program that you may find useful:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
 
#include <iostream>
#include <iomanip>
#include <string>
#include <limits>

int main()
{
    bool new_game{ false };
    bool continue_game{ false };
    int menu_choice{};  // <--- ALWAYS initialize your variables.

    do
    {
        std::cout <<
            "\n"
            "(1) New Game\n"
            "(2) Continue\n"
            "(3) Quit\n"
            " Enter Choice: ";
        std::cin >> menu_choice;

        if (!std::cin || (menu_choice < 1 || menu_choice > 3))
        {
            if (!std::cin)
            {
                std::cerr << "\n     Invalid Input! Must be a number.\n";

                std::cin.clear();
                std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');  // <--- Requires header file <limits>.
            }
            else if (menu_choice < 1 || menu_choice > 3)
            {
                std::cerr << "\n     Invalid Choice! Try again.\n";
            }

            continue;  // <--- Skips the if statements and go directly to the while condition.
        }

        if (menu_choice == 1)
        {
            new_game = true;
            //break;  // <--- This makes the previous line pointless because it leaves the do/while loop.
        }
        else if (menu_choice == 2)
        {
            continue_game = true;
            //break;
        }
        else if (menu_choice == 3)
        {
            break;  // <--- Changed. Just break out of the loop and let the program end.
        }
        else
        {
            std::cout << "\nThat is not a valid selection, please try again.\n\n";
        }
    } while (!continue_game && !new_game);  // <--- Changed.

    // <--- Keeps console window open when running in debug mode on Visual Studio. Or a good way to pause the program.
    // The next line may not be needed. If you have to press enter to see the prompt it is not needed.
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');  // <--- Requires header file <limits>.
    std::cout << "\n\n Press Enter to continue: ";
    std::cin.get();

    return 0;  // <--- Not required, but makes a good break point.
}

Line 57 is a better way to use what you have. Checking if the bool variable is == to true or false is just extra work that you do not need to do because the bool variable can only be (0) zero false or (1) true and you can use this fact.

I applaud your variable names they are descriptive and good, but for " main_screen_choice" "menu_choice" is sufficient, but does not meean that you have to change it.

In line 6 of your code the use of (\t) is easy, but when used after the first character it may not tab out the way that you want. Especially when you get to a program that displays a table. One or more lines may be set to far to the right. Better to use spaces or something else like "setw()". You may not be up to using "setw()" from the "<iomanip>" header file yet.

In the above code line 14 is an easier way to present a menu of choices and much easier to change or adjust as it looks very much like what will end up on the screen. Also all those quoted strings are considered 1 large string not seperate strings.

The next thing that you can take advantage of is when a "cout" is followed be a "cin" the "cin" will flush the output buffer before any input is taken. So you do not need any "endl" before the "cin".

Also prefer to use the new line (\n) over endl as much as possible. The "endl" is a function that takes time to process. The more that you have the more time it takes.

Hope that helps a bit,

Andy
Handy Andy (5051)
Hello Sevvy,

Sorry I had to leave the computer and lost what I was thinking about.

The above code looks like this when run:

(1) New Game
(2) Continue
(3) Quit
 Enter Choice: a  <---

     Invalid Input! Must be a number.

(1) New Game
(2) Continue
(3) Quit
 Enter Choice: 0  <---

     Invalid Choice! Try again.

(1) New Game
(2) Continue
(3) Quit
 Enter Choice: 5  <---

     Invalid Choice! Try again.

(1) New Game
(2) Continue
(3) Quit
 Enter Choice: 3  <---


Andy
seeplus (6457)
For simple functions to obtain an integer from the console (easily changed for other types such as double), consider:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
 
#include <limits>
#include <iostream>
#include <string>
#include <cctype>

int getInt(const std::string& prm)
{
	int i {};

	while ((std::cout << prm) && (!(std::cin >> i) || !std::isspace(std::cin.peek()))) {
		std::cout << "Not an integer\n";
		std::cin.clear();
		std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
	}

	std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
	return i;
}

int getInt(const std::string& prm, int minv, int maxv)
{
	do {
		std::cout << prm << " (" << minv << " - " << maxv << "): ";
		int i {};

		if ((std::cin >> i) && (i >= minv) && (i <= maxv) && std::isspace(std::cin.peek())) {
			std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
			return i;
		}

		if (!std::cin || !std::isspace(std::cin.peek()))
			std::cout << "Not an integer\n";
		else
			std::cout << "Input not in range\n";

		std::cin.clear();
		std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
	} while (true);
}

Sevvy (2)
Hey guys, first off, I wanted to say thank you for the help and support on this topic. Since I'm only about 6 hours into learning this I am obviously missing quite a lot of information that I am sure I will learn soon. The information provided worked wonders, and now I am off on an adventure to decipher and master it for future use. I appreciate the insight on future programming "do {" and donts. Little C++ joke for ya right there. Again, thank you guys very much.
Topic archived. No new replies allowed.