3
\$\begingroup\$

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 << "Welcome to Dollar Break Down!" << endl; //print welcome message
cout << "Please enter a dollar amount: ";        // prompt for input of a dollar amount

double amt;                                                         // var to store the amount
cin >> 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;                                                   // spacing between dollars and cents

dollars = amt;                                                  // reset dollars to amt for cents calculation
amt *= 10000;                                                   // multiply to remove rounding errors
dollars *= 10000;                                               // same thing
int cents = (amt - dollars) / 100;                              // compute cents without rounding errors

// compute cents
cents / 25 == 1 ?
    cout << cents / 25 << " quarter" << endl :
    cout << cents / 25 << " quarters" << endl;
cents = cents % 25;
cents / 10 == 1 ?
    cout << cents / 10 << " dime" << endl :
    cout << cents / 10 << " dimes" << endl;
cents = cents % 10;
cents / 25 == 1 ?
    cout << cents / 5 << " nickel" << endl :
    cout << cents / 5 << " nickels" << endl;
cents = cents % 5;
cents / 25 == 1 ?
    cout << cents / 1 << " penny" << endl :
    cout << cents / 1 << " pennies" << endl;
\$\endgroup\$
  • \$\begingroup\$ Where's main()? \$\endgroup\$ – Jamal Sep 25 '13 at 19:13
  • 3
    \$\begingroup\$ Why are you using ? here as oposed to if? \$\endgroup\$ – olevegard Sep 25 '13 at 19:30
  • \$\begingroup\$ Because it looks cleaner (to me) since the format is if one bill then singular, else plural and I preferred the look over if else statements, I did it both ways and liked how this way looked better. \$\endgroup\$ – Tyler Hills Dec 1 '13 at 19:25
2
\$\begingroup\$

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?
\$\endgroup\$
  • \$\begingroup\$ +1 good stuff. An array is good, but I think a map could work here as well. \$\endgroup\$ – Jamal Sep 26 '13 at 0:17
  • \$\begingroup\$ @Jamal good point! Updated. \$\endgroup\$ – TemplateRex Sep 28 '13 at 8:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.