5

I was creating a very simple program that determines how many coins you need to return the change to a client, using a greedy algorithm. The algorithm is really obvious, you just need to determine which is the bigger coin you can use, subtract its value from the change and update the coins counter.

I have thought of two really similar implementations.

note: changeInt is the change, multiplied by 100 and converted into a integer.

1) A single "complex" loop

while(changeInt != 0) {
       if(changeInt - 25 >= 0){
           changeInt -= 25;
           coins++;
       }
       else if(changeInt - 10 >= 0){
           changeInt -= 10;
           coins++;
       }
       else if(changeInt - 5 >= 0){
           changeInt -= 5;
           coins++;
       }
       else if(changeInt - 1 >= 0){
           changeInt -= 1;
           coins++;
       }

   }

2) Multiple simple loops

    while(changeInt - 25 >= 0) 
   {
       changeInt -= 25;
       coins++;
   }
    while(changeInt - 10 >= 0)
   {
       changeInt -= 10;
       coins++;
   }

    while(changeInt - 5 >= 0)
   {
       changeInt -= 5;
       coins++;
   }

    while(changeInt - 1 >= 0)
   {
       changeInt -= 1;
       coins++;
   }

Now, I know the performance will probably be similar in both cases, since the algorithm is the same, but I was wondering which approach is better.

The single loop is the first idea I came up with, then I thought of the second method and it intuitively seems better to me.

I don't really care about my exact scenario, I'm more interested in the general one (several simple loops vs few more complex loops)

1) Which approach is better in terms of performance?

2) Is the difference noticeable, at least when working with huge numbers?

3) Is one approach significantly more readable than the other? (not sure if I can ask that here)

Thank you!

Carlos
  • 5,991
  • 6
  • 43
  • 82
mauroSabella
  • 71
  • 1
  • 5
  • 1
    @mauroSabella: Why `chageInt - X >= 0` instead of `changeInt >= X`? They're equivalent, so it's really just a stylistic question? Why did you choose to do it the way you did? – rjp Aug 11 '16 at 19:42
  • 1
    @trincot What do you mean? I get the same result. Why are you saying the algorithm is not the same? Both do the exact same thing in the exact same order, from what I can see. – mauroSabella Aug 11 '16 at 19:47
  • You are right. I completely misjudged the first code block. – trincot Aug 11 '16 at 19:48
  • 2
    Is your homework running too slow, or why do you care about preliminary optimisations? Concentrate on writing *readable** code until you really have an issue. Then profile your code and optimise hotspots only. – too honest for this site Aug 11 '16 at 19:48
  • @rjp Yes, changeInt - X >= 0 and changeInt >= X are equivalent. But that is not what I was asking about :). The difference between the two scenarios is that in one I use a single loop with several IFs inside, in the other scenario I use several loops, but less comparisons overall. – mauroSabella Aug 11 '16 at 19:52
  • 1
    Personally, I find the second code more readable, but YMMV. – John Bollinger Aug 11 '16 at 19:53
  • @Olaf This is not a "homework"... but anyway, I know optimization isn't important at all in my code. My entire program isn't important at all. I just used it to show what I wanted to ask. The question, a curiosity of mine, is about the performance of multiple short loops vs few bigger loops. Thanks for answering, anyway :) – mauroSabella Aug 11 '16 at 19:55
  • 1
    @mauroSabella it's not just a style issue with the comparison. With small numbers like these it is not an issue, but with large numbers, the subtraction might put the value out of range of the type. Just do a direct comparison, it's easier to read, less work for the processor and safer. Win-win. – Weather Vane Aug 11 '16 at 19:57
  • @WeatherVane Yes, you're right. Thank you. – mauroSabella Aug 11 '16 at 20:07
  • Assuming that I didn't just adopt division, I would use an array with the different values as the entries: `int coins[] = { 25, 10, 5, 1 };` and use that in pair of nested loops. The code repetition is not particularly healthy. – Jonathan Leffler Aug 11 '16 at 23:26

4 Answers4

2

As others have mentioned, the second approach is preferable since it uses less comparisons.

A cleaner, more concise way would be to use division and modulus:

int current = changeInt;
coins += current / 25;
current %= 25;
coins += current / 10;
current %= 10;
coins += current / 5;
current %= 5;
coins += current;

While the div and mod operators are more expensive than subtracting, it's likely to be faster for larger value of changeInt and there are no branches.

dbush
  • 205,898
  • 23
  • 218
  • 273
