Biz & IT —

Is it OK to replace optimized code with readable code?

Weighing the opportunity cost of making improvements.

Is it OK to replace optimized code with readable code?
Stack Exchange
This Q&A is part of a biweekly series of posts highlighting common questions encountered by technophiles and answered by users at Stack Exchange, a free, community-powered network of 80+ Q&A sites.

Coder asks:

Sometimes you run into a situation where you have to extend/improve some existing code. You see that the old code is very lean, but it's also difficult to extend, and takes time to read.

Is it a good idea to replace it with modern code?

Some time ago I liked the lean approach, but now, it seems to me that it's better to sacrifice a lot of optimizations in favor of higher abstractions, better interfaces, and more readable, extendable code.

The compilers seem to be getting better as well, so things like struct abc = {} are silently turned into memsets, shared_ptrs are pretty much producing the same code as raw pointer twiddling, templates are working super good because they produce super lean code, and so on.

But still, sometimes you see stack based arrays and old C functions with some obscure logic, and usually they are not on the critical path.

Is it a good idea to change such code if you have to touch a small piece of it either way?

See Also: "Clean readable code vs. fast hard-to-read code: When to cross the line?"

Answer: Depends... (91 Votes)

MainMa replies:
Where?

  • On a home page of a Google-scale website, it is not acceptable. Keep the things as quick as possible.
  • In a part of an application which is used by one person once a year, it is perfectly acceptable to sacrifice performance in order to gain code readability.

In general, consider the non-functional requirements for the part of the code you're working on. If an action must perform under 900 ms. in a given context (machine, load, etc.) 80% of the time, and actually, it performs under 200 ms. 100% of the time, sure, make the code more readable even if it might slightly impact the performance. If, on the other hand, the same action never performed under ten seconds, well, you should rather try to see what's wrong with the performance (or the requirement in the first place).

Also, how will a readability improvement decrease the performance? Often, developers adapt the behavior close to premature optimization: they are afraid to increase the readability, believing that it will drastically destroy the performance, while the more readable code will spend a few microseconds more doing the same action.

Related: "Why should I care about micro performance and efficiency?"

Answer: Usually, no (28 Votes)

Demian Brecht replies:

