A few months ago I got some feedback on a Pull Request review about an optimization I had made to a piece of code. Before we get into the Fallacy of Premature Optimization discussion, which is a good one to have, it goes beyond the scope of this article.
To give some context, this particular optimization was essentially to parallelize a number of calls to an external service (there was a little more logic to it, but this was the main point), thus potentially improve performance and responsiveness. Yeah, you did read “potentially”; parallelizing calls does not automatically guarantee a performance improvement but, unless there are so many calls that 1) it could cause thread exhaustion, 2) the callee gets flooded and can’t cope, it generally does. But that’s besides the point…
One particular review feedback I got from this change was, summing it up, along the lines of:
“You’re sacrificing readability for performance, which is not good.”
After some healthy discussion, this statement, coming from someone very experienced and competent, was based on two premises:
- A foreach loop is said to be “more readable” and simpler to understand than a single line;
- It may become a problem with trainees or junior developers who might not be familiar with certain patterns or techniques used by seasoned developers.
As far as the first premise goes, “readability” can be a very subjective concept, in the sense that what’s very readable to one developer might not be to another at all. Experience also plays a big part in judging how readable some piece of code is. Coming from the old days when loops were a very common practice, I can concede to that, but we’ve come a long way since then with stuff like LinQ, etc.
The second one was said to be a major concern because we all want new joiners to basically “hit the ground running”. That causes some itching and raises some concerns of my own…
Are we compromising code quality and performance so we can save some time during onboarding? Do developers want to learn and improve their skills or do we just want to keep things stale and make it easy for them? Are we setting the bar high or low?
In my view, which many other people I spoke with share, having smart designs and solutions that challenge and inspire junior developers to learn is one of the best things when onboarding new joiners. You do need to foster a culture of learning and make them feel comfortable to ask about something they don’t understand and even to question certain design choices that are already in place. Do not underestimate the refreshing views of less experienced developers. And let’s not forget about seasoned developers that come onboard and, them too, get to see very interesting new perspectives and approaches they hadn’t been able to explore in previous work environments.
Having said this, I’m excluding quick gains, dirty hacks and the like from these considerations. However, that can teach us a lot too: knowing what not to do is just as important as knowing what to do.
Adding features to code, especially ones that increase performance, gives us ALL great opportunities to gain knowledge.
There’s a huge caveat to this, though: the view I’ve been expressing assumes there are potentially reasonable performance gains. Even with all said, we should not compromise readability and maintainability for negligible or virtually nonexistent performance gains, unless it’s a performance critical component where we need to squeeze every millisecond.
Make no mistake: readability is something of utmost importance and something every codebase should aim for, but not at the cost of performance or skills development.