Stop Sacrificing Readability For Efficiency!

Much was made last week over the topic of micro optimization in PHP. Most of these argued that micro optimization was a bad idea, from the perspective of unit testing, the idea that “premature optimization is the root of all evil”, the fact that it takes too much time, and that it violates the rules of development and optimizaton.

There’s another reason that micro optimization can be a bad choice: it makes code absolutely impossible to read!

The original article by Alex Netkachov makes arguments about calling static methods, using magic methods, getting the time with $_SERVER[‘REQUEST_TIME’], output buffering, and loops. But what this article completely misses is that many of these tips would render code completely useless if you had to maintain it.

Imagine the following code:

<?php

$string = 'This is my string.';

if(isset($string&#91;4&#93;)  && !isset($string&#91;5&#93;))
{
     moveOn();
}
else
{
   raiseError();
}

?>

What are we doing there? Every developer should be able to figure it out: we’re addressing the string’s individual characters as if they were array elements, and using isset() to determine whether the string is longer or shorter than five characters. This was a suggestion given by “10 Advanced PHP Tips to Improve Your Programming. It’s not a bad suggestion, but it creates a very real readability problem. I rarely do this and I would bet that most people rarely do this either (though it’s a neat trick).

The code’s readability (but not performance) would be enhanced if you did the following:

<?php

$string = 'This is my string.';

if(strlen($string) == 5)
{
     moveOn();
}
else
{
   raiseError();
}

?>

Every single PHP developer on earth should be able to read that code at first glance and understand. Is this the most efficient way to the end solution? In terms of the time it would take to complete this script, no. Calling functions is generally more expensive than calling language constructs like isset(). However, this savings would probably be realized only in millions of operations, and if your optimization strategy relies on saving 2 one-hundredths of a second, you’re going to spend a sum total of more time actually doing the “optimization” than the total time saved in terms of code execution.

Optimization is not bad, and writing clean code is also a good thing. You should never sacrifice one thing for the other.

Brandon Savage is the author of Mastering Object Oriented PHP and Practical Design Patterns in PHP

Artur Ejsmont wrote at 3/26/2009 9:44 am:

God yes you are right !

I would say its even worse than optimize prematurely as its not only wasting your time but any person who will ever read your code.

All the developers with perl background …. let it go! less characters does not make code better, does not make code faster and above all does not make code cleaner!

Your fancy optimizations may depend on platform or version of PHP and you might have code that is slower.

We should create ‘PHP – WTF?!’ website with all the examples listed above or recent articles. I have seen many many cherries like that. Please dont code like that even if you think its cool.

btw. the article in smashing magazine is a disaster! as now people will think, ‘oh wow not only i can code like that but some people actually recommend it’.

Spread the word!

Brandon Savage (@brandonsavage) wrote at 3/26/2009 9:50 am:

It’s worth noting that I failed to mention in my article that Chris Shiflett and Sean Coates called 10 Useful PHP Tips Revisited that basically disassembles the prior article and turns it to mush. I didn’t see the need to cite it, since it wasn’t on topic, but since you mentioned it…

Sean Coates (@coates) wrote at 3/26/2009 10:36 am:

Hey Brandon, thanks for linking us. I also want to make it clear that the reason we mention this tip in our rebuttal article is because it was in the original.

We wouldn’t use this method in regular code, but if we were optimizing a tight loop (especially one that the profiler pointed out as a problem area), we might consider it. The reality is that isset() is faster than strlen(), but most code doesn’t need to do this because, as you mentioned, readability suffers, and this is almost always more important.

S

Brandon Savage (@brandonsavage) wrote at 3/26/2009 11:06 am:

Sean, thanks for the note. Yes, I totally understand that you’re not endorsing the method per se, and that the example you provided was because the original article had a reference to it. I know you and Chris are better developers than that. ;-)

I’ve written on using isset() instead as well (http://brandonsavage.net/searching-arrays-for-values/) though the array functions can be REALLY slow on large arrays, which is why I suggest it in the case above. isset() is probably 5x faster on a 20,000 item array.

Sebs (@phptestfest) wrote at 3/26/2009 11:32 am:

hmmm …. not yet readable enough … if you talk about this topic, maybe first of all speak about the useless usage of ELSE!

$string = ‘This is my string.’;

if(strlen($string) != 5)
{
raiseError();
// if raise error != exception
return
}
moveOn();

I dont mind big conditionals, but i really mind waste of space. If code gets more complex you can build nice pyramids of if and else with that ;)

Brandon Savage (@brandonsavage) wrote at 3/26/2009 11:37 am:

Sebs, I think you misunderstood the example.

I’m not encouraging needless use of else conditions. And you can bet I’d probably wrap that code in a function. But for the sake of the example, I used an else condition simply for the sample.

You raise a very valid point, however; else conditions can be removed for the most part, unless you’re doing unique processing in the else condition.

Giorgio Sironi (@giorgiosironi) wrote at 3/26/2009 1:27 pm:

Totally agree… “Let’s rewrite this procedure in assembly…”…

Jay Muntz (@docmonk) wrote at 3/27/2009 3:22 pm:

The first example is actually testing for a six-character string. Should be:

if(isset($string[4]) && !isset($string[5]))

And of course – that just proves your point even more. The more-efficient way is also more bug-prone!

Brandon Savage (@brandonsavage) wrote at 3/27/2009 3:27 pm:

Jay, you’re absolutely right. And, in fact, 10 other developers who I talked to about that blog post failed to notice that, too.

Tibo wrote at 3/30/2009 11:04 am:

Interesting post. I think that in quite a lot of cases there’s just no real need to go into these kind of details to gain a little bit of performance boost. And the gain is usually negated by maintenance cost. (Upgrading your server might be cheaper than having a development team waste time).

Besides choice of functions: Imo the availability of good (and free) IDE’s make it easy to use good descriptive (and possibly long) variable names, so on that part there’s no reason for keeping variable names confusingly short.

Jim R. Wilson (@jimbojw) wrote at 3/30/2009 5:16 pm:

Agreed – it’s definitely a tradeoff.

I tend to do micro-optimizations in the face of something known to loop or be expensive, and leave it more readable otherwise.

For example, in some JavaScript implementations, doing someArray.push(x) is slower than someArray[someArray.length] = x. Neither is especially more readable than the other, so I tend to use the faster version (especially since appending to an array so often happens within a loop).

Also, it might be worth mentioning that code execution itself is rarely the bottleneck – so often it’s file access, or database queries, or some other even more expensive task (such as performing an uncached HTTP request to an external 3rd party web service). Given this, code micro-optimizations like the string length example in the article are generally only going to help the part of the code which is already the fastest.

Ben wrote at 3/30/2009 6:01 pm:

shouldn’t that be strlen($string) < (rather than ==) ?

Andries Inzé wrote at 3/31/2009 8:38 am:

A good solution is to write an efficient isShorterThen($string, 5).
The isShortThen method can then do the optimized version.

Brandon Savage (@brandonsavage) wrote at 3/31/2009 9:08 am:

Since we’re testing for exactly 5 characters, no. The first code sample tests for five characters too.

Brandon Savage (@brandonsavage) wrote at 3/31/2009 9:10 am:

Andries, I see your point. However, at some point you’re still going to be writing the source code that I found confusing. That said, I like custom true/false functions, and your point has merit.

Joshua Ross wrote at 3/31/2009 9:10 pm:

I am in complete agreement and blogged about this very thing not too long ago… shortly after becoming frustrated at my team for headscratching optimizations and abbreviations of variables/methods/classes. Now if I could only get my team to read our blogs. :)

Dave Walker wrote at 4/7/2009 1:42 pm:

For a personal web project, or for a small internal app, I could not agree with you more. For a site that gets 1+mil hits every hour, i could not disagree with you more. The framework we use at work is huge (powerful) but highly efficient, because we mostly stick to the language constructs. If, however, for every page-load we needed to add to the stack 3k+ function calls to make code ‘readable’ that would probably have a noticeable and detrimental effect of the app as a whole.

While I mostly agree, yes you should write clean, readable code, you can also do what I’ve done, and make a ‘optimization’ parser that takes clean readable code and converts as much as I tell it to do from functions to language constructs.

Copyright © 2023 by Brandon Savage. All rights reserved.