Avoiding a infinite loop caused by non integer input.

Started by
13 comments, last by Zahlman 14 years, 1 month ago
I am going through the book called "Beginning C++ through games development", i'm up to the end of chapter 8, trying to do the exercises and have come across a niggling problem I keep coming across in nearly all of the examples that ask for user input. The problem is that when a player enters something other than what the user input is asking for, the program enters an infinite loop. The user is supposed to enter an integer value when prompted. This works perfectly fine IF the user enters anything from 0 to 4. However the program enters an infinite loop if the user inputs a value that is NOT AN INTEGER. I have tried to reset the value of 'choice' to a value that is not between 0 and 4, but IS an integer at the end of the loop and/or the beginning, however the compiler just avoids this all together, and repeats the DO loop forever. I also ran the debugger in microsoft visual studio, and I noticed that after the player enters a choice that is a letter or something other than a number between 0 and 4, the value of choice becomes some long negative integer (i'm guessing due to the compiler converting characters and strings to integer?), it then skips over the switch statement (because the value does not equal anything from the switch statement), outputs the message saying the value is not valid, repeats the main loop again, but instead of getting user input, it seems to retain the previous value rather than stop at the user input and allow for a new input. So after all that long winded text, is this a simple problem for a beginner, or is it a little more complex than I think? I swear i've fixed something like this before, but this has me baffled. And what would I need to do to fix this. Also, is it possible to use a tag other than [code] which shows colour coding and so on? Just like in microsoft visual studio. [Yes; the other tag is [source], and I have made the necessary edits for you. --Zahlman] Thanks in advance, here is the following code i'm talking about.


//Critter Caretaker
//Simulates caring for a virtual pet

#include<iostream>

using namespace std;

class Critter
{
public:
	Critter(int hunger = 0, int boredom = 0);
	void talk();
	void eat(int food = 4);
	void play(int fun = 4);
	void stats();

private:
	int m_hunger;
	int m_boredom;

	int GetMood() const;
	void PassTime(int time = 1);
};

Critter::Critter(int hunger, int boredom):
m_hunger(hunger),
m_boredom(boredom)
{}

inline int Critter::GetMood() const
{
	return (m_hunger + m_boredom);
}

void Critter::PassTime(int time)
{
	m_hunger += time;
	m_boredom += time;
}

void Critter::talk()
{
	cout << "I'm a critter and I feel ";
	int mood = GetMood();
	if (mood > 15)
		cout << "mad.\n";

	else if (mood > 10)
		cout << "frustrated.\n";
	else if (mood > 5)
		cout << "okay\n";
	else
		cout << "HAPPY! :D \n";
	PassTime();
}

void Critter::eat(int food)
{
	cout << "Brruuppp\n";
	m_hunger -= food;
	if (m_hunger < 0)
		m_hunger = 0;
	PassTime();
}

void Critter::play(int fun)
{
	cout << "WHEEEE! THIS IS FUN!\n";
	m_boredom -= fun;
	if (m_boredom < 0)
		m_boredom = 0;
	PassTime();
}

void Critter::stats()
{
	cout << "Creature's Happiness level is: ";
	cout << Critter::GetMood() << "\n";
	cout << "Creature's Hunger level is: \n";
	cout << Critter::m_hunger << "\n";
	cout << "Creature's Boredom level is: \n";
	cout << Critter::m_boredom << "\n";
}

int main()
{
	Critter crit;
	crit.talk();
	int choice;
	
	do
	{
		
		cout << "\nCritter Caretaker\n\n";
		cout << "0 - QUIT\n";
		cout << "1 - LISTEN TO YOUR CRITTER\n";
		cout << "2 - FEED YOUR CRITTER\n";
		cout << "3 - PLAY WITH YOUR CRITTER\n\n";

		cout << "(DEVELOPERS MENU)\n";
		cout << "4 - (LIST CREATURES STATISTICS)\n";

		cout << "Please type a number to choose an action\n";
		cout << "Choice: ";
		cin >> choice;
		
		switch (choice)
		{

		case 0:
				cout << "Good-bye.\n";
				break;
		case 1:
				crit.talk();
				break;
		case 2:
				crit.eat();
				break;
		case 3: 
				crit.play();
				break;
		case 4:
				crit.stats();
				break;
		default:
			cout << "\nSorry, but " << choice << " is not a valid choice.\n";
			}
		
	}while(choice != 0);
	return 0;
		}


[Edited by - Zahlman on February 22, 2010 2:18:31 PM]
Advertisement
Add choice = 0 inside the default case of the switch:

default:     cout << "\nSorry, but " << choice << " is not a valid choice.\n";     choice = 0;


You can use source tags (just like the code tags, except using source instead of code in the tag) to get the color coding.

