2

I am very new to C. I come from a python background. I would like to know where I went wrong with my code.

I am doing the cs50 greedy problem. What is wrong with my code? It works with some numbers but others don't work. I am trying to get an input from the user asking how much change to give back, then calculate the minimum number of coins I can give back using only $.25, $.10, $.05, $.01

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


int main(void)
{
    float n;
    do
    {
        n = get_float("How much change is owed?\n");
    }
    while(n == EOF); 
    int minimumamountofcoins = 0;
    if (n/.25 >=1){
        do 
        {
            n -= .25;
            minimumamountofcoins++;
        }
        while (n/.25 >= 1);
    }
    if (n/.1 >=1){
        do
        {
            n -= .1;
            minimumamountofcoins++;
        }
        while (n/.1 >=1);
    }
    if(n/.05 >=1){
        do
        {
            n -= .05;
            minimumamountofcoins++;
        }
        while (n/.05 >=1);
    }
    if (n/.01 >=1){
        do
        {
            n -= .01;
            minimumamountofcoins++;
        }
        while (n/.01 >=1);
    }
    printf("The minimum amount of coins is %d\n", minimumamountofcoins);
}

New code: (works perfectly except when entering 4.2)

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


int main(void)
{
    float n;
    do
    {
        n = get_float("How much change is owed?\n");
    }
    while(n == EOF);

    int cents = (int)(n * 100);
    int minimumamountofcoins = 0;
    if (cents/25 >= 1){
        while (cents/25 >= 1)
        {
            cents -= 25;
            minimumamountofcoins++;
        }

    }
    if (cents/10 >= 1){
        while (cents/10 >= 1)
        {
            cents -= 10;
            minimumamountofcoins++;
        }
    }
    if(cents/5 >= 1){
        while (cents/5 >= 1)
        {
            cents -= 5;
            minimumamountofcoins++;
        }
    }
    if (cents/1 >= 1){
        while (cents/1 >= 1)
        {
            cents -= 1;
            minimumamountofcoins++;
        }
    }
    printf("The minimum amount of coins is %d\n", minimumamountofcoins);
}
R_C
  • 333
  • 2
  • 6
  • 17
  • 1
    Why not give an example of an input for which your code doesn't work? For that input, say what output you expect and what your program returns instead. – John Coleman Aug 16 '17 at 00:19
  • Please research "change" questions. The advice is to work with integers and cents. [Is floating point math broken?](https://stackoverflow.com/questions/588004/is-floating-point-math-broken) – Weather Vane Aug 16 '17 at 00:21
  • 1
    A few suggestions: use integers (with all monetary amounts in pennies), you don't need the final .01 while loop (the remaining amount *is* the number of pennies); replace the word "amount" throughout by "count", – jarmod Aug 16 '17 at 00:23
  • What is cs50.h ? Does it contains definition of get/_float(...) function? – MCG Aug 16 '17 at 00:24
  • Similar code [Printing values of float and int for a simple calculation](https://codereview.stackexchange.com/q/173091/29485) recently reviewed. – chux - Reinstate Monica Aug 16 '17 at 00:45
  • 1
    @MCG look at the tag info for `cs50`. It contains a library that handles certain basic IO tasks, with the theory that it allows the students to concentrate on what is more important. Personally, I think it a bad idea in the sense that in C such details *are* one of the important things that a student needs to learn. – John Coleman Aug 16 '17 at 00:52
  • Rather than `while (n/.1 >=1);`, use `while (n >= (0.01 - 0.005));` ... – chux - Reinstate Monica Aug 16 '17 at 02:05

1 Answers1

2

Since you did not include test cases, I did my own. Here are some of the cases for which your algorithm does not return the correct answer:

.04, .11, .17, .19, .21, .26, .32, etc.

These cases all fail when calculating the number of pennies (in the final do-while loop), and they all return one less coin then they should. This is because of error with floating-point numbers. With print statements, I discovered that when the division for the final penny was being calculated, the same thing occurred each time:

n/.01 = 0.99999999

This is obviously not intended, as this should be equal to 1, and the final penny should be added to the total. Thus, your code, which works in theory, is broken because of floating-point numbers.

To avoid this, you could do any number of things, such as keeping track of dollars and cents separately as integers, change the condition to be n/.01 >= .9999 instead of n/.01 >= 1, treat the amount of money you are doing the calculations on as an integer number of cents, or any other number of things.

Personally, I prefer that last option, treating the amount of money as an integer number of cents. This is easy to do, as all you have to do to convert from dollars to cents is multiply by 100. Thus, the easiest thing to do is to use this same algorithm, except use integers.

Thus, the code would look something like this:

int main(){
    float n;
    //code that parses in the amount of money, n now stores that amount
    int cents = (int)(n * 100);
    int minimumamountofcoins = 0;
    if (cents/25 >= 1){
        while (cents/25 >= 1)
        {
            cents -= 25;
            minimumamountofcoins++;
        }

    }
    if (cents/10 >= 1){
        while (cents/10 >= 1)
        {
            cents -= 10;
            minimumamountofcoins++;
        }
    }
    if(cents/5 >= 1){
        while (cents/5 >= 1)
        {
            cents -= 5;
            minimumamountofcoins++;
        }
    }
    if (cents/1 >= 1){
        while (cents/1 >= 1)
        {
            cents -= 1;
            minimumamountofcoins++;
        }
    }
    printf("The minimum amount of coins is %d\n", minimumamountofcoins);
}
Lavaman65
  • 863
  • 1
  • 12
  • 22