tags:

views:

383

answers:

8

A peer of mine is working on a report that displays the weekly (Sunday to Saturday) advance of every employee in our small consultancy firm. There's a piece of code he wrote that shows the columns corresponding to the days in the target week. His algorithm is the following:

  1. Get which day of the week the first day of the month is. If it's Sunday, set a flag to zero; otherwise, set it to one.
  2. Iterate through all the days of month. If it's Sunday, increment the flag. Then, if the flag's value is equal to the week to be displayed, show the column corresponding to the current day; otherwise, hide the column.

Of course, the flag indicates what the current week is.

I suggested another algorithm:

  1. Get which days of the month are the first (F) and last (L) days of the specified week. For example, the first week of October 2009 starts on Tuesday 1st and ends on Saturday 3rd.
  2. Iterate through the columns corresponding to days 1 to F-1, and hide them.
  3. Iterate through the columns corresponding to days F to L, and show them.
  4. Iterate through the columns corresponding to days L+1 to DaysOfMonth, and hide them.

The "difficult" part in my algorithm is part 1. I mean "difficult" as in "difficult to understand", because the algorithmic complexity of doing it is constant. And my algorithm has the advantage of having tighter loop. My peer's loop does a comparison for every day of the month. Mine doesn't.

This was a little example and you might say that over-optimizing here is a bit too paranoid. But his programming style doesn't change a bit when we write actual performance-critical code.

His code is also full of these tests:

/* doSomething() doesn't change the state of the relevant variables. */
if (condition)
{
    flag++;
    if (flag > test)
        doSomething();
}
else
    if (flag >= test)
        doSomething();

When, of course, it can be done like this:

if (flag >= test);
    doSomething();
if (condition)
    flag++;

What do I do?!?!?!

EDIT: I corrected the comparisons in the code samples.

+10  A: 

I think your friend has the right idea. Take the algorithm that's obviously correct over the algorithm that would take an hour to explain but, with no particular performance goal in mind, is faster.

If you have a specific performance requirement, like 'the code needs to be able to give correct results for all months in the next ten years within 200 microseconds on machine X', and the simpler code fails the requirement, then you might consider using your version.

(The code sample you posted is indeed obviously better in your way, because it's less convoluted.)

Joren
I actually found the OPs code easier to understand.
Joe Philllips
What I do is the following: First I come up with an inefficient version of the program, then I move as many comparisons or inner loops as I can out of the main loops.
Eduardo León
I voted this up (as well as several others anti-premature-optimization answers) but I think we may also need to recognize that sometimes there's a manifestly superior alternative that doesn't take an hour to explain, and the original poster's peer *may just not have thought of it*. What about these cases? Can anyone give the original poster the benefit of the doubt and provide advice about that case? Probably not, because it's a social problem, but I'm interested in hearing that too.
Jeff Sternal
+5  A: 

From your description, I'm not sure I disagree with your colleague. The key issue here is whether or not this piece of code is a performance bottleneck.

To convince me to switch to your algorithm you would have to profile the application in question and show me that this piece of code is performance-critical. Then make your change and profile it again. That way you have an objective basis for comparison.

If there is a meaningful difference between the two algorithms then the two of you can discuss whether or not it's worthwhile to make the switch.

If you are worried about page-load times of a web application, remember the lessons of High Performance Web Sites and the Yahoo performance guidelines - how you handle CSS, javascript and caching will have a far greater impact than optimising the algorithms that run on your server.

Advocating optimisation without measurement is just as dangerous as ignoring the performance implications of naive algorithms.

ctford
Oh, well... In this case, I better use whatever algorithm comes to my mind first. I write PeopleCode, and there's a component in PeopleSoft's architecture that handles JavaScript, CSS, etc.
Eduardo León
+3  A: 

Is this your task, or his? If its his, let him do it. I am a person who thrives on efficiency, in fact I become very frustrated when something seems inefficient and is out of my control.

There are well over 200 people on SO who could put my most 'efficient' ideas, algorithms and code to shame. Probably more, you can't go by rep alone. If Linus Torvalds himself signed up, he'd start at 1 like the rest of us.

What you have to consider is that people need to be able to maintain the code that they write. This means, they have to understand it as if they gave birth to it. Even if someone demonstrated another algorithm as much more efficient than my own, I would not use it unless I was comfortable with it.

If this is a joint project, write it your way, demonstrate the speed up and then spend some very, very patient hours with your peer to help him really grasp it.

Look back on stuff you wrote 5 years ago, everyone has to learn by doing and everyone does stuff at their own speed, especially learning.

Tim Post
"...they have to understand it as if they gave birth to it." I know a lot of women who don't understand their own children...
Dave Sherohman
@Dave Sherohman: Behold the safety of metaphors :) We're not talking about women, we're talking about algorithms. If women were so easy to figure out, a patent would exist. "Method of ... " Lets stay on topic?
Tim Post
+3  A: 

