using fmod in a function, loop logic is returning unexpected value

Question

I am writing a program that takes the cost of an item, and the amount payed, and calculates how much of each coin you should get back in change (quarter, dime, nickel, penny). In the function that is calculating how many quarters are needed back, it always returns 2

float calculateCoins(float change)

{
    int x=1;
    float result=1;

    while (result>0)
    {
        result =fmod(change, (.25 * x));
        x++;
    }

    return x;

I am not sure what is wrong.

Also, excuse my inefficient code and probably ugly code, I am still learning

#include <iostream>
#include <math.h>
using namespace std;

float calculateChange(float, float);
float calculateCoins(float);

int main()
{
    float amountPay, amountDue, changeDue, quarter;

    cout << "This program calculates how much change should be returned "
         << "\nwhen a payment is made" << endl << endl;
    cout << "Please input the cost of the item:" << endl;
    cin >> amountDue;
    cout << endl << "Please input the amount paid:" << endl;
    cin >> amountPay;

    changeDue = calculateChange(amountDue,amountPay);
    quarter = calculateCoins(changeDue);

    cout << changeDue << endl;
    cout << quarter << " quarters needed";
    return 0;
}

float calculateChange(float amount, float payment)
{
    return payment-amount;
}
float calculateCoins(float change)
{
    int x=1;
    float result=1;

    while (result>0)
    {
        result =fmod(change, (.25 * x));
        x++;
    }

    return x;


}

Don't calculate with floats, they are not exact. Calculate with an integer type (`int` or `long int`) and with cents instead of dollars. — Werner Henze, Nov 26 '18 at 16:27
You should not be using floating point types to represent money. [Floating point math is hazardous](https://stackoverflow.com/questions/588004/is-floating-point-math-broken) and can make your life difficult. Instead, use an integer type and store the amount of cents you have. This way you can use integer math which will be exact. — NathanOliver, Nov 26 '18 at 16:28
@WernerHenze Technically speaking, floats are perfectly accurate if you only work with dollars and quarters. Not that you should use them for that, of course... — Max Langhof, Nov 26 '18 at 16:29
As for the question: Did you attempt to find the problem through [debugging](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) already? — Max Langhof, Nov 26 '18 at 16:33
If I had 10 cents for every time this has been asked on SO, I'd have... wait, I can write a program for that! — paddy, Nov 26 '18 at 16:35
What is your intention with `result =fmod(change, (.25 * x));`? What shall that code do? I don't see how that makes sense. — Werner Henze, Nov 26 '18 at 16:35
@WernerHenze The point of `result = fmod(change, (.25 *x));` was to take the remainder of `change / (.25 *x)` and store it in the variable `result` since you can't use % for floats — D-Russ, Nov 26 '18 at 16:42
That is what it does. But what can you do with that result? Think about when `fmod(change, .25*x)` is greater than zero (especially for x=1), what will then the result of `fmod(change, .25 * (x+1))` (especially for x=1) be? Can it change from >0 to <=0? — Werner Henze, Nov 26 '18 at 16:47
Good point, faulty logic on my part. I think since I was increasing X by 1 each time, `.25*x` would eventually be more than `change`, which I thought would return a negative value for the remainder, and when I got the first negative value the `count` had stored how many times `.25` can go into `change`, that would be the number of quarters. That is not how remainders work at all, that was a brain fart. — D-Russ, Nov 26 '18 at 16:55

score 1 · Accepted Answer · answered Nov 26 '18 at 16:46

1

Your problem is, that the logic behind function calculateCoins(float change) is wrong.

You first initialize result with a value of 1. In your loop, you check against result > 0.

In your first iteration, this will always be true since you have initialized result with 1. In your loop body, you change the value of result to the modulo and increase your value of x which is now 2. The loop only stops, if the modulo of your change and x is 0. This is clearly not what you expect.

Try this: cost = 1.1 paid = 2 and you end up with an infinite loop.

take this as a start point:

float calculateCoins(float change)
{
    int x=1;
    float result=1;

    while (change - x*0.25 > 0)
    {
        x++;
    }

    return x;
}

I'm not sure what your expected result is. Try to figure out what happens if you input strange numbers like cost = 1.1 paid = 2.

answered Nov 26 '18 at 16:46

user7431005

3,899
4
22
49

1

I downvoted because this function will make you poor if I come a million times and request my change of 3 cents. – Werner Henze Nov 26 '18 at 16:50
That's why I said `take this as a start point`. I do not want to simply provide a solution here because he is clearly learning and it will help him to figure out the rest by himself. – user7431005 Nov 26 '18 at 16:51
Ah this makes sense, I am not sure why I did result = 1, I think it was just so that result had a value to begin with so that the loop started. – D-Russ Nov 26 '18 at 16:52
I you don't want to give the answer, then you also have the option to write a comment. – Werner Henze Nov 26 '18 at 16:52
I gave an answer and fixed the problem with the code. It is no longer an infinite loop and works now. Unfortunately, it was not clear what the expected result is. It is clear that you will have a problem to return 0.01 cash if your program only allows you to return quarters, but this is clearly wanted by the program. I do not claim that this answer solves ALL problems with this code but the rest is for D-Russ to fix. – user7431005 Nov 26 '18 at 16:57
@WernerHenze It will not make you poor, this is only a small part of the program. I will also add functions that calculate (based on the remainder) how many dimes you should get back, nickels, and pennies – D-Russ Nov 26 '18 at 18:45
@D-Russ Check the given function. It returns x=1 even when change is only 0.03. If you give me a quarter everytime where I only should get 3 cents, then over time this can make *you* poor (not me). – Werner Henze Nov 26 '18 at 18:55
@WernerHenze You're right, I supposed I could find a way to check which change to start with – D-Russ Nov 26 '18 at 22:47

score -1 · Answer 2 · answered Nov 26 '18 at 17:09

@D-Russ. First your function CalculateCoins is returning an int but you demand a float as output. If this is what you really want, consider casting the result before returning it. Second, you should use the appropriate remainder function from the math library.

Your code could change to the following:

int calculateCoins(float change)
{
   int x=1;
   float result=1;

   while (result>0)
   {
      result =fmodf(change, (.25f * x)); // Note the use of "fmodf"
      x++;
   }

   return x;
}

In fact, the number of quarter returned is an int, doesn't it make sense. Note that in my suggested function, instead of float CalculateCoins(float change), I wrote int CalculateCoins(float change).

using fmod in a function, loop logic is returning unexpected value

2 Answers2