Change dispenser

[adam2016]

Hi guys,

doing a question from a book, problem seems pretty simple enough to solve, my solution is partially correct but technically it's wrong as it shortchanges the customer by one cent.

I'm looking through the logic and I can't seem to find out why it never reaches 0 in my while loop, in my example I have 200.42 euro exactly and I buy an item for 88.55 euro, I enter the while loop I dispense a 100 note, subtract 100 then dispense a ten note, subtract 10 then dispense a 1 euro coin, subtract 1 euro then dispense a 50 cent coin, subtract 50 cent then dispense a 20 cent coin, subtract 20 cent then dispense a 10 cent coin, subtract 10 sent then dispense a 5 cent, and this is pretty much where my program stops working,

amountChangePaid now equals 0.02 so it should enter the next if statement ( amountChangePaid >= 0.02 ) but instead it skips this and enters the next statement which checks if amountChangePaid >= 0.01

here is the output note I have some debugging couts mixed in.

edit* also if I don't break in the final if statement the while loop will iterate infinitely

class Money{

   public:

   string makePureVirtual;
   string name;
   double amount;

   Money(){};
   Money(string name,double amount):name(name),amount(amount){};
   virtual string getMakePureVirtual()=0;
};

// notes / bills

class Bill : public Money{

   public:
       string makePureVirtualBill;
       Bill(){}
       Bill(string name,double amount): Money(name,amount){};
       virtual string getMakePureVirtualBill()=0;

};

class Five : public Bill{

public:
    Five():Bill("five",5){};
    virtual string getMakePureVirtualBill(){ cout << "now a class" << endl;}
    virtual string getMakePureVirtual(){ cout << "2" << endl;}
};

class Ten : public Bill{

public:
    Ten():Bill("ten",10){};
    virtual string getMakePureVirtualBill(){ cout << "now a class" << endl;}
    virtual string getMakePureVirtual(){ cout << "2" << endl;}
};

class Twenty : public Bill{

public:
    Twenty():Bill("twenty",20){};
    virtual string getMakePureVirtualBill(){ cout << "now a class" << endl;}
    virtual string getMakePureVirtual(){ cout << "2" << endl;}
};

class Fifty : public Bill{

public:
    Fifty():Bill("Fifty",50){};
    virtual string getMakePureVirtualBill(){ cout << "now a class" << endl;}
    virtual string getMakePureVirtual(){ cout << "2" << endl;}
};

class OneHundred : public Bill{

public:
    OneHundred():Bill("one hundred",100){};
    virtual string getMakePureVirtualBill(){ cout << "now a class" << endl;}
    virtual string getMakePureVirtual(){ cout << "2" << endl;}
};


// coins

class Coin : public Money{

   public:
       string makePureVirtualCoin;
       Coin(){}
       Coin(string name,double amount): Money(name,amount){};
       virtual string getMakePureVirtualCoin()=0;
};

class OneCent : public Coin{

public:
    OneCent():Coin("one Cent",0.01){};
    virtual string getMakePureVirtualCoin(){ cout << "now a class" << endl;}
    virtual string getMakePureVirtual(){ cout << "2" << endl;}
};

class TwoCent : public Coin{

public:
    TwoCent():Coin("two Cent",0.02){};
    virtual string getMakePureVirtualCoin(){ cout << "now a class" << endl;}
    virtual string getMakePureVirtual(){ cout << "2" << endl;}
};

class FiveCent : public Coin{

public:
    FiveCent():Coin("five Cent",0.05){};
    virtual string getMakePureVirtualCoin(){ cout << "now a class" << endl;}
    virtual string getMakePureVirtual(){ cout << "2" << endl;}
};

class TenCent : public Coin{

public:
    TenCent():Coin("ten Cent",0.10){};
    virtual string getMakePureVirtualCoin(){ cout << "now a class" << endl;}
    virtual string getMakePureVirtual(){ cout << "2" << endl;}
};

class TwentyCent : public Coin{

public:
    TwentyCent():Coin("twenty Cent",0.20){};
    virtual string getMakePureVirtualCoin(){ cout << "now a class" << endl;}
    virtual string getMakePureVirtual(){ cout << "2" << endl;}
};

class FiftyCent : public Coin{

public:
    FiftyCent():Coin("fifty Cent",0.50){};
    virtual string getMakePureVirtualCoin(){ cout << "now a class" << endl;}
    virtual string getMakePureVirtual(){ cout << "2" << endl;}
};

class OneEuro : public Coin{

public:
    OneEuro():Coin("one ",1){};
    virtual string getMakePureVirtualCoin(){ cout << "now a class" << endl;}
    virtual string getMakePureVirtual(){ cout << "2" << endl;}
};

class TwoEuro : public Coin{

public:
    TwoEuro():Coin("two",2){};
    virtual string getMakePureVirtualCoin(){ cout << "now a class" << endl;}
    virtual string getMakePureVirtual(){ cout << "2" << endl;}
};


