1

This is a simple program for a class that prompts the user for the length of his or her shower in minutes (as a positive integer, re-prompting as needed) and then prints the equivalent number of bottles of water (as an integer).

It assumes the shower uses 1.5 gallons of water per minute (192 oz) and a plastic bottle size of 16 oz

My do-while loop successfully rejects negative numbers and 0, however, if I input text such as "foo" when prompted for the length of a shower in minutes, the program runs into an infinite loop, forever running the loop and printing "How long is your shower(in minutes)?:"

Any ideas how to refine the while condition to avoid this?

#include <stdio.h>

int min_to_bottles(int *n);

int main(void)
{
    int minutes;
    int bottles;
    do
    {
        printf("How long is your shower(in minutes)?:");
        scanf("%i", &minutes);
    }
    while (minutes < 1);

    bottles = min_to_bottles(&minutes);

    printf("Equivalent of bottles used per shower is: %i\n", bottles);

}

int min_to_bottles(int *n)
{
    int bottles;
    int oz = 192 * *n;
    bottles = oz/16;
    return bottles;
}
Danny_ds
  • 11,201
  • 1
  • 24
  • 46
Tomas Vrba
  • 21
  • 4
  • `while ((scanf("%d", &minutes) == 1) && (minutes < 1));`! There is a return value for `scanf()` just use it. This will not fix the problem, you still can have minutes wrong so set it to `-1` initially and check after the `while` loop to ensure it's correct. – Iharob Al Asimi Jan 15 '16 at 21:36
  • @iharob but why isn't scanf blocking after the first incorrect input? – Tarik Jan 15 '16 at 21:44
  • How do you know it's infinite? – Jasper N. Brouwer Jan 15 '16 at 22:13

3 Answers3

4

Always check the return value of scanf():

int result;
do {
    printf("How long is your shower(in minutes)?:");
    result = scanf("%d", &minutes);
    if(result != 1) 
        break;
} while (minutes < 1);

A shorter version (if only one scan is needed):

printf("How long is your shower(in minutes)?:");

while ((scanf("%d", &minutes) == 1) && (minutes < 1))
    ;

There is no need to use a pointer as parameter in int min_to_bottles(int *n);:

#include <stdio.h>

int min_to_bottles(int n)
{
    return (192 * n) / 16;
}

int main(void)
{
    int minutes = 0;
    int bottles = 0;

    printf("How long is your shower(in minutes)?: ");

    while ((scanf("%d", &minutes) == 1) && (minutes < 1 || minutes > 100))
        printf("Enter a number between 1 and 100 : ");

    // if(minutes == 0) here means invalid data was entered.
    //    so a check could be done before continuing.

    bottles = min_to_bottles(minutes);

    printf("Equivalent of bottles used per shower is: %d\n", bottles);

    return 0;
}

Initializing minutes to 0 will avoid calculating the bottles with an undefined value in case scanf() failed (by entering text for example).

Danny_ds
  • 11,201
  • 1
  • 24
  • 46
  • Thank you. I'm still quite the beginner with C, I appreciate any answer that helps me to better understand all the lower level aspects of programming. – Tomas Vrba Jan 15 '16 at 22:55
  • @TomasVrba - Glad to help. Don't hesitate to ask, and enjoy your C-journey! – Danny_ds Jan 15 '16 at 23:06
1

When you enter in text, it doesn't match the %i format specifier, so the text gets stuck in the input buffer and it keeps trying to read the same thing.

You need to flush the buffer if you didn't get a good match. You'll know if that the case by checking the return value of scanf, which returns the number of patters successfully matched.

int minutes = 0;
while (minutes < 1)
{
    printf("How long is your shower(in minutes)?:");
    int count = scanf("%i", &minutes);
    if (count < 1) {
        scanf("%*s");   // The * tells scanf to read the input but not assign it to anything
    }
}
dbush
  • 205,898
  • 23
  • 218
  • 273
0

Do not use scanf("%i",...).

The main problem is that bad input that did not convert to a number remains in stdin until another function reads it. As code did not check the return value of scanf(), the value of minutes is not known to be good and do ... while (minutes < 1); can easily repeated loop.

Solution: Read a line of input, convert to number, valid number:


To handle unexpected user input begin by reading a line of input

char buf[80];
if (fgets(buf, sizeof buf, stdin) == NULL) Handle_EOF();

Then parse the buffer for the number.

errno = 0;
char *endptr;
long num == strtol(buf, &endptr, 0);

// detect overflow and no conversiosn
if (errno || buf == endptr) Handle_BadInput();

// Ignore trailing white-space
while (isspace((unsigned char) *endptr) endptr++;

if (*endptr) Handle_BadInput();

Validate the number.

#define SHOWER_TIME_MIN (1 /* minute */)
#define SHOWER_TIME_MAX (60 /* minute */)
if (num < SHOWER_TIME_MIN || num > SHOWER_TIME_MAX) Handle_BadInput();

Put this all in a helper function

Example How to test input is sane

Community
  • 1
  • 1
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256