6
\$\begingroup\$

I made windows form in C# that takes the amount of change given, 1-99 and tells you the amount of quarters, dimes, nickels, and pennies you would get back. Can I improve the math in any way? It takes the total change and subtracts the value of the largest coin value that applies till the change becomes equal to zero.

        //set the variables
        int quarters = 0;
        int dimes = 0;
        int nickels = 0;
        int pennies = 0;
        int change = Convert.ToInt32(txtChange.Text);

        //logic
        do
        {
            if (change >= 25 && change <= 99)
            {
                change = change - 25;
                quarters++;
            }
            else if (change <= 24 && change > 9)
            {
                change = change - 10;
                dimes++;
            }
            else if (change == 5)
            {
                change = change - 5;
                nickels++;
            }
            else if (change <=4)
            {
                change = change - 1;
                pennies++;
            }

            txtQuarters.Text = quarters.ToString("n");
            txtDimes.Text = dimes.ToString("n");
            txtNickels.Text = nickels.ToString("n");
            txtPennies.Text = pennies.ToString("n");

        } while (change > 0);
\$\endgroup\$

closed as off-topic by paparazzo, Graipher, Edward, Heslacher, t3chb0t Sep 5 '17 at 4:32

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – paparazzo, Graipher, Edward, Heslacher, t3chb0t
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Why do you set the text properties inside your loop? \$\endgroup\$ – oerkelens Sep 3 '17 at 17:04
  • \$\begingroup\$ If you input change above 99, your loop never ends, by the way. If you check it beforehand, you don;t need o have it in your first if. \$\endgroup\$ – oerkelens Sep 3 '17 at 17:06
  • \$\begingroup\$ codereview.stackexchange.com/questions/169874/… \$\endgroup\$ – paparazzo Sep 4 '17 at 10:37
  • \$\begingroup\$ This fails on 6-9 and > 99. Why print out every loop? \$\endgroup\$ – paparazzo Sep 4 '17 at 17:45
5
\$\begingroup\$

Your loop only adds one coin to the change every repetition, which can be done faster: you don;t need the else if.

You don't want to set your Text properties inside your loop, but only when you're finished.

Also, you want to check if change > 99 before starting, otherwise your loop never ends.

Instead of

something = something + x;

you can use

something += x;

Once you're down to pennies, the number of pennies is your remaining change.

Instead of adding one coin every time, simply add all the quarters possible, then all the dimes, etcetera:

if (change > 99) throw new ArgumentOutOfRangeException("Too much change!");

while (change > 24)
{
    change -= 25;
    quarters++;
}
while (change > 9)
{
    change -= 10;
    dimes++;
}
while (change > 4)
{
    change -= 5;
    nickels++;
}

pennies = change;

Now, you can actually do away with all the loops very simply, by dividing the remaining change by the size of your next coin. The resulting integer is the amount of coins.

Then you get:

int quarters = change/25;
change -= quarters*25;
int dimes = change/10;
change -= dimes*10;
int nickels = change/5;
int pennies = change - nickels * 5;

Even shorter, using the % (modulo) operator:

int quarters = change/25;
change %= 25;
int dimes = change/10;
change %= 10;
int nickels = change/5;
int pennies = change%5;
\$\endgroup\$
  • \$\begingroup\$ Thanks! You really helped me out. Though, I'd have to make the loop change <= 99, because the change can be anywhere from 1 cent to 99 cents \$\endgroup\$ – weisbeym Sep 3 '17 at 20:12
  • \$\begingroup\$ @weisbeym Sure, even in the short version, I would keep the if (change > 99) in there, though there is no loop necessary for that :) \$\endgroup\$ – oerkelens Sep 4 '17 at 6:05

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