Question

#include <cs50.h>
#include <stdio.h>

int main(void) {

    printf("Enter your change: ");
    int pennies = 0, nickels = 0, dimes = 0, quarters = 0;
    float change = GetFloat();

    while (change > 0) {
        if (change >= 0.25) {
            quarters++;
            change -= 0.25;
        }
        else if (change >= 0.10) {
            dimes++;
            change -= 0.10;
        }
        else if (change >= 0.05) {
            nickels++;
            change -= 0.05;
        }
        else if (change >=0.01) {
            pennies++;
            change -= 0.01;
        }
        // force break
        else {
        printf("%1.2f - Num. of change left\n", change);
        break;
        }
    }
    printf("Quarters: %d\n", quarters);
    printf("Dimes: %d\n", dimes);
    printf("Nickels: %d\n", nickels);
    printf("Pennies: %d\n", pennies);
    return 0;
}

Hello, I'm currently new to C and I'm taking Harvard's CS50 class online. The "change" variable seems to go down to 0.00 without stopping the while-loop. This forced me to type in "break" at the end. What is wrong with my code?

This is from problem set 1 by the way.

Was it helpful?

Solution

There are problems with how floating point numbers are represented in a computer memory. If in short: not all numbers can be stored precisely. Please read this page for more details: http://en.wikipedia.org/wiki/Floating_point#Accuracy_problems

You can use this service to check the representations of floats in a computer: http://www.binaryconvert.com/result_float.html

Regarding your case, let's assume you have entered 0.4. That means it should be divided into 0.25 + 0.1 + 0.05. And change is supposed to be zero, but:

0.40 == 0.4000000059604644775390625000,
 minus
0.25 == 0.2500000000000000000000000000 (exact),
 minus
0.10 == 0.1000000014901161193847656250,
 minus
0.05 == 0.0500000007450580596923828125
 is
0.00 == 0.0000000037252902984619140625

As you can see, the final result is slightly above zero, what prevents your loop from ending.

Generally, if you need to count money, you should use int instead to count "cents". Or custom types. Or long arithmetic. Or whatever, but not floating points, because money in most countries needs only two positions after a point, thus this point needs not to be floating.

OTHER TIPS

You should not use floating point numbers for representing numbers that are integral by nature. Floating point arithmetic is a problem here. Change in most cases will never be equal to 0. You should just do everything with integers and it will work. Algorithm looks fine.

http://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html

http://floating-point-gui.de/

As others have said, this is a floating point issue. Here's your problem:

else if (change >=0.01) {
            pennies++;
            change -= 0.01;
        }

What's probably happening is that change ends up very slightly higher than 0.01, so when you subtract 0.01 from it, it ends up being slightly greater than zero, but less than 0.01, and you have no if clauses to deal with that eventuality, so it goes on forever. Your printf() shows zero because you're rounding it down to two decimal places, so it looks like 0.00 even though it's probably 0.00001 or something.

it isn't wise to use floats for this, as stated a several times here...

but logically you can do what you are trying, and I suspect it has to do with comparison and promotion rules in C... but I would guess this will fix your problem:

while (change > 0.0f)

that way you are comparing like types...

really you should change to using int, long, or long long... and representing cents rather than dollars.

Untested, but here's how I'd have written it. It uses int rather than float to avoid rounding problems, has a table-driven computation for each of the coins, and does division rather than repeated subtraction.

int change = GetFloat() * 100 + 0.5;
struct {const char *name; int value;} coins[] = {
    {"Quarters", 25},
    {"Dimes", 10},
    {"Nickels", 5},
    {"Pennies", 1},
};
for (int i = 0; i < 4; i++) {
    int coin = change / coins[i].value;
    change %= coins[i].value;
    printf("%s: %d\n", coins[i].name, coin);
}

You should use the integer equivalent of getFloat(), or use an integer variable to do the comparisions. The code is going wrong because of floating point issues.

    #include<math.h>  // for round
...
...
    int change = int)100 * round(getFloat());
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top