1

If you had to choose between the looped approaches you described, the second would be preferable (with a slight variation). It is cleaner, and mostly avoids unnecessary testing.

Here's the slight variation ...

while(changeInt >= 25) {
   changeInt -= 25;
   coins++;
}

while(changeInt >= 10) {
   changeInt -= 10;
   coins++;
}

while(changeInt >= 5) {
   changeInt -= 5;
   coins++;
}

while(changeInt > 0) {
   changeInt -= 1;
   coins++;
}

The primary advantage that this offers is that it helps ensure that 'changeInt - X' never wraps around. From what you described in your post, it is unlikely that it would be an issue, but if the type were change from a signed integer to an unsigned integer, then you might have found yourself trying to figure out where the bug lay.

Alternatively, you may wish to use a combination of the division and modulus operators to calculate the change and avoid the loops.

Hope this helps.

Sparky
  • 13,505
  • 4
  • 26
  • 27
  • Thank you for answering. You're absolutely right about your variation, as you are about the possibility of using the modulus operator. However, I'm more interested in the general idea of using a few complex loop rather than many simple loops. I think the second approach is better because it requires less comparisons overall (in my case), but I don't really know how this affects performance. Is there a noticeable difference (even tiny) when working with huge numbers, or is it really just a question of readability? (of course, using modulus would be better, but i'm interested in the comparisons) – mauroSabella Aug 11 '16 at 20:06
  • 1
    @mauroSabella: Until you've clearly measured that one is more performant than another for your code, then all that matters is big-O notation, and readability. – Mooing Duck Aug 11 '16 at 20:22
0

As you said, both are pretty similar, if we are talking about the readability I prefer the first one (but it is a subjective opinion).

If we think in performance, IMHO the second is a little bit faster than the previous one. We can try to traslate to ASM for for detail comparison:

1) A single "complex" loop (aprox ASM x86-64)

jmp
mov
sub
test
js
sub
add
jmp
mov
sub
test
js 
sub
add
jmp 
mov 
sub 
test
js  
sub 
add 
jmp 
mov 
sub 
test
js 
sub
add
cmp
jne

2) Multiple simple loops (aprox ASM x86-64)

jmp
sub
add
mov
sub
test
jns
jmp
sub
add
mov
sub
test
jns
jmp
sub
add
mov
sub
test
jns
jmp
sub
add
mov
sub
test
jns

If we count the x86-64 ASM instructions:

jmp: 4
mov: 4
sub: 8
test: 4
add: 4
js: 4
cmp: 1
jne: 1

vs 

jmp: 4
mov: 4
sub: 8
test: 4
add: 4
jns: 4

Then to sum up:

js 4
cmp 1
jne 1

vs

jns 4

And "js" is similar to "jns".

But this can change with other compilers or architectures, even so, I think the second one is a little bit faster than the first one.

David Pérez Cabrera
  • 4,960
  • 2
  • 23
  • 37
  • 3
    Though I respect your counting, I think it is wrong. When pipelined, most of the instructions are affectively executed in zero clocks. Only (conditional) jumps/branches are expensive, because the processor will take **both** paths (and roll back the wrong one). Too many conditions *inside a loop* will always screw up the pipeline somewhere. (apart from register allocation/ cache locality, and all that) Normally, a *thin loop* will execute in _about_ zero clocks for the loop body and a few more for the loop machinery. – wildplasser Aug 11 '16 at 20:35
  • Thank you very much, this is exactly what I was asking about. – mauroSabella Aug 11 '16 at 20:39
  • @wildplasser I think you are right, I simplified the answer, but If you think in your (very important) considerations, your answer should be the same, because the first one have a big loop with many conditions. – David Pérez Cabrera Aug 11 '16 at 20:45
0

I see no reason why you need a loop for this. Adding a temp variable in case you need to preserve your original value:

int tempChangeInt;

tempChangeInt = changeInt;
coins = changeInt / 25;
tempChangeInt = changeInt % 25;
if (tempChangeInt != 0)
{
    coins += tempChangeInt / 10;
    tempChangeInt = tempChangeInt % 10;
}
if (tempChangeInt != 0)
{
    if (tempChangeInt >= 5)
        coins += (tempChangeInt - 5) + 1;
    else
        coins += tempChangeInt;
}
David Ranieri
  • 39,972
  • 7
  • 52
  • 94
Anjani Kumar Agrawal
  • 365
  • 1
  • 14