Infinite loop when validating input as integer

Question

I am having problems with the error handling portion of the mario assignment. When I try to check if the input is a non-integer my program gets stuck on an infinite loop.

Also, how should we check if the input is empty and print a prompt asking for input?

Below is the error handling portion of my code:

int n, height;
printf("How high do you want Mario to jump?\n");
printf("Enter a value between 0 and 23: ");
n = scanf("%d", &height);

while (height < 0 || height > 23 || height == 0 || n != 1)
{
    printf("Aww, come on, just go along with this...\n");
    printf("A value between 0 and 23, pretty please: ");
    scanf("%d", &height);
}

Answer 1

From the comments above, it seems that you may have solved this already by using GetInt() from the cs50 library (and you should use it of course). But just if you're curious about why that happened, here's what was causing the infinite loop problem.

Per the manual page for scanf()

The format string consists of a sequence of directives which describe how to process the sequence of input characters. If processing of a directive fails, no further input is read, and scanf() returns.

The main reason is that when the input doesn't match the format string, a matching failure occurs and the input is left in the standard input stream (stdin). So when scanf() is called the next time, it reads the same thing again and again and that's why we're stuck at an infinite loop.

To solve this problem, we need to get rid of the input in the input stream. Luckily, we can do that by reading it with a function like getchar() until we read a digit character (since we're looking for a number).

Once we read a digit (using getchar()), we give it back to the standard input stream using a function like ungetc() so that the next call to scanf() reads that and everything works as we expect.

int height;

do
{
    char c = getchar(); // read what's in the stdin

    if (isdigit(c))
    {
        ungetc(c, stdin); // give the digit back
        scanf("%d", &height); // scan it
    }    
}
while (height <= 0 || height > 23);

Now, there's still another problem. We shouldn't really reject the height of 0. I did that up here just for demonstration purposes. Unfortunately, 0 might be stored in height as a garbage value (even if a matching failure occurred). So our loop might not execute the next time if we didn't check whether height is equal to 0 and it was.

A simple fix to that is to initialize height with a random value out of the range [0, 23]. I chose -1. Now we can safely remove the = from height <= 0 and we still accept 0 as a height.

If a matching failure occurs though, -1 stays in height and the loop continuation condition is evaluated to true so the loop body executes again and if we got what we expect, it overwrites the -1 in height and all works perfectly.

int height = -1;

do
{
    char c = getchar(); // read what's in the stdin

    if (isdigit(c))
    {
        ungetc(c, stdin); // give the digit back
        scanf("%d", &height); // scan it
    }    
}
while (height < 0 || height > 23);

It's worth mentioning that using this solution though, if your input was something a mix between digits and non-digits, the first integer is read and everything else is ignored. So probably it's not a good thing since something like "abc10deg" is accepted!

One last thing in your code is that you're not really benefiting from n at all. If you input an integer that out of the specified range, this is regarded as a successful match by scanf() so n is equal to 1 in that case. Besides, you're not really updating n within the loop, so its value never changes after it was initialized the first time.