5

I have a "minutes" variable that I'd like the user to enter a positive number into. 

int main(void)
{
    float minutes;
    minutes = -1;
    printf("Find out how many bottles worth of water your showers use!\n");
    printf("How many minutes do you spend in the shower? ");
    scanf("%f", &minutes);
    while(minutes < 0)
    {
        printf("Please enter a positive number: ");
        scanf("%f", &minutes);
    }
}

It works as intended for numbers. If minutes >= 0, it accepts it, and if minutes < 0 it keeps asking. If I enter a string it infinitely loops 

printf("Please enter a positive number: ");

and never gives me a chance to enter a new value. Why is this and how can I fix it? Thanks!

Nicholas Hassan
  • 949
  • 2
  • 10
  • 27
  • What do you intend with this program? What do you mean with negative and positive minutes? – user3078414 Jul 11 '16 at 16:42
  • Note that `scanf` isn't the safest way to get input in the first place, especially seeing as you want to handle the different possibilities of input. See the accepted answer [here](http://stackoverflow.com/questions/2430303/disadvantages-of-scanf) – iRove Jul 11 '16 at 16:49
  • Why is the title phrased as if this is a bug in C? – cat Jul 11 '16 at 17:06

2 Answers2

6

If you don't enter a numerical value, whatever you types stays in the input buffer. You can check for this by reading the return value of scanf, which tells you the number of items read. If it is 0, you can use getchar to read characters until the next newline to flush the buffer.

int main(void)
{
    int rval, c;
    float minutes;
    minutes = -1;
    printf("Find out how many bottles worth of water your showers use!\n");
    printf("How many minutes do you spend in the shower? ");
    rval = scanf("%f", &minutes);
    if (rval == 0) {
        while (((c = getchar()) != '\n') && (c != EOF));
    }
    while(minutes < 0)
    {
        printf("Please enter a positive number: ");
        rval = scanf("%f", &minutes);
        if (rval == 0) {
            while (((c = getchar()) != '\n') && (c != EOF));
        }
    }
}
dbush
  • 205,898
  • 23
  • 218
  • 273
  • Where do you declare and set 'rval' in the first pass? – user3078414 Jul 11 '16 at 16:51
  • @user3078414 Missed a copy/paste there. Fixed. – dbush Jul 11 '16 at 16:51
  • Great solution. +1 for using `getchar` which is the POSIX compliant way of clearing the buffer – Ishay Peled Jul 11 '16 at 16:52
  • You also have to set `rval` on the first `scanf` call, right? – thedouglenz Jul 11 '16 at 16:53
  • @thedouglenz Right. Fixed. – dbush Jul 11 '16 at 16:54
  • Or something like `while( (rval = scanf("%f", &minutes)) != 1 || minutes < 0) { if (rval == EOF) { printf("Ops...\n"; exit(EXIT_FAILURE);} if (rval == 0) { while (getchar() != '\n'); } printf("Please enter a positive number: "); }` – Bob__ Jul 11 '16 at 17:07
  • @dbush Thanks! I understand the problem now. Can you explain how while (((c = getchar()) != '\n') && (c != EOF)) works? It seems like there's just a condition and nothing actually being done, yet it clears the input buffer? Also it appears that in the second while, instead of rval = scanf("%f", &minutes); you can just use scanf("%f", &minutes); Does rval automatically get "updated?" – Nicholas Hassan Jul 13 '16 at 16:26
  • @NicholasHassan There's more than once thing happening in the `while` loop at once. It calls `getchar` to get a character from the buffer and assigns it to `c`, then we test to see if `c` is either a newline or EOF. Because the call to `getchar` happens as part of the conditional, the loop has an empty body. It's equivalent to `do { c = getchar(); } while ((c!='\n') && (c!=EOF));` – dbush Jul 13 '16 at 16:42
  • @NicholasHassan You still need to assign the return value of `scanf` to `rval`. Otherwise it will contain whatever value it had from the last time `scanf` was called. Doing `rval = scanf(...` assigns the value of *that particular function call* to `rval`. It does not permanently associate `rval` with what `scanf` returns whenever it is called. – dbush Jul 13 '16 at 16:45
  • @dbush Thanks again! The first part makes perfect sense. I see your point in the second one too - it just turns out that the code accidentally works such that you don't need the second rval. If a positive number is successfully entered, that whole while loop is skipped since minutes > 0. Crazy! – Nicholas Hassan Jul 14 '16 at 02:03
2

The %f conversion specifier tells scanf to stop reading input as soon as it sees a character that isn't part of a legal floating-point constant (i.e., something that isn't a digit, decimal point, or sign). That bad character gets left in the input stream, so the next call to scanf fails, and the next, and the next, etc.

You should always check the return value of scanf - it will tell you how many items were successfully read and assigned from the input stream. In this case, you're expecting a single item, so you should get a return value of 1. If you get a return value of 0, then it means the input is not a proper floating point value, and that bad input has to be cleared somehow. Here's one possible solution:

if ( scanf( "%f", &minutes ) == 1 )
{
  // process minutes as normal
}
else
{
  // clear everything up to the next whitespace character
  while ( !isspace( getchar() ) )
    ; // empty loop 
}

The only problem with this is that scanf is kind of dumb, and if you type something like 123fgh, it will convert and assign the 123 while leaving fgh in the input stream; you would probably want to reject that whole input completely.

One solution is to read the input as text, and then convert it using strtod:

char buffer[BUFSIZE]; // where BUFSIZE is large enough to handle expected input
...
if ( fgets( buffer, sizeof buffer, stdin ) )
{
  char *chk; // chk will point to the first character *not* converted; if
             // it's anything other than whitespace or the string terminator,
             // then the input was not a valid floating-point value.
  double tmp = strtod( buffer, &chk );
  if ( isspace( *chk ) || *chk == 0 )
  {
    minutes = tmp;
  }
  else
  {
    // input was not a proper floating point value
  }
}

This has the benefit of not leaving crap in the input stream.

John Bode
  • 119,563
  • 19
  • 122
  • 198