Infinite looping in hacker 1 - water

Question

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;
}

Answer 1

I like that you don't want to use CS50's tools and use tools that you are going to have available everywhere outside of CS50.

I was reviewing your code. I tried to solve it with a fflush(stdin) line, since I had that working for me on a Windows program in the past, but it did not work in this case.

However, I found this very useful post and built an answer for you based on it:

Create a char array of arbitrary size, read into it with fgets(), sscanf the string into your int variable and check for success. We can do this because sscanf returns the number of items successfully read.

Usage for fgets():

 fgets(char* buffer, int size, FILE* stream);

buffer is the char array or pointer where you are receiving the input.

size is the amount of bytes you are going to write to the buffer.

stream is the stream you are reading from (standard input in our case).

So I suggest you change your loop to something like this:

char line[256];
do
{
    printf("How long is your shower(in minutes)?:");

    // read input to string line
    fgets(line, sizeof line, stdin);

    // attempt to read an int from line
    if(sscanf(line, "%i", &minutes) != 1)
        minutes = 0;
}
while (minutes < 1);

I arbitrarily set minutes to 0 so the loop will continue.

Answer 2

Yes, don't use scanf. You have a library that helps you avoid using scanf. The reason is probably related to the fact that scanf still keeps the string you input in its buffer and it won't empty it, nor will it do anything useful with it. scanf is a function that will only work as expected when it reads what you expected (which means it's not really good for getting user input, which is always unreliable). You can read a bit more about it here: http://c-faq.com/stdio/scanfprobs.html

But, as I said, use GetInt() from the cs50 library.

Also, why are you making things so complicated? The calculation of minutes to bottles is very simple and it needs only one multiplication. There's no reason to do all that you do in your function and, above all, no reason to pass "n" by pointer. You're free to do things as simply or as complicated as you want, of course. But try to always choose simple, and avoid extra operations :)

Answer 3

Well, you can use gets() where the loop block ends. It works but the compiler will give you warning that it's a dangerous function.