I am new to programming and I am taking CS50 2014 online through iTunes University. This is the change-making problem in Problem Set 1:
Write a program that first asks the user how much change is owed and then spits out the minimum number of coins with which said change can be made. Assume that the only coins available are quarters (25¢), dimes (10¢), nickels (5¢), and pennies (1¢).
I am pretty sure that my code is super "hacky." I would love understandable input on how I could have made a better algorithm or how I should have broken down the problem or "You did 'x' here"..."it would be better if you did 'y'."
#include <stdio.h>
#include <cs50.h>
#include <math.h>
int main(void)
{
// declare float
float money;
// Get user input and check if input is positive
do
{
printf("Please input the some amount of money you would like to make change for: \n");
money = GetFloat();
} while (money < 0);
// Get the Dollars from the Float
int getDollars = money;
// Get the cents from the Float
float getCents = roundf((money * 100) - (getDollars * 100));
int convertCents = getCents;
// Greedy Algorithm for Dollars
int getHundreds = getDollars / 100;
int getFifties = ((getDollars) - (100 * getHundreds)) / 50;
int getTwenties = ((getDollars) - (50 * getFifties) - (100 * getHundreds)) / 20;
int getTens = ((getDollars) - (20 * getTwenties) - (50 * getFifties) - (100 * getHundreds)) / 10;
int getFives = ((getDollars) - (10 * getTens) - (20 * getTwenties) - (50 * getFifties) - (100 * getHundreds)) / 5;
int getOnes = ((getDollars) - (10 * getTens) -(5 * getFives) - (20 * getTwenties) - (50 * getFifties) - (100 * getHundreds)) / 1;
//Greedy Algorithm for Cents
int getQuaters = convertCents / 25;
int getDimes = ((convertCents) - (getQuaters * 25)) / 10;
int getNickles = ((convertCents) - (getQuaters * 25) - (getDimes * 10)) / 5;
int getPennies = ((convertCents) - (getQuaters * 25) - (getDimes * 10) - (getNickles * 5)) / 1;
printf("\n");
if (convertCents == 0)
{
printf("The least possible steps you can make change from $%i.%i0 is %i. \n", getDollars, convertCents, (getHundreds + getFifties + getTwenties + getTens + getFives + getOnes + getQuaters + getDimes + getNickles + getPennies));
}
else if (convertCents < 10 && convertCents > 0)
{
printf("The least possible steps you can make change from $%i.0%i is %i. \n", getDollars, convertCents, (getHundreds + getFifties + getTwenties + getTens + getFives + getOnes + getQuaters + getDimes + getNickles + getPennies));
}
else
{
printf("The least possible steps you can make change from $%i.%i is %i. \n", getDollars, convertCents, (getHundreds + getFifties + getTwenties + getTens + getFives + getOnes + getQuaters + getDimes + getNickles + getPennies));
}
}
Also please forgive me I kind of went out of the scope of the problem. I appended it knowing the fact that I would like to use currently circulating paper denominations as well as coins.
-
1\$\begingroup\$ I don't have time to write an exhaustive review but I'll leave this as a comment: Never ever use floating point types for monetary values. \$\endgroup\$Emily L.– Emily L.2015年06月15日 15:34:56 +00:00Commented Jun 15, 2015 at 15:34
-
\$\begingroup\$ In your solution, why do you include paper money when the question asks only for quarters, dimes, nickels, and pennies? \$\endgroup\$user2023861– user20238612015年06月15日 18:17:46 +00:00Commented Jun 15, 2015 at 18:17
-
\$\begingroup\$ Good question. On iTunes University (through iTunes for OS X) the content is not laid out very well(This is not the case for the iTunes University App on iOS devices). So initially I was having a hard time finding the requirements and rubric for this assignment. Secondly, I thought this would be closer to a real world example. A cashier wouldn't give out a 100 dollars worth of quarters, so it seemed weird for me to try and create a program that would do this. \$\endgroup\$Brandon– Brandon2015年06月15日 18:31:36 +00:00Commented Jun 15, 2015 at 18:31
-
\$\begingroup\$ Perhaps you could tell us what you have learned so far, so we can better understand the limitations of your program. For example, have you learned about functions yet? Arrays? Structures? \$\endgroup\$JS1– JS12015年06月15日 19:42:05 +00:00Commented Jun 15, 2015 at 19:42
-
\$\begingroup\$ I understand a function to be a way in which you can write code that you would like to be repeated in a segment of code that can be called to reduce redundancy. I am a little cloudy as to how to set them up in C, because I haven't gotten that far. An array is a variable that can hold multiple values of the same type in C, i think. I do kind of understand other concepts, but as in my experience with trying to learn languages the hardest part for me is know when to use a certain code construct. I understand: do while, for, if, and ternary statements pretty well as far as flow control goes. \$\endgroup\$Brandon– Brandon2015年06月15日 20:01:50 +00:00Commented Jun 15, 2015 at 20:01
2 Answers 2
Printf format
I noticed that at the end of your program you had 3 cases due to trying to print the number of cents out correctly. You can do it all in 1 case by using the %02i
format. This format prints an integer with 2 digit width and pads the leading digits with zeros.
Simplifying the logic
Right now, your logic for each denomination gets more and more complicated as you go along because you are subtracting more and more things from the total on each step. If you simply subtracted the amount from the total right away, you wouldn't have to repeat it on each step. For example:
int getHundreds = getDollars / 100;
getDollars -= getHundreds * 100;
int getFifties = getDollars / 50;
getDollars -= getFifties * 50;
int getTwenties = getDollars / 20;
getDollars -= getTwenties * 20;
// etc.
Using functions
Now that you have simplified the logic, you can put that logic into a function. For example:
/**
* @param pAmount Points to amount of money. Returns the amount
* minus the amount used up by this denomination.
* @param denomination The value of this denomination.
* @return The number of units of this denomination.
*/
int makeChange(int *pAmount, int denomination)
{
int ret = *pAmount / denomination;
*pAmount -= ret * denomination;
return ret;
}
int main(void) {
// ...
int remainingDollars = getDollars;
int getHundreds = makeChange(&remainingDollars, 100);
int getFifties = makeChange(&remainingDollars, 50);
int getTwenties = makeChange(&remainingDollars, 20);
// etc.
}
Reducing the number of variables
Since you aren't actually printing out how many of each bill/coin you are using, you don't need so many variables. You only actually need to know how many bills plus coins you need. So you could change your code to:
int remainingDollars = getDollars;
int numBillsPlusCoins = 0;
numBillsPlusCoins += makeChange(&remainingDollars, 100);
numBillsPlusCoins += makeChange(&remainingDollars, 50);
numBillsPlusCoins += makeChange(&remainingDollars, 20);
// etc.
Using arrays
There's still a lot of repetition. It would be nice to not have to copy/paste one line per denomination. You can use arrays to handle each denomination in a loop instead. You just need to create an array with the possible bill denominations and one for the coin denominations.
Some other miscellany:
- I added my own
GetFloat()
function because I don't have whatever library you are using. - I removed the use of
roundf()
and just did the rounding myself. - I used a
DIM()
macro which returns the dimensions of a given array. You will encounter this in C a lot for loops which need to loop over a statically defined array.
Here is the final code:
#include <stdio.h>
#define DIM(array) (sizeof(array)/sizeof(array[0]))
float GetFloat(void)
{
float ret = 0;
scanf("%f", &ret);
return ret;
}
/**
* @param pAmount Points to amount of money. Returns the amount
* minus the amount used up by this denomination.
* @param denomination The amount for this denomination.
* @return The number of units of this denomination.
*/
int makeChange(int *pAmount, int denomination)
{
int ret = *pAmount / denomination;
*pAmount -= ret * denomination;
return ret;
}
int main(void)
{
// declare float
float money;
const int billDenominations[] = { 100, 50, 20, 10, 5, 1 };
const int coinDenominations[] = { 25, 10, 5, 1 };
// Get user input and check if input is positive
do
{
printf("Please input the some amount of money you would like to "
"make change for: \n");
money = GetFloat();
} while (money < 0);
// Get the Dollars from the Float
int numDollars = (int) money;
// Get the cents from the Float (rounded to nearest cent)
int numCents = (int) ((money - numDollars) * 100 + 0.5f);
int remainingDollars = numDollars;
int remainingCents = numCents;
int numBillsAndCoins = 0;
// Greedy Algorithm for Dollars
for (int i = 0;i < DIM(billDenominations);i++)
numBillsAndCoins += makeChange(&remainingDollars, billDenominations[i]);
// Greedy Algorithm for Cents
for (int i = 0;i < DIM(coinDenominations);i++)
numBillsAndCoins += makeChange(&remainingCents, coinDenominations[i]);
printf("\nThe least possible steps you can make change from "
"$%i.%02i is %i. \n", numDollars, numCents, numBillsAndCoins);
}
-
1\$\begingroup\$ Wow far more than I could have ever asked for! Thank you so much! \$\endgroup\$Brandon– Brandon2015年06月15日 21:10:15 +00:00Commented Jun 15, 2015 at 21:10
You should use a loop, the algorithm I suggest is this:
def minimum_coins (amount, coins):
coins_used = []
while amount:
coin = max(c for c in coins if amount >= c)
amount -= coin
coins_used.append(coin)
return len(coins_used)
of course translating this into low level C won't be short, but for sure shorter and more understandable than your implementation.
Let me make an example of my greedy algorithm, take the biggest possible coin each time:
How do I pay 117 cents using only [100, 20, 10, 5, 1] ?
- 117 - 100 = 17
- 17 - 10 = 7 # 100 and 20 would be too big
- 7 - 5 = 2 # 100, 20 and 10 would be too big
- 2 - 1 = 1
- 1 - 1 = 0
Using five coins, namely [100, 10, 5, 1, 1].
-
1\$\begingroup\$ I genuinely appreciate that advice. I kind of get the gist of your function. That being said you are using a lot of things I haven't learned yet. Im trying to stuck within what would have been learned in the first week. This is helpful though! \$\endgroup\$Brandon– Brandon2015年06月15日 16:34:39 +00:00Commented Jun 15, 2015 at 16:34
-
\$\begingroup\$ I don't understand so many downvotes, i just gave an alternative algoritmh and the OP is fine with it... \$\endgroup\$Caridorc– Caridorc2015年06月17日 08:22:24 +00:00Commented Jun 17, 2015 at 8:22
-
1\$\begingroup\$ No worries I'm not sure why either. \$\endgroup\$Brandon– Brandon2015年06月17日 14:22:51 +00:00Commented Jun 17, 2015 at 14:22
-
1\$\begingroup\$ @Brandon, hasn't upvoted your post Caridorc. and from what the comment says, you are using tools from a toolbox that the OP doesn't have. it looks like you are giving OP a good Algorithm in the Second part of your answer though. there are no upvotes on this answer yet. \$\endgroup\$Malachi– Malachi2015年06月30日 13:59:00 +00:00Commented Jun 30, 2015 at 13:59
-
2\$\begingroup\$ I must have hit the upvote twice because I did when he initially wrote the post intend to upvote it. \$\endgroup\$Brandon– Brandon2015年06月30日 14:14:16 +00:00Commented Jun 30, 2015 at 14:14
Explore related questions
See similar questions with these tags.