While loop is stuck in infinite loop and I don't know why

Question

#include    <stdio.h>
#include    <stdlib.h>
#include    <ctype.h>
#include    <math.h>

int main(int argc, char * argv[])
{
   printf("This program tests your integer arithmetic skills.\n"
          "You should answer the questions following the same \n"
          "rules that computers do for integers arithmetic, not \n"
          "floating-point arithmetic. Hit the 'Enter' key after \n"
          "you have typed in your input. When you wish to finish \n"
          "the test, enter -9876 as the answer to a question.\n"
          "\n");

   int n1, n2, answer, user_answer, a, b, int_per;
   char op, c;
   float per, count, count_r, count_w;

   count = 0;
   count_r = 0;
   count_w = 0;

   printf("What is your question? ");
   scanf("%d %c %d", &n1, &op, &n2);


   do
   {
      count++;

      printf("What is %d %c %d ? ", n1, op, n2);

      if (op == '+')
      {
         answer = n1 + n2;
      }
      else if (op == '-')
      {
         answer = n1 - n2;
      }
      else if (op == '%')
      {
         answer = n1 % n2;
      }
      else if (op == '/')
      {
         answer = n1 / n2;
      }
      else if (op == '*')
      {
         answer = n1 * n2;
      }

      c = scanf("%d", &user_answer);

      if (user_answer == answer)
      {
         printf("Correct!\n\n");
         count_r++;

      }
      else if (user_answer == -9876)
      {
         count = count - 1;
         break;  
      }
      else if (c != 1)
      {
         printf("Invalid input, it must be just a number\n\n");
         printf("What is %d %c %d ? ", n1, op, n2);
      }
      else if (user_answer != answer)
      {
         printf("Wrong!\n\n");
         count_w++;
      }

   } while(user_answer != -9876);

   per = (count_r / count) * 100;

   a = (int) count_r;
   b = (int) count_w;
   int_per = roundf(per);

   printf("\nYou got %d right and %d wrong, for a score of %d%c\n", a,
          b, int_per, 37);

   return EXIT_SUCCESS;

}

The code above is supposed to loop through asking for questions and then the answer, until the user inputs -9876 as an answer then the program terminates and gives them their score.This all works EXCEPT!! for one thing. When the user enters a non number into the input. When this happens it is supposed to say "invalid input, please try again" then ask the same question again. For example

What is your question? 9+9

What is 9 + 9? hmmm, 8

invalid input, please try again

What is 9 + 9?

SO.. the user entered "hmmm" and instead of prompting the user with the same question again then scanning properly it just jumps into an infinite loop.I was wondering how to fix that.

Thanks

When you scan an integer with `scanf("%d", &value)` and the input isn't a valid integer, `scanf` returns 0 and the input goes back to where it started, i.e. before the non-numeric input. Subsequent calls will then try to re-parse the same invalid input over and over. You should `scanf("%*s")` to skip it. Better yet, read whole lines with `fgets` and `sscanf` into them. — M Oehm, Sep 23 '15 at 17:10
you are using space between %d %c %d in scanf function..that assign garbage value into among &n1, &op, &n2 variables — Neeraj Kumar, Sep 23 '15 at 17:40
@NeErAjKuMaR: No, it doesn't; blanks in the format string just tell `scanf` to skip over whitespace characters. — John Bode, Sep 23 '15 at 19:38

score 2 · Accepted Answer · answered Sep 23 '15 at 18:04

In the call

c = scanf("%d", &user_answer);

the %d conversion specifier expects to see a sequence of decimal digit characters; it will tell scanf to skip over any leading whitespace, then read decimal digit characters up to the first non-decimal-digit character, then convert and save the result to user_answer. If you type in 12aEnter, scanf will read and consume the '1' and '2' characters, assign the value 12 to user_answer and return 1 (for one successful conversion and assignment) leaving the 'a' and newline characters in the input stream.

When you type "hmmm", the first non-whitespace character is not a decimal digit, so scanf leaves it in place, doesn't assign anything to user_answer, and returns 0. All the remaining calls to scanf with a "%d" conversion specifier will do the same thing.

So, you need to make sure your scanf was successful, and if not, clear all characters out of the input stream before doing another read, something like the following:

if ( (c = scanf( "%d", &user_answer ) ) == 0 )
{
  /**
   * input matching failure, clear stray characters from input stream
   * up to the next newline character
   */
  while ( getchar() != '\n' )
    ; // empty loop 
}
else if ( c == EOF )
{
  // error occurred during input operation
}
else
{
  // do something with user_answer
}

You'll notice in my first example, the %d conversion specifier accepted the input "12a"; it converted and assigned 12 to user_answer, leaving the 'a' character in the input stream to foul up the next read. Ideally, you'd like to reject such badly formed inputs altogether. You could do something like the following:

