Simple Programming Task

[xNovak]

I saw a practise task on a website or forum somewhere yesterday and thought I'd give it ago tonight. It basically functions as a cashiers till, where the user inputs the total price of the goods, the payment given and then outputs the coins required for change. (I have used British Pounds, as I'm British.)

I can't see why my code isn't working, so I'm having to ask for some help here. The first parts work fine, asking for the prices and returning the total change needed. (All in pennies, as I wanted to test this simple version of the program before I expand to include verification and some other more "fancy" features.)

#include <iostream>

using namespace std;

int main()
{
    //Variables
    int price;
    int payment;
    int changeDue;
    int coin_1p;
    int coin_2p;
    int coin_5p;
    int coin_10p;
    int coin_20p;
    int coin_50p;
    int coin_100;
    int coin_200;
    int note_500;
    int note_1000;
    int note_2000;

    cout << "Enter the total price: ";
    cin  >> price;
    cout << "Enter payment amount: ";
    cin  >> payment;
    changeDue = payment - price;
    cout << "Total change due = " << changeDue << " pence.";

    while(changeDue > 0)
    {
        while(changeDue >= 2000)
        {
            changeDue = changeDue - 2000;
            note_2000++;
        }
        while(changeDue >= 1000)
        {
            changeDue = changeDue - 1000;
            note_1000++;
        }
        while(changeDue >= 500)
        {
            changeDue = changeDue - 500;
            note_500++;
        }
        while(changeDue >= 200)
        {
            changeDue = changeDue - 200;
            coin_200++;
        }
        while(changeDue >= 100)
        {
            changeDue = changeDue - 100;
            coin_100++;
        }
        while(changeDue >= 50)
        {
            changeDue = changeDue - 50;
            coin_50p++;
        }
        while(changeDue >= 20)
        {
            changeDue = changeDue - 20;
            coin_20p++;
        }
        while(changeDue >= 10)
        {
            changeDue = changeDue - 10;
            coin_10p++;
        }
        while(changeDue >= 5)
        {
            changeDue = changeDue - 5;
            coin_5p++;
        }
        while(changeDue >= 2)
        {
            changeDue = changeDue - 2;
            coin_2p++;
        }
        while(changeDue >= 1)
        {
            changeDue = changeDue - 1;
            coin_1p++;
        }
    }
    cout << "\n\nCoinage to recieve: \n";
    cout << coin_1p << " 1p's.\n";
    cout << coin_2p << " 2p's.\n";
    cout << coin_5p << " 5p's.\n";
    cout << coin_10p << " 10p's.\n";
    cout << coin_20p << " 20p's.\n";
    cout << coin_50p << " 50p's.\n";
    cout << coin_100 << " 1 pound coins.\n";
    cout << coin_200 << " 2 pound coins.\n";
    cout << note_500 << " 5 pound notes.\n";
    cout << note_1000 << " 10 pound notes.\n";
    cout << note_2000 << " 20 pound notes.\n";

    return 0;
}

Sorry for not commenting the code, I felt like it was pretty easy to see what was going on still at this early stage.

Something is going wrong with the "While" loops...but that's as much as I can tell.

[tfityo]

You forgot to initialize your variables.
Do something like it:
int coin_1p = 0;
or
coin_1p = 0;

[xNovak]

Well...now I just feel stupid. :P
Thanks for noticing it...it's one of those situations where the harder I looked, the less obvious the solution became.

One other thing. Is there a better way to write my code? I remember something from a book that talked about "if you're copy/pasting a line of code again and again, you're probably doing something the long way."

[zvRX92yv]

After 5 minutes of looking at it, it seems you forgot to initialize the variables to 0!

And I think it would be more space efficient if you used an array, use #define for the slots so it's easier to read.

[Peter87]

Do not use #define . There are better alternatives.

I see you write a lot of statements of the form a = a - b;. I just thought I should mention that there is a shorthand for that doing the same thing: a -= b;.
So instead of writing changeDue = changeDue - 2000; you can write changeDue -= 2000;.

[tfityo]

You can create array of denominations and than have a loop through it. The lines 30-87 will be compressed to 3-6 lines of code.
Also, as Peter87 suggested, use compound assignment operators.

[xNovak]

Thanks for all the help guys. I am enjoying adding functionality to this program, so I'll post it again once I've added some nice features.

*Edit* I've been trying to validate that the user enters a valid price for each good, however I can't seem to ensure that they enter an Int. I can make it so that they have to enter a number greater than 0, but when I test with a letter/string, the program loops the next part of good forever. Is there an easy way to only allow int's to be entered?

[jmbauer]

one suggestion to clean up your code would be to make parallel arrays so that you can use a function call on all of those while loops (except the first one of course). To do that just make 2 arrays that are parallel so they line up together:

#include <iostream>
using namespace std;

void calcChange(int & changeDue, int index, int [], int [] );

int main()
{
    //Variables
    int price;
    int payment;
    int changeDue;
	int coins[11] = { 1,2,5,10,20,50,100,200,500,1000,2000 };
	int count[11] = { 0 };   // keeps the count and initialized at zero
   
    cout << "Enter the total price: ";
    cin  >> price;
    cout << "Enter payment amount: ";
    cin  >> payment;
    changeDue = payment - price;
    cout << "Total change due = " << changeDue << " pence.";

    if (changeDue > 0)
    {
		for(int i = 10; i >= 0; i--)
		{
			calcChange(changeDue, i, coins, count);
			if (changeDue == 0)
				break;
		}
    }

    cout << "\n\nCoinage to recieve: \n";
	for(int i = 0; i < 11; i++)
		cout << count[i] << " " << coins[i] << "p's.\n";

    return 0;
}

void calcChange(int & changeDue, int index, int coins[], int count[] ) 
{
	 while(changeDue >= coins[index])  
	 {
		 changeDue -= coins[index];
         count[index] += 1;
     }
}

*Edit* I've been trying to validate that the user enters a valid price for each good, however I can't seem to ensure that they enter an Int. I can make it so that they have to enter a number greater than 0, but when I test with a letter/string, the program loops the next part of good forever. Is there an easy way to only allow int's to be entered?

I think you can make a while looping testing the ASCII values of the variable.

[bluecoder]

what is the code you have written for the validation part.
Are you entering the string and checking wheather the input string value is in .. in that case you can use .
ostreamstring ostring ;
if you are inputting float type value than you can convert it to int .. as

float  f ;
cin>>f ; 
int t = f ; 

if you are trying other thing . please past your code .