Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I am creating a flowchart for a program with multiple sequential steps. Every step should be performed if the previous step is succesful. I use a c-based programming language so the lay-out would be something like this:

METHOD 1:

if(step_one_succeeded())
{
    if(step_two_succeeded())
    {
        if(step_three_succeeded())
        {
            //etc. etc.
        }   
    }
}

If my program would have 15+ steps, the resulting code would be terribly unfriendly to read. So I changed my design and implemented a global errorcode that I keep passing by reference, make everything more readable. The resulting code would be something like this:

METHOD 2:

int _no_error = 0;

step_one(_no_error);
if(_no_error == 0) step_two(_no_error);
if(_no_error == 0) step_three(_no_error);
if(_no_error == 0) step_two(_no_error);

The cyclomatic complexibility stays the same. Now let's say there are N number of steps. And let's assume that checking a condition is 1 clock long and performing a step doesn't take up time.

The processing speed of Method1 can be anywhere between 1 and N.

The processing speed of Method2 however is always equal to N-1.

So Method1 will be faster most of the time. Which brings me to my question, is it bad practice to sacrifice time in order to make the code more readable? And why (not)?

share|improve this question
2  
Compile it and benchmark it. I bet you there won't be any measurable difference. That is because your model of performance has been out of touch with reality since a decade or two, possibly longer. Learn how compilers and CPUs work. –  delnan Jun 24 '13 at 11:43
    
I agree that for every pc since 1970 this shouldn't be an issue. But I also plan to use this architecture in a 8-bit microntroller. –  Jordy Jun 24 '13 at 11:55
3  
"performing a step doesn't take up time" - honestly, that is IMHO a very unrealistic assumption. I guess in most real-world cases performing a step will take significantly more time that those error-checking. And that should answer your question. –  Doc Brown Jun 24 '13 at 11:57
1  
@Jordy Even on a microcontroller, the compiler may output identical code for the two (or for an equally flat and readable rewrite of the second). I can't believe I'm saying this, having suffered at the hands of Keil µC, but even microcontroller compilers have a few tricks up their sleeves. –  delnan Jun 24 '13 at 12:01
    
"The processing speed of Method2 however is always equal to N-1." I'm inclined to say this quote is flat out wrong. –  rwong Oct 23 '13 at 4:29

5 Answers 5

up vote 12 down vote accepted

Sacrificing code readability to save a few clock cycles is a clear sign of premature optimisation.

Your first aim should always be to write the best readable/maintainable code that gives a reasonable performance. Only after measurements have indicated that your N condition tests actually form a bottleneck for the overall performance and there are no optimisation possible that would yield a larger effect (such as using a different algorithm), then you could consider sacrificing readability to get the required performance.

Even if you are in the extremely rare situation that you are writing a library that must always give the very highest performance, you should consider carefully if the loss of readability is worth those few instructions and if a good optimising compiler would not generate the same code for both anyway.

share|improve this answer
    
Thanks, this post makes me more confident about my code. –  Jordy Jun 24 '13 at 11:58

And let's assume that checking a condition is 1 clock long and performing a step doesn't take up time.

No.

Let's not make assumptions that completely skew the discussion. In reality, the steps take up time, most likely much, much more time than all the condition checking.

Which means that the performance considerations are irrelevant.

And that is the reason why premature optimization is, in fact, bad even when performance is very important.

share|improve this answer
    
So if I understand correctly, I should make performance optimilizations after I have (failed to achieve)functioning code? –  Jordy Jun 24 '13 at 12:34
1  
@Jordy: you should make performance optimizations when you have measurements that tell you performance is worse than required and, ideally, which parts of the code are performing badly. And after you've done them, measure again and see whether you actually did improve performance. It's not at all uncommon to see "performance optimization" result in less maintainable and slower code. –  Michael Borgwardt Jun 24 '13 at 12:48
    
So basically: if it works, don't touch it. I think I got it, thanks! –  Jordy Jun 24 '13 at 13:05

Compilers usually slice and dice and completely rebuild any code they compile. Since readable code is more common, the optimizations are usually tuned for that, so readable code actually stands better chance being optimized well by the compiler.

On a side note, avoid returning by reference. It is both more difficult to read and more difficult for compiler to optimize. The variable should always be set like _no_error = _no_error && step_one() (which, accidentally, acts as condition, so you don't need them than; but it's not idiomatic in C++).

Last, I don't consider the _no_error (besides the bad name of that variable; symbols starting with _ are reserved for library extensions) version much more readable. A readable code in any language that supports RAII (C++, D, Rust) or equivalent (finally) (C#, Java) is:

if(!step_one())
    return;
if(!step_two())
    return;
...

In plain old C, using goto is preferable—if you need some clean-up there, that is, otherwise you can of course use return directly too:

if(!step_one())
    goto failed;
if(!step_two())
    goto failed;
...
failed:
clean_up();
share|improve this answer
    
Thank you, I found your post very helpful. And I think I'd better start searching for some documentation about naming variables. –  Jordy Jun 24 '13 at 12:36
    
I agree with @Jan Hudec, I think it's debatable as to which is more readable. But then again, I think that's kind of auxiliary to the point of this question in the first place. –  Brian Snow Jun 24 '13 at 15:43

If you say precisely and concisely what is desired, the compiler can produce the most efficient code. Almost all the simple optimizations we used to do by hand are now done automatically by compilers.

You have no need for a state variable. Have each function return true on success and false on failure, then write:

if (step_one()
    && step_two()
    && step_three()
    && ...) {
  /* success */
  ...
} else {
  /* failure */
  ...
}
...

In most imperative programming languages, (C, Java, C#, Python, Perl, Ruby, LISP, et. al.) the and operator (&& or and) evaluates clauses left to right and stops as soon as one is false.

share|improve this answer

If you have lots of steps and need to exit as soon as one fails, consider putting function pointers in an array and execute all of them, like this:

// The array
int (*steps[2]) ();

// Assign functions
steps[0] = step_one;
steps[1] = step_two;

// Call them
for (int i = 0; i < 2; i++) {
     if (!(*steps[i])()) 
         break;
}

This approach lets you configure during run-time in what order you which to run them as well.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.