vector<Money*> buySomething(double amount,double price){

     vector<Money*> money;

     if(amount < price)
        return money;

     double change = amount - price;
     cout << "CHANGE :: " << change << endl;
     double amountChangePaid = change;

     while(amountChangePaid > 0){

        cout << amountChangePaid << "top" << endl;

        if(amountChangePaid >= 100){

            amountChangePaid -= 100;
            money.push_back(new OneHundred);
            continue;
        }

        if(amountChangePaid >= 50){

            amountChangePaid -= 50;
            money.push_back(new Fifty);
            continue;
        }

        if(amountChangePaid >= 20){
            amountChangePaid -= 20;
            money.push_back(new Twenty);
            continue;
        }

        if(amountChangePaid >= 10){
            amountChangePaid -= 10;
            money.push_back(new Ten);
            continue;
        }

        if(amountChangePaid >= 2){
            amountChangePaid -= 2;
            money.push_back(new TwoEuro);
            continue;
        }

        if(amountChangePaid >= 1){
            amountChangePaid -= 1;
            money.push_back(new OneEuro);
            continue;
        }

        if(amountChangePaid >= 0.50){
            amountChangePaid -= 0.50;
            money.push_back(new FiftyCent);
            continue;
        }

        if(amountChangePaid >= 0.20){
            amountChangePaid -= 0.20;
            money.push_back(new TwentyCent);
            continue;
        }

        if(amountChangePaid >= 0.10){
            amountChangePaid -= 0.10;
            money.push_back(new TenCent);
            continue;
        }

        if(amountChangePaid >= 0.05){
            amountChangePaid -= 0.05;
            money.push_back(new FiveCent);
            cout << "five was entered" << endl;
            continue;
        }

        if(amountChangePaid >= 0.02){
            amountChangePaid -= 0.02;
            money.push_back(new TwoCent);
            cout << "two" << endl;
            continue;
        }

        if(amountChangePaid >= 0.01){
            amountChangePaid -= 0.01;
            money.push_back(new OneCent);
            break; // without this break loop will continue infinitely
        }
        cout << amountChangePaid <<  "end " << endl;
     }
     return money;
}


int main()
{

    vector<Money*> money = buySomething(200.42,88.55);


    for(int i = 0; i < money.size(); i++){

        cout << money.at(i)->name << endl;
    }
    return 0;
}

  CHANGE :: 111.87
111.87top
11.87top
1.87top
0.87top
0.37top
0.17top
0.07top
five was entered
0.02top
one hundred
ten
one
fifty Cent
twenty Cent
ten Cent
five Cent
one Cent

Thanks

[helios]

it shortchanges the customer by one cent

I'm not even going to read anything else. You're using double in your code, and that's a problem, because binary floating point is not capable of exactly representing simple decimal values such as 0.1 or 0.01*. When you write 0.01 in your code your compiler actually converts that to a value such as 0.01000000000761945, or 0.009999999999999817651856. In other words, something that's very close to, but not exactly, the value you wanted. Under most circumstances this isn't a problem, but if you want to accumulate sums and get an exact value by comparing with ==, you're going to encounter problems like yours, where you get errors by fractional amounts, even though in theory you should be getting an exactly correct answer.

What you should do is deal in cents (or whatever fraction you want) rather than monetary units. So if right now you're specifying 1 in your code, that becomes 100. 0.1 becomes 10, 0.01 becomes 1, and so on.
You can also work in tenths of a cent by using 1000, 100, and 10 for the aforementioned values.

Binary floating point is forbidden in any code that deals with money because these off-by-one-cent errors are so common.




* This is because 0.1 and 0.01 are repeating decimals in binary. 0.1 is exactly 0.0(0011) in binary.

[adam2016]

really interesting, I'm sure this has bound to have slipped through the cracks in some real world applications.

so checking doubles for equality is not reliable in any case?

also very good idea, I could use integers in that manner to represent cents, the only problem that I may have is converting lets say the amount an item is to an int, so a pair of shoes may cost 55.25 euro. that 55.25 would have to be converted to an int. I'm guessing this would be 5525 in the form we want? but we would need to convert it into this form first right?

[helios]

really interesting, I'm sure this has bound to have slipped through the cracks in some real world applications.

Yep, I've seen it first hand at work. We'd done the DB side for a client and we took care to use bignums, but they had used floating point on their web backend, so they kept getting off-by-one errors in their code.

so checking doubles for equality is not reliable in any case?

Well, not in every case. For example, this check:

double x = whatever();
if (random_function())
    x = 0;
//...
if (x == 0)
    do_something();

will reliably run do_something() if random_function() returned non-zero.
Equality can also be mostly reliable if your code only deals with integers (why are you using floating point then?); or if you only use additions, and multiplications by powers of two. For example, (1.0 / 2.0 + 1.0) / 4.0 + 3.0 is exactly equal to 3.375. But this is tricky to get right, so most of the time it's preferable to just do

bool approximately_equal(double x, double y, double epsilon){
    return abs(x - y) < epsilon;
}

where epsilon is some value that's considered negligibly small in that particular context.

the only problem that I may have is converting lets say the amount an item is to an int, so a pair of shoes may cost 55.25 euro. that 55.25 would have to be converted to an int. I'm guessing this would be 5525 in the form we want? but we would need to convert it into this form first right?

It just becomes a matter of user interface. You just need to implement these two functions:

int read_amount(std::istream &);
void write_amount(std::ostream &, int);

[adam2016]

Thanks Helios :)