Edit - mattd has a much better answer
"All you have to decide is what to do with the time that is given to you." - Gandalf
When the user enters a line of text that cin (or any other istream) can't convert to the required type (int), cin (or the other istream) keeps the line of text in its stream buffer, and sets its fail flag bit.

You're meant to check whether the extraction (via operator >>) actually succeeded or not, and if not, clear the fail flag, read in and ignore the line entered, and retry.

Like this:

	cout << "Choice: ";	if(!(cin >> choice)) // check if extraction failed, it returns a bool        {            cin.clear(); // reset error flags            getline(cin, string()); // ignore the line entered            cout << "Please enter a number.\n"; // complain            continue; // retry        }
What mattd kind of makes sense, but i'd like to confirm that I understand it.

The way I read that code is it checks to see if the input is not an integer (choice)?

Then goes through a loop that first clears the input cin.clear() , which also clears those "flags" (could you please explain these? I'm guessing these "flags" basically work like a bunch of check boxes?).

Then the next line getline(cin, string()) I'm not too sure about. I think i've only seen this used once. Could you please explain what this line does in more detail?

I had a quick read in my other books at what getline is, and still i'm not sure how it ties into what you've written. You say it "ignores" the line entered. Is this because it gets the line in the input (even if it is just one character) stores it, then when getline reaches the end of the input value, it discards whatever was in input?....or am I just talking gibberish? :S

Sorry if these questions seem stupid. But I believe the the only stupid question is the sensible one that is not asked. If I don't ask, I can't really learn.
Quote:Original post by otreum
What mattd kind of makes sense, but i'd like to confirm that I understand it.

The way I read that code is it checks to see if the input is not an integer (choice)?

Right. It checks to see if a value couldn't be extracted from cin to choice (because it couldn't convert a line containing alphabetical characters to a number).

Quote:Then goes through a loop..

Not a loop, just a regular statement block.

