HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Breaking down a dollar amount into bill and coin denominations

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
coindenominationsamountintodollarbreakingdownbilland

Problem

I've started coding C++ again after a 6 month break to prepare for a class I'm taking this quarter. I found this idea and thought it would be fun to try and code.

I originally started with input as a string, and wanted to split it into two ints at ., but this way seemed easier. Then came the rounding errors. This is why I multiply by such a large number, and 100 was still giving me incorrect values. I'm really just looking for any feedback on ways I could improve my code, stylistically or otherwise.

```
cout > amt; // read in and store amount
cout << endl; // spacing

int dollars = amt; // dollars = (int)amt ex: int(45.97) = 45

// compute dollars
dollars / 100 == 1 ? // check if bill amount is single or plural
cout << dollars / 100 << " hundred" << endl : // print singe
cout << dollars / 100 << " hundreds" << endl; // print plural
dollars = dollars % 100; // compute remainder after removing bill amount
dollars / 50 == 1 ?
cout << dollars / 50 << " fifty" << endl : // repeat for each bill type
cout << dollars / 50 << " fifties" << endl;
dollars = dollars % 50;
dollars / 20 == 1 ?
cout << dollars / 20 << " twenty" << endl :
cout << dollars / 20 << " twenties" << endl;
dollars = dollars % 20;
dollars / 10 == 1 ?
cout << dollars / 10 << " ten" << endl :
cout << dollars / 10 << " tens" << endl;
dollars = dollars % 10;
dollars / 5 == 1 ?
cout << dollars / 5 << " five" << endl :
cout << dollars / 5 << " fives" << endl;
dollars = dollars % 5;
dollars / 1 == 1 ?
cout << dollars / 1 << " one" << endl:
cout << dollars / 1 << " ones" << endl;

cout << endl;

Solution

Stylistically:

  • just write std:: instead of using namespace std; (which you didn't show but was implicit)



  • use a regular if() instead of the ternary operator ?



  • use compound assignment operators like dollars %= 10; instead of dollars = dollars % 10;



  • I'm not a big fan of end-of-line comments, but at the very least, try and put them inside the 80 column mark (most editors will show a marker)



Furthermore, it's a good exercise to make your code more general:

  • put the bills (1, 5, 10, 20, 50, 100) and their strings in a std::map, and write your code as a loop over that map. This way you can start to test with only small bills, and then expand to the general problem



  • a propos, where are your tests?

Context

StackExchange Code Review Q#31811, answer score: 2

Revisions (0)

No revisions yet.