/**
 * Repeatedly prompt and read input until we get a valid decimal string
 */
for( ;; ) 
{
  int c, dummy;
  printf("What is %d %c %d ? ", n1, op, n2);

  if ( ( c = scanf("%d%c", &user_answer, &dummy ) == 2 )
  {
    /**
     * If the character immediately following our numeric input is
     * whitespace, then we have a good input, break out of the read loop
     */
    if ( isspace( dummy ) )
      break;
    else
    {
      fprintf( stderr, "Non-numeric character follows input, try again...\n" );
      while ( getchar() != '\n' )
        ; // empty loop body
    }
  }
  else if ( c == 1 )
  {
    /**
     * No character following successful decimal input, meaning we
     * hit an EOF condition before any trailing characters were seen.  
     * We'll consider this a good input for our purposes and break
     * out of the read loop.
     */
    break;
  }
  else if ( c == 0 )
  {
    /**
     * User typed in one or more non-digit characters; reject the input
     * and clear out the input stream
     */
    fprintf( stderr, "Non-numeric input\n" );

    /**
     * Consume characters from the input stream until we see a newline.
     */
    while ( ( getchar() != '\n' )
      ; // empty loop body
  }
  else
  {
    /**
     * Input error or EOF on read; we'll treat this as a fatal
     * error and bail out completely.
     */
    fprintf( stderr, "Error occurred during read, panicking...\n" );
    exit( 0 );
  }
}

Another option would be to read your input as text, then use the strtol library function to convert it to the result type:

for ( ;; )
{
  char input[SIZE]; // for some reasonable SIZE value, at least 12 to handle
                    // a 32-bit int (up to 10 digits plus sign plus
                    // string terminator

  printf("What is %d %c %d ? ", n1, op, n2);
  if ( fgets( input, sizeof input, stdin ) )
  {
    char *check; // will point to first non-digit character in input buffer
    int tmp = (int) strtol( input, &check, 10 );
    if ( isspace( *check ) || *check == 0 )
    {
      user_answer = tmp;
      break;
    }
    else
    {
      fprintf( stderr, "%s is not a valid input, try again\n", input );
    }
  }
  else
  {
    /** 
     * error or EOF on input, treat this as a fatal error and bail
     */
    fprintf( stderr, "EOF or error while reading input, exiting...\n" );
    exit( 0 );
  }
}

That's my preferred method.

Either one of those messes would replace the line

c = scanf("%d", &user_answer);

After reading all that you're probably thinking, "interactive input in C sure is a pain in the ass". You'd be right.

score 1 · Answer 2 · answered Sep 23 '15 at 17:13

1

When there is an error in reading the input, you should clear the error state and skip the rest of the line before attempting to read more user input.

Replace the lines:

  else if (c != 1)
  {
     printf("Invalid input, it must be just a number\n\n");
     printf("What is %d %c %d ? ", n1, op, n2);
  }

by

  else if (c != 1)
  {
     printf("Invalid input, it must be just a number\n\n");

     // Clear the error states.
     clearerr(stdin);

     // Skip the rest of the line
     skipRestOfLine(stdin);

     printf("What is %d %c %d ? ", n1, op, n2);
  }

where skipRestOfLine() can be implemented as:

void skipRestOfLine(FILE* stream)
{
   int c;
   while ( (c = fgetc(stream)) != '\n' && c != EOF );
}

answered Sep 23 '15 at 17:13

R Sahu

204,454
14
159
270

Why `fgetc()` instead of `getc()`? – Jonathan Leffler Sep 23 '15 at 17:35
@JonathanLeffler, I don't understand why one would be better than the other. – R Sahu Sep 23 '15 at 17:58
`getc()` can be a macro; `fgetc()` is guaranteed to be a function. If `getc()` is a macro, it is because it can perform better. To some extent, this dates back to the days before inline functions. It's not a big issue, but it is often considered better to use `getchar()` and `getc()` than `fgetc()`. – Jonathan Leffler Sep 23 '15 at 18:09
@JonathanLeffler, thanks for the explanation. Learned something today. – R Sahu Sep 23 '15 at 18:13

score -1 · Answer 3 · answered Sep 23 '15 at 17:42

-1

scanf("%d%c%d",&n1, &op, &n2) use this code

answered Sep 23 '15 at 17:42

Neeraj Kumar

3,851
2
19
41

That doesn't solve the OP's problem, and introduces a new one. There at least needs to be a blank between the first `%d` and the `%c` conversion specifiers, or `op` will pick up the first character immediately following the first input, whether it's an operator or whitespace. – John Bode Sep 23 '15 at 18:06

While loop is stuck in infinite loop and I don't know why

3 Answers3