Quote:..that first clears the input cin.clear() , which also clears those "flags" (could you please explain these? I'm guessing these "flags" basically work like a bunch of check boxes?).

clear only clears the flags. You can just think of flags as boolean values - have you read about those yet?

Quote:Then the next line getline(cin, string()) I'm not too sure about. I think i've only seen this used once. Could you please explain what this line does in more detail?

I had a quick read in my other books at what getline is, and still i'm not sure how it ties into what you've written. You say it "ignores" the line entered. Is this because it gets the line in the input (even if it is just one character) stores it, then when getline reaches the end of the input value, it discards whatever was in input?....or am I just talking gibberish? :S

You're not far off.

You'd normally use getline like this, to read in a line from a stream:
string line;getline(stream, line); // read from stream to line

However, we use:

getline(stream, string())

"string()" constructs a new string variable, and getline dutifully stores a line from the stream into it. The trick is that the string() value is just an unnamed temporary - so yes, it disappears right after getline is finished.

You could just use:
string ignored;getline(cin, ignored);

if the temporary value thing isn't making sense right now.

Quote:But I believe the the only stupid question is the sensible one that is not asked.

Too right.
Quote:Original post by mattd
However, we use:

getline(stream, string())

This has two problems. One, it isn't legal C++. A temporary can't be bound to a non-const reference. Your compiler may allow it as a non-standard extension, but on most compilers this will get rejected. Two, if you want to ignore what's in the buffer, istream has a member function for that: ignore().
For simple interactive programs like this, I recommend that you always read the standard input a line at a time (with std::getline), and then use the standard library class std::stringstream to re-parse that line of input. This makes it easier to ignore errors (you don't have to worry about resetting flags on std::cin, because they will be set on your temporary stringstream object instead, and you can just go through the loop again and it will be automatically thrown away and re-created with the next line of input) and to keep synchronized with the input (there is never any "left-over" data to ignore, because it's thrown away with the stringstream).

// std::stringstream comes from the library header <sstream>.int main() {	Critter crit;	crit.talk();	int choice;		do	{		cout << "\nCritter Caretaker\n\n";		cout << "0 - QUIT\n";		cout << "1 - LISTEN TO YOUR CRITTER\n";		cout << "2 - FEED YOUR CRITTER\n";		cout << "3 - PLAY WITH YOUR CRITTER\n\n";		cout << "(DEVELOPERS MENU)\n";		cout << "4 - (LIST CREATURES STATISTICS)\n";		cout << "Please type a number to choose an action\n";		cout << "Choice: ";		string line;		getline(cin, line);		// We set 'choice' to an invalid value first. That way,		// if the input is garbage, nothing is read in, so it's still		// invalid, and the default case of the switch is triggered.		choice = -1;		stringstream parser(line);		parser >> choice;		// If you want, you can do additional checking that there is		// no extra text at the end of the line, e.g. "76 trombones":		// char c;		// if (parser >> c) { choice = -1; }		// That way lets the user put spaces and tabs at the end, which		// I think is reasonable. If you want to be even stricter, try		// using parser.get().		switch (choice)		{		case 0:				cout << "Good-bye.\n";				break;		case 1:				crit.talk();				break;		case 2:				crit.eat();				break;		case 3: 				crit.play();				break;		case 4:				crit.stats();				break;		default:			// Notice the change here; we want the error message			// to reflect what the user actually typed.			cout << "\nSorry, but " << line << " is not a valid choice.\n";		}	} while(choice != 0);}


As an exercise, pull out the part of the code that gets the number from the user, and make it into a function.
Boost::lexical_cast can also be used as a solution to this.
Thanks for the responses again, I'm looking at Zahlmans response in particular, and TRYING to figure out how it works, but I seem to be stuck at a brick wall.

This morning I spent a good 1 and a half hours reading that code you posted and looking at my books to see what stringstream did and looking at what parser did, but I didn't find anything on parsers, I only found information on the stringstream, which now to my knowledge can read or write to a string.

I also ran the debugger on the code you wrote to try and understand it better, and I have learnt a bit of how it works, but I get stuck.

I'm going to write down what I think those few lines of code are doing based on what I learned from the debugger.

1. Creates a string variable called line. This will serve as a temporary value holder.

2. Getline reads/gets the user input and stores/writes it to 'line'.

3. THIS IS WHERE I HIT A BUMP.
Choice is then set to a non-valid value. So that if the string in 'line' is not an integer, then choice will go to the default arguement in the switch statement.
(But i'm stuck wondering how the compiler knows to not change the value of choice if the user input for line is something like 'h' or 'bobs your uncle')

4. THIS IS WHERE I HIT A BRICK WALL.
stringstream parser(line);parser >> choice;

I really don't understand what a parser is, and I vaguely understand what stringstream is, but haven't studied anything about stringstream just yet, i've only found it through the glossary of C++ primer this morning.
What I did study of stringstream, I didn't fully understand.

I had a look online for what a parser is in C++ and it looks like it could require a really complicated explanation, however for this example, perhaps you could explain the purpose of parser in these lines of code and in the extra code below that says:


char c;
if (parser >> c)
{
choice = -1;
}



I feel a bit stupid for asking all of this, but i'd like to try and understand this at least a little bit before I move on to the next chapter, even though the book doesn't require that I know this stuff just yet.

Thankyou in advance for any help provided. I'm sorry if i've missed something, i've just spent a few hours trying to figure out what to write in this post while at the same time understanding new things about the code while I was writing here, so i'm just going to leave the post at that and if I need to ask anymore questions, I'll be sure to ask ;)
A variable of type stringstream[b/] can be used exactly like cin except for two differences. First, it will read data from an internal string while cin reads data from a keyboard. Second, stringstream can be destroyed when you are done using it while cin will not be destroyed until after the end of main(). Both of these differences can make stringstream more useful for converting text to an integer than cin.

The problem is users are really bad at entering in data correctly for a computer to understand. If you look at *any* online form to enter data you will see that each box only gets *one* type of information. You never see a box that let's people type their entire address, just parts of it in each box. One for the name (with the name split up between first/last), street name/number, another for city, another for state, then another for zip code. Why not one box like this?
Sherlock Holmes221B Baker StreetLondon, England
Because being dumb a user might forget to put their name. Put the address above the name. Combine all lines into one line. And so on. Much easier to just read one line at a time.

Using stringstream like this is to accomplish that individual text box data entry in the console. You want to get an integer, but what's to stop someone from typing something you don't want. Like "21 years, 4 months, 3 days" instead of just "21" when you ask for the user's age. If you read in just one word you'll read 21, but then the input will still contain "years, 4 months, 3 days" which will screw up further data. You could just tell cin to ignore any other data saved in its internal buffer. Or, you can read an entire line of data at once clearing the buffer and then convert the line of data to whatever you want it to be. Doing it this way has the advantage of being incredibly consistent from a programming perspective. Sometimes you want a user to type "21 years, 4 months, 3 days" with all spaces. In that case you need to use getline. So why not always use getline?

Ok, so you have the line of input. It is all text at this point and we don't know what the heck is inside of it. But what we do know is we want to convert whatever is inside to an integer if possible. Zahlman went with "extract the first word from the string 'line' by letting stringstream convert it to an integer. If that fails, ignore the rest of the string by just letting stringstream get destroyed by going out of scope and repeat the process until the conversion works.

As for your question "what is a parser?" Doing this is parsing, basically.

C++: A Dialog | C++0x Features: Part1 (lambdas, auto, static_assert) , Part 2 (rvalue references) , Part 3 (decltype) | Write Games | Fix Your Timestep!

This topic is closed to new replies.

Advertisement