So my problem is in problem set 1 of week 1 of the CS50 Harvard course. The task was writing a program that calculates the smallest amount of coins possible when getting change. I've already written the program and it works, but I wanted to know if any of you can tell me how to further improve my function that calculates the amount of coins. It is basically the same lines of code copied and pasted 4 times and just changing the coin value inside of it (25, 10, 5, 1). Can anyone help me transform this into some sort of loop, since although copying & pasting works it feels kind of bad to look at for me.

int calculate_coins(int change)
//Count coins from 0
int n = 0;

//How much 25s can be used? 
n += change / 25;
//Calculate remaining change
change %= 25;

n += change / 10;
change %= 10;

n += change / 5;
change %= 5;

n += change / 1;
change %= 1;

return n;

Its good that you're thinking about how to make the code more efficient. Kudos to you!

Yes, repeating code is a red flag that things can be simplified. But here, the code is very similar but not the same. It's only the same pattern performed on different variables. It's good that you recognize that there may be a better way, but it's also very important to recognize the difference between very similar code and identical code. In this case, if you were writing the same code over and over many more times, then it would be appropriate to write a function to do the work. But, since it's only 4 times, and they're all bunched together, this is still better. The overhead of a function would outweigh it's usefulness.

You'll learn more about code efficiency in a later lesson! ;-)

Interestingly, this is a case where the code you wrote is about as efficient as it can get. There are two lines of simple code to calculate how many of each type of coin and how much change is left. That means two calculations per coin, or 8 calculations overall.

If you were to add a loop, that would mean all the overhead of a loop, and calculations inside each pass would be multiplied by the number of passes. While it may be good practice in writing loops, adding any loops here would just make the code more complex.

In coding, simpler is usually better. There's an old programming adage, one that I usually tell myself on every project - "KISS, Keep It Simple, Stupid!" It means that if the code seems like it is getting too complicated, it's probably wrong and needs to be simplified. Complexity invites bugs.

Having said all that, the code could be simplified just a little. Pennies can be handled with one statement. Think about this. What are the possible results of x = x % 1?

If you were ambitious, you could actually combine it with the nickels calculation, but that would be complicating things unnecessarily. KISS, right? ;-)

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)

  • 1
    Thanks for the answer! I guess I was just overthinking a bit then :) Also following your suggestion, could I just replace this part of the code n += change / 1; change %= 1; with just n += change; ?
    – xbufu
    Apr 21 '19 at 22:36

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .