Question regarding Readability vs Processing Time

Question

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)?

Answer 1

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.

Answer 2

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.

Answer 3

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();

Answer 4

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.

Answer 5

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.