char type cause loop explosion

Question

I was wondering if anyone could tell me why the following code;

int main ()
{

        while((true))
        {
            int userChoice;
            //fprintf(stdout, "Press 1 for Coke.\nPress 2 for Sprite.\nPress 3 for Dr. Pepper.\nPress 4 for Mountain Dew.\nPress 5 for Monster.\nPress 6 for Help.\n\n");

            fprintf(stdout, "What would you like? (Press 6 for assistance): ");
            std::cin >> userChoice;
            if ((userChoice == 1))
            {
                fprintf(stdout, "\nYou get a Coke and you get a Coke, EVERYONE GETS A COKE!\n\n");
            }
            else if ((userChoice == 2))
            {
                fprintf(stdout, "\nDispensing Sprite.\n\n");
            }
            else if ((userChoice == 3))
            {
                fprintf(stdout, "\nDropping the Dr. P!\n\n");
            }
            else if ((userChoice == 4))
            {
                fprintf(stdout, "\nDo the Dew!\n\n");
            }
            else if ((userChoice == 5))
            {
                fprintf(stdout, "\nHere's your Monster, but don't go crazy\n\n");
            }
            else
            {
                fprintf(stdout, "\nPress 1 for Coke.\nPress 2 for Sprite.\nPress 3 for Dr. Pepper.\nPress 4 for Mountain Dew.\nPress 5 for Monster.\nPress 6 for Help.\n\n");
            }
        }

}

causes an infinite loop instead of printing the "else" statement when a non-integer is recieved via std::cin.

I'm assuming it is because userChoice is stored as an integer. How would I prevent this from happening? First guess would be to change it to a string or char but would like a better explanation...

Thank you in advance.

EDIT: From Comments;

Thank you for the answer and input;

The if's instead of switch statements are part of the "Learning". Im brand new, the "beginners challenge" wanted it in if's, then change it to use a switch.
the parentheses, Code::Blocks was throwing compiler warnings so i added them...
using the different printf/scanf etc makes sense.

Maybe more specifically i should ask; why is it that when i input a "6", it properly breaks and returns to the top of the loop but when i input an "a" it repeatedly prints out the else statement as fast as possible

@BatCoder please read the code careful. It's `while((true))` This is Super infinite. — bolov, Jun 15 '16 at 13:54
`fprintf(stdout, "What would you like? (Press 6 for assistance): "); std::cin >> userChoice;` is one of the most strange code I've ever seen. — SergeyA, Jun 15 '16 at 13:58
in the else statement you should `clear` and `getline` to clean `std::cin` for further input - better yet, you should accept input using `getline` and then parse that input, this also eats multiple space-separated inputs, allows outputting of parser error messages and would leave `std::cin` in working order automatically — BeyelerStudios, Jun 15 '16 at 14:00
You should pair `std::cout` with `std::cin` and `fprintf` with `scanf`. Don't cross the streams. — Thomas Matthews, Jun 15 '16 at 14:37
@ThomasMatthews , assuming fscanf?, but what is the reasoning behind not crossing them (other than the not crossing streams joke) :P. — Snow, Jun 15 '16 at 14:44
Synchronization between input and output. Also, console attributes - there may be differences in the console attributes between the two. — Thomas Matthews, Jun 15 '16 at 15:39

score 4 · Answer 1 · answered Jun 15 '16 at 13:55

4

When you work with *printf it is better to combine with scanf( "%d", &userChoice ) instead of cin;
Use printf( <format>, <arg ) instead of fprintf( stdout, ... - it is the same;
There are many unnecessary parentheses: while ((true)), if ((userChoice == x));
For more readable syntax use switch - case instead of all these if-s;
The infinite loop seems as expected with while ( true ) and without break nor return.

answered Jun 15 '16 at 13:55

i486

6,491
4
24
41

2

Why did you put actual answer as the last bullet point? I mean, your bullet points are OK, but I'd rather put the answer first. – SergeyA Jun 15 '16 at 14:01
1

@SergeyA 1-4 are as introduction :) – i486 Jun 15 '16 at 14:06
Thank you for the answer and input; - The if's instead of switch statements are part of the "Learning". Im brand new, the "beginners challenge" wanted it in if's, then change it to use a switch. - the parentheses, Code::Blocks was throwing compiler warnings so i added them... - using the different printf/scanf etc makes sense. Maybe more specifically i should ask; why is it that when i input a "6", it properly breaks and returns to the top of the loop but when i input an "a" it repeatedly prints out the else statement as fast as possible – Snow Jun 15 '16 at 14:09
As suggested, do not mix printf/cin. This makes the std library try to sync C I/O and C++ I(O, which can harm performance.I suggest using only cin/cout to get type safety. Sprintf and friends is another matter, those are OK. (snprintf is better than sprintf though) – Erik Alapää Jun 15 '16 at 14:19
@Erik: While I agree, the library will sync them whether you use `printf` or not. (however, you can fix this with `ios_base::sync_with_stdio(false);` – Jun 15 '16 at 14:31

score 1 · Accepted Answer · answered Jun 15 '16 at 14:30

1

Short answer: if you did proper error checking, you'd already know the answer. (or, at least, be closer than you currently are)

Long answer since userChoice is of type int: std::cin >> userChoice; fails when it tries to parse a. It does nothing to the value of userChoice and sets the fail bit on std::cin.

Since userChoice retains the previous value, it simply repeats whatever the previous choice does.

Furthermore, next time you execute std::cin >> userChoice;, you're doing it on a stream that's still in the fail state, and so the operation does nothing yet again.

To fix: test for errors. And if your error handling decides to continue looping, make sure to clear the flags to put std::cin back in the good state.

answered Jun 15 '16 at 14:30

I lament the fact that so many introductions to C and C++ I/O first teach people to do I/O without error checks. :( – Jun 15 '16 at 14:36
Would have a resource for "learning error checking", All i was aware of where compiler errors/warnings... Marking as answer for "Long answer since userChoice is of type int: std::cin >> userChoice; fails when it tries to parse a. It does nothing to the value of userChoice and sets the fail bit on std::cin. Since userChoice retains the previous value, it simply repeats whatever the previous choice does." Thank you. – Snow Jun 15 '16 at 14:37
One solution: initialization `int userChoice = 0` – i486 Jun 15 '16 at 14:40

score 0 · Answer 3 · answered Jun 15 '16 at 15:03

0

The simple solution for your simple program:

std::cin >> userChoice;  // insert the following lines
if (std::cin.fail())
{
  std::cin.clear(std::ios::goodbit);
  char c;
  std::cin >>c;
}

answered Jun 15 '16 at 15:03

Roland

336
2
8

char type cause loop explosion

3 Answers3