Wednesday, November 9, 2011

Clever Code

Every once in a while I run across a bit of code that I find rather clever and I examine it to see if I like it or not. Really it's to see if I ever want to keep it around for my bag of tricks. Sometimes the code is a nice design pattern or sometimes it's rather small, like today's clever bit of code. Today's clever bit of code is this rather simple loop written in Java, but it'd work just fine in C or C++:

  int i = list.size();

  while (--i >= 0)
  {
    list.remove(i);
    //..do something
  }

At first glance I kinda liked it. Simple, elegant does the job in a few less lines than I would have written. Here's is probably what I would have written:
  int i = list.size();

  while (i >= 0)
  {
    i--;
    list.remove(i);
    //..do something
  }

Now after thinking about this bit of code I've come to the conclusion after trying it under a few different C compilers and the Java compiler, I don't like it. i is evaluated then no matter what in all the cases that I tested i is decremented. Not a huge deal, in this small example i will always be in a register, but if the body of the loop contains a bit more in it like the one I was looking at then there's a couple extra mov instructions generated as well.

Am I being picky? Yes, yes I am. I'm not always this picky. I prefer readable code to optimized code, but in this case I don't find it more readable and it is less optimal.

7 comments:

Unknown said...

Your more complicated block might merit a while loop like that but for the small example you provided I think a for loop would be more clear:

for( int i = list.size(); i >= 0; i--) {
list.remove(i);
//..do something
}

Unknown said...

Derp. that should be "int i = list.size() -1;"

Chris Bensen said...

Actually the for loop is optimized to not call the i-- so it isn't the same. This code exhibits the problem more clearly:

int i = 0;

while (--i >= 0) {}

This is the 0th case so it isn't totally contrived.

Jouni Aro said...

Chris, your version should of course be

int i = list.size();

while (i > 0)
{
i--;
list.remove(i);
//..do something
}

But you could also write it

int i = list.size();

while (i > 0)
{
list.remove(--i);
//..do something
}

But that's even less clear...

Also, I cannot figure out a single case where it would matter at all how optimized this piece of code is. In my book, reliability and correctness always come first. Optimization last. In that case, you profile and find out the real bottle necks. If you profile real code, this kind of issues will come up.

So, I would also prefer the for-loop.

Chris Bensen said...

Jouni, readability is very important of course, but now that everything runs on batteries optimization does really matter. My point is, this clever code that I ran across is a tiny bit slower and to clever for it's own good. So be careful when writing clever code because you could inadvertently be writing code that isn't as optimized.

In this case I agree, the for loop is preferred.

Warren said...

If you could find a way to measure the difference (in battery power even) between any of these, in a real application scenario (not a pure benchmark scenario), I'd be shocked, and I'd buy you some nice Canadian beer.

My guess is that we're twiddling nanoseconds and femtoamphours here.

Warren

Chris Bensen said...

Warren, it certainly is splitting hairs. But if this for some odd reason gets called a lot it could add up. But my point is simple, be mindful of the code you write because it can have effects you didn't foresee. Usually clever code is hard to read and not as performant and often times compilers have a difficult time optimizing it too.

Post a Comment