-3

The aim of this is to give an amount of change in the minimum number of notes/coins. So far, my code works fine for all values except those that include 2p or 1p coins, eg. 12.33 gives 1 £10, 1 £2, 1 20p, 1 10p, and 1 2p, but no 1p. I also tried just 2p which gives 1 1p and 1p gives 0 coins at all. I can't figure out what is wrong for these specific values, since the code is identical for each denomination.

#include <iostream>

using namespace std;

int main()
{
  float change;
  int fifty = 0;
  int twenty = 0;
  int ten = 0;
  int five = 0;
  int two = 0;
  int one = 0;
  int fiftyp = 0;
  int twentyp = 0;
  int tenp = 0;
  int fivep = 0;
  int twop = 0;
  int onep = 0;

  cout << "Please enter the amount of change given:\n";
  cin >> change;

  while ( change >= 50)
    {
      fifty++;
      change -= 50;
    }


  while ( change >= 20)
    {
      twenty++;
      change -= 20;
    }


  while ( change >= 10)
    {
      ten++;
      change -= 10;
    }


  while ( change >= 5)
    {
      five++;
      change -= 5;
    }


  while ( change >= 2)
    {
      two++;
      change -= 2;
    }


  while ( change >= 1)
    {
      one++;
      change -= 1;
    }


  while ( change >= 0.5)
    {
      fiftyp++;
      change -= 0.5;
    }


  while ( change >= 0.2)
    {
      twentyp++;
      change -= 0.2;
    }


  while ( change >= 0.1)
    {
      tenp++;
      change -= 0.1;
    }


  while ( change >= 0.05)
    {
      fivep++;
      change -= 0.05;
    }


  while ( change >= 0.02)
    {
      twop++;
      change -= 0.02;
    }


  while ( change >= 0.01)
    {
      onep++;
      change -= 0.01;
    }


    cout << "Give " << fifty << " £50 notes.\n"; 
    cout << "Give " << twenty << " £20 notes.\n";
    cout << "Give " << ten << " £10 notes.\n";
    cout << "Give " << five << " £5 notes.\n";
    cout << "Give " << two << " £2 coins.\n";
    cout << "Give " << one << " £1 coins.\n";
    cout << "Give " << fiftyp << " 50p coins.\n";
    cout << "Give " << twentyp << " 20p coins.\n";
    cout << "Give " << tenp << " 10p coins.\n";
    cout << "Give " << fivep << " 5p coins.\n";
    cout << "Give " << twop << " 2p coins.\n";
    cout << "Give " << onep << " 1p coins.\n";

    return 0;
}
user193369
  • 17
  • 4
  • 1
    The right tool to solve such problems is your debugger. You should step through your code line-by-line *before* asking on Stack Overflow. For more help, please read [How to debug small programs (by Eric Lippert)](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). At a minimum, you should \[edit] your question to include a [Minimal, Complete, and Verifiable](http://stackoverflow.com/help/mcve) example that reproduces your problem, along with the observations you made in the debugger. – πάντα ῥεῖ Oct 12 '16 at 10:49
  • Use modulo operations instead. Perhaps an array of denominations as well. – Ed Heal Oct 12 '16 at 10:55
  • 1
    You might also want to consider migrating to having your input in pence and just using an integer. – UKMonkey Oct 12 '16 at 10:56
  • 1
    All answers below are right but you should rethink the way you're actually coding, try learning about functions – Treycos Oct 12 '16 at 11:03
  • @EdHeal change >= 0.01 - nope, pounds – UKMonkey Oct 12 '16 at 11:06
  • @UKMonkey - Oops - my mistake – Ed Heal Oct 12 '16 at 11:10

3 Answers3

4

The problem is floating point maths. Consider adding the following line at the end of your program:

cout << "Change left: " << change << '\n';

Running your program with 12.33 as input results in:

Change left: 0.00999992

Clearly this is less than 0.01. Although you can try to add fudge factors to handle "small" deviations from perfect numbers, it is generally best to use integers for money, such that 1 equals 1 cent. You should do this conversion with the proper rounding:

long int cents = lrintf(change * 100.0);

Then replace your while loops like this:

while (cents >= 5000) {
  fifty++;
  cents -= 5000;
}

Et cetera.

G. Sliepen
  • 7,637
  • 1
  • 15
  • 31
  • Upvote for proposing money calculations being done with integer values. – Simon Kraemer Oct 12 '16 at 11:08
  • Once you have integers, you can use integer division and the modulo operator to get rid of the while loops, like so: `fifty = cents / 5000; cents %= 5000; twenties = cents / 2000; ...` – G. Sliepen Oct 12 '16 at 11:18
2

The problem is that float can't represent all decimal numbers, and 0.02 is actually slightly less than 0.02 (and 0.01 is less than 0.01).

The remedy is to do the calculations on integers representing your smallest denomination (that is, one penny).

Either let the user input the amount in pence, or read a string that you parse.
The latter method is both more complicated and user-friendly, but it's probably a good idea to get a working program using the former method first.
(Both methods let you ignore nonsense amounts like 3.14159265.)

I also recommend that you read about loops and arrays in your fine C++ book.

molbdnilo
  • 64,751
  • 3
  • 43
  • 82
  • Ahh thankyou! I'll change the input to pence instead. Out of interest, why are floats not the exact value? – user193369 Oct 12 '16 at 11:22
  • 1
    @user193369 See [Is floating point math broken?](http://stackoverflow.com/questions/588004/is-floating-point-math-broken). – molbdnilo Oct 12 '16 at 11:27
0

Answers below/above solves the question you're actually asking
But imo, i don't think you're actually asking yourself the right question
Here is another simpler way to make your program using arrays:

int main(){
    double change;
    double coins[12]{ 50,20,10,5,2,1,0.5,0.2,0.1,0.05,0.02,0.01 }; //All coins that can be given
    int coinsChanged[12]{0,0,0,0,0,0,0,0,0,0,0,0}; //All coins that are going to be given

    std::cout << "Please enter the amount of change given:\n";
    std::cin >> change;

    while (change >= 0.01) { //If the amount gets below the minimum coin you can give, the progrm stops
        for (int i = 0; i < 12; ++i) //Checks all available coins one by one
            while (change >= coins[i]) { 
                coinsChanged[i]++;
                change -= coins[i]; //Add the coin to the "giving" array
            }
    }

    for (int i = 0; i < 12; ++i)
        std::cout << "Give : " << coinsChanged[i] << "  £" << coins[i] << " notes" << std::endl; // Prints everything

    return 0;
}

If you want to change the amount/value of coins at any time, consider using vectors instead of arrays

Treycos
  • 7,373
  • 3
  • 24
  • 47