Well you should not interfere because obviously you're wrong ... The two pieces of code are not equivalent. Just take condition=1, flag=0 and test=1

246tNt
Thank you! I got the comparisons wrong!
Eduardo León
Oh now this is classic :) Darnit, do as I say .. not as I do
Tim Post
+2  A: 

I don't see the problem here ... although his version has a little roundabout way of getting to the goal, he does get there.

This statement I'm about to say should not be taken out of context: but there will always be people who do it like that - and that is not wrong, quite the opposite ... he maybe doesn't know a good way to do something, but he does know how to get the result no matter how ugly his method is.
That can be a real lifesaver when nobody knows the correct algorithm for something ... chances are he'll duck tape it somehow.

ldigas
I'm myself not that into duck tape programming.
Eduardo León
@Eduardo Leon: How does one program duck tape? I've been trying to keep mine from sticking to itself when I tear off large pieces for years. I think what Idigas meant is _everyone learns_. When is the last time you had to maintain code you wrote 5 years ago? Its a process ...
Tim Post
I once wrote a discrete simulation engine when I was in college, and some of the code is actually very contrived. A month ago I found a bug in one of the most contrived parts, and I corrected it without any problems. Of course, that was because the source had enough comments to be understood.
Eduardo León
I think "duct tape" is intended here. Not to be confused with "duck typing".
Michael Easter
I know what duct tape programming is. I'm not really into it.
Eduardo León
+3  A: 

G'day,

I'd suggest that you should favour algorithmic simplicity that performs within your constraints over clever, overly complicated algorithms that provide no real gain. In my experience these sorts of overly clever tricks more than likely turn out to be a future maintenance nightmare!

This idea is better expressed by the following quote:

"The competent programmer is fully aware of the strictly limited size of his own skull; therefore he approaches the programming task in full humility, and among other things he avoids clever tricks like the plague." - Edsger Dijkstra in his 1972 ACM Turing Lecture "The Humble Programmer"

BTW That paper is a great read! Along with many of his other papers that are available online at the E.W.Dijkstra Archive

HTH

cheers,

Rob Wells
+2  A: 

I agree with the prevailing sentiment here: if a performance improvement would make code significantly more difficult to read, there had better be a good reason to make that change.

On the other hand, that doesn't mean that we should always settle for the first thing that comes to mind. Sometimes there are unambiguously superior alternatives that are equally readable or nearly so.

If you want to persuade your colleague to take his choice of algorithms seriously, you should pick your battles carefully. Don't try to turn his world upside down and force the issue on every single case you find. Start by making your suggestions only when you can think of a replacement algorithm that is demonstrably superior and also clear, and be ready to back off if he resists.

If you're feeling devious, you might even give him hints, ask leading questions, or whatever other technique you can think of to help him figure it out by himself.

Jeff Sternal
+1  A: 

I feel your pain. Programmers are really stubburn in nature. Everyone has his own principles and rules for constructing programs. If other opinions collide with some of then, changing programmer's opinion is like moving fundament pillars of a skyscraper building. :)

AareP