Changing the code can cause unforeseen knock-on issues elsewhere in the system (which can sometimes go unnoticed until much later in a project if you don't have solid unit and smoke tests in place). I usually go by the "if it ain't broke, don't fix it" mentality.

The exception to this rule is if you're implementing a new feature that touches this code. If, at that point, it doesn't make sense and refactoring really needs to take place, then go for it as long as refactoring time (and sufficient testing and buffer for dealing with knock-on issues) is all accounted for in estimates.

Of course, profile, profile, profile, especially if it's a critical path area.

Answer: The extensive answer (19 Votes)

haylem replies:

In brief: It depends

  • Are you really going to need or use your refactored / enhanced version?
    • Is there a concrete gain, immediate or long-term?
    • Is this gain only for maintainability, or really architectural?
  • Does it really need to be optimized?
    • Why?
    • What target gain do you need to aim for?

In details

Are you going to need the cleaned up, shiny stuff? There are things to be cautious about here, and you need to identify the limit between what is real, measurable gain and what is just your personal preference and potential bad habit of touching code that shouldn't be.

More specifically, know that there is such a thing as Over-Engineering. It's an anti-pattern, and it comes with issues built-in:

  • It may be more extensible, but it may not be easier to extend
  • It may not be simpler to understand
  • Last, but definitely not least here: you might slow down the whole code

Some could also mention the KISS principle as a reference, but here it's counter-intuitive: is the optimized way the simple way or the cleany architectured way? The answer is not necessarily absolute, as explained in the rest below.

You ain't gonna need it: The YAGNI principle is not completely orthogonal with the other issue, but it helps to ask yourself the question: are you going to need it?Does the more complex architecture really present a benefit for you, apart from giving the appearance of being more maintainable?

If it ain't broke, don't fix it: Write this on a big poster and hang it next to your screen or in the kitchen area at work, or in the dev meeting room. Of course there are a lot of other mantras that are worth repeating yourself, but this particular one is important whenever you try to do "maintenance work" and feel the urge to "improve" it.

It's natural for us to want to "improve" code or even just touch it, even unconsciously, as we read through it to try to understand it. It's a good thing, as it means we're opinionated and try to get a deeper understanding of the internals, but it's also bound to our skill-level, our knowledge (how do you decide what's better or not? well, see sections below...), and all the assumptions we make about what we think we know the software:

  • Actually does,
  • Actually needs to do,
  • Will eventually need to do,
  • And how well it does it.

Does it really need to be optimized? All this said, why was it "optimized" in the first place? They say that premature optimization is the root of all evil, and if you see undocumented and seemingly optimized code, usually you could assume it probably didn't follow the Rules of Optimization, didn't dearly need the optimization effort, and that it was just the usual developer's hubris kicking in. Yet again, maybe it's just yours talking now.

If it does, within which limits does it become acceptable? If there's a need for it, this limit exists, and gives you room to improve things, or a hard-line to decide to let it go.

Also, beware of invisible characteristics. Chances are, your "extensible" version of this code will use up more memory at runtime as well, and present even a larger static memory footprint for the executable. Shiny OO features come with unintuitive costs like these, and they may matter to your program and the environment it's supposed to run on.

Measure, measure, measure

As the Google folks now, it's all about data! If you can back it up with data, then it's necessary.

There's this not so old tale that for every $1 spent in development it will be followed by at least $1 in testing and at least $1 in support (but really, it's a lot more).

Change impacts a lot of things:

  • You might need to produce a new build.
  • You should write new unit tests (definitely if there were none, and your more extensible architecture probably leaves room for more, as you have more surface for bugs).
  • You should write new performance tests (to make sure this stays stable in the future, and to see where the bottlenecks are), and these are tricky to do.
  • You'll need to document it (and more extensible means more room for details).
  • You (or someone else) will need to extensively re-test it in QA.
  • Code is (almost) never bug-free, and you'll need to support it.

So it's not just hardware resources consumption (execution speed or memory footprint) that you need to measure here, it's also team resources consumption. Both need to be predicted to define a target aim, to be measured, accounted for, and adapted based on development.

And for you manager, that means fitting it into the current development plan, so do communicate about it and do not get into furious cow-boy/submarine/black-ops coding.

In general...

Yes, but...Don't get me wrong, in general, I'd be in favor of doing why you suggest, and I often advocate it. But you need to be aware of the long-term cost.

In a perfect world, it's the right solution if:

  • Computer hardware gets better over time,
  • Compilers and runtime platforms get better over time,
  • You get close-to-perfect, clean, maintainable, and readable code.

In practice:

  • You may make it worse. You need more eyeballs to look at it, and the more you complexify it, the more eyeballs you need.
  • You can't predict the future. You can't know with absolute certainty if you'll ever need it and not even if the "extensions" you'll need would have been easier and quicker to implement in the old form, and if themselves would need to be super-optimized.
  • It represents, from management's perspective, a huge cost for no direct gain.

Make it part of the process: You mention here that it's a rather small change, and you have some specific issues in mind. I'd say it's usually OK in this case, but most of us also have personal stories of small changes, almost surgical-strike edits, which eventually turned into maintenance nightmare and nearly-missed or exploded deadlines because Joe Programmer didn't see one of the reasons behind the code and touched something that shouldn't have been.

If you have a process to handle such decisions, you take the personal edge off of them:

  • If you test things correctly, you'll know more quickly if things are broken.
  • If you measure them, you'll know if they improved.
  • If you review it, you'll know if it throws people off.

Test coverage, profiling, and data-collection are tricky: But, of course, your testing code and metrics might suffer from the same issues you're trying to avoid for your actual code: do you test the right things, and are they the right thing for the future, and do you measure the right things?

Still, in general, the more you test (until a certain limit) and measure, the more data you collect and the safer you are. Bad analogy time: think of it like driving (or life in general): you can be the best driver in the world, if the car breaks down on you or someone decided to kill themselves by driving into your car with their own today, your skills might not be enough. There are both environmental things that can hit you, and human errors also matter.

Code reviews are the development team's hallway testing: And I think the last part is key here: do code reviews. You won't know the value of your improvements if you make them solo. Code reviews are our "hallway testing:" follow Raymond's version of the Linus' Law both for detecting bugs and detecting over-engineering and other anti-patterns, and to ensure that the code is in line with your team's abilities. There's no point in having the "best" code if nobody else but you can understand and maintain it, and that goes both for cryptic optimizations and 6-layers deep architectural designs.

As closing words, remember:

Everyone knows that debugging is twice as hard as writing a program in the first place. So if you're as clever as you can be when you write it, how will you ever debug it?—Brian Kernighan

Think you know how best to prioritize readable vs. optimized code? Disagree with the opinions expressed above? Downvote or upvote an answer, or leave your own answer at the original post at Stack Exchange, a network of 80+ sites where you can trade expert knowledge on topics like web apps, cycling, scientific skepticism, and (almost) everything in between.

Channel Ars Technica