Clean Code Takes Priority: Choosing Readability Over Speed

Feb 12, 2023

I got a lot of comments on my recent refactoring article and the corresponding video. I replied to some of them here and there, and soon I realised it would be helpful to put these into one place and discuss the topic a bit deeper.

There is a particular comment about the refactored version having too much abstraction and can be less performant than the original approach. Seemingly, less abstraction and fewer layers could lead to a faster program. But is running fast our only goal for regular applications?

Update 27 Feb. I have published a corresponding video just in case you prefer another format:

Let the machine do the hard work.

I prioritise readability over a fast-loading page, though I acknowledge the importance of a speedy experience. I won’t sacrifice clean and understandable code for performance optimisation unless it becomes a noticeable issue. I believe maintainable code is of higher importance than code that performs well, especially for a large project that requires more people to work on.

“Programmer time is expensive; conserve it in preference to machine time.”
— Eric S. Raymond

I prefer to have the code clean and intuitive, even if it’s not perfect in terms of performance. I will give you a quick example here. If we pursue high performance, we can iterate through a collection in one single round to finish all the necessary operations. On the other hand, chaining these different operations together with multiple collection APIs ( e.g. data.filter().map().reduce()), and with the help of smaller but dedicated functions, might make the code more readable. What would you choose then?

Let’s zoom in a bit

For example, I have a list of users data, and each user is defined in a type like this:

[
  {
    "id": 1,
    "name": "Desdemona",
    "role": "Engineer",
    "experience": 1,
    "location": null
  },
  {
    "id": 2,
    "name": "Paloma",
    "role": "Construction Foreman",
    "experience": 18,
    "location": "Hali"
  },
  //...
]

Now assume I need to finish two tasks:

Find all “Engineer” role who has more than three years of experience and
If the location is null, use a fallback value as “N/A” that will be used on a UI component

I have two options: in a single for-loop, I put all the logic in and get the result. Or I can split the process into two individual steps and then connect them with collection APIs.

A “fast” transform function

We can finish the task in one round pretty straightforward like the snippet below:

const transformUsersPerformant = (users: User[]) => {
  const results = [];

  for (let i = 0; i < users.length; i++) {
    const user = users[i];
    if (user.experience > 3 && user.role === "Engineer") {
      results.push({
        ...user,
        location: user.location === null ? "N/A" : user.location,
      });
    }
  }

  return results;
};

It works fine. And when the users is a large dataset, which might be the best option. On the other hand, I would refactor the code into something more abstract. Please note that my assumption here is the users list isn’t a large dataset (less than 1k items, for example).

A slower-but-cleaner version

In contrast, we can extract a few lower-level details into some named functions and link them by collection APIs like filter and map.

const isNoLocation = (user: User) => {
  return user.location === null;
};

const fillBlankLocationForUser = (user: User) => {
  if(isNoLocation(user)) {
    return {...user, location: 'N/A'}
  }
  return user;
}

const isSeniorEngineer = (user: User) =>
  user.experience > 3 && user.role === "Engineer";

const transformUsersAbstract = (users: User[]) =>
  users.filter(isSeniorEngineer).map(fillBlankLocationForUser);

From the performance perspective, this code is “bad”. It iterates the users twice (if you swap the map and filter call to get the same result but even slower). It has many “unnecessary” function calls (so more stack push/pops and ultimately slower).

However, I think the new approach is better, the reasons being:

isSeniorEngineer is much cleaner than the raw user.role === '' && user.experience > 3
The criteria of isSeniorEngineer might be changed, and we don't have to guess what defines a senior engineer but read it literally
fillBlankLocation and isNoLocation can be reused in other places

So how fast of the one-round iteration?

I did a simple benchmark for these two approaches, with 1000 user objects (117kb). The performance is, well, roughly the same (I used performance.now(), so the result is in milliseconds, by the way).

In addition, in most cases I’ve worked on, manipulating 1000 items in an array is rare, especially in the front-end world. And you can always use pagination or other UI designs to avoid this kind of calculation.

What about maintainability

I think you would agree that as more tasks add to the collection calculation, the latter option will be much more readable and scalable.

The abstracted version will be linear in response to new changes. As time goes by, we might need to add new nodes into the pipeline, or maybe the pipeline remains, but you only need to update these small functions, e.g. fillBlankLocationForUser.

users.filter(isSeniorEngineer).map(fillBlankLocationForUser).reduce(summarise)

In contrast, changing the logic inside the one-round loop can be challenging. When you read the code, things at different abstraction levels are mixed. You’re not only dealing with business logic but also need to consider the index of the item ([i]) and the list API (.push), and all these mixes will make it hard to understand.

for (let i = 0; i < users.length; i++) {
  const user = users[i];
  if (user.experience > 3 && user.role === "Engineer") {
    results.push({
      ...user,
      location: user.location === null ? "N/A" : user.location,
    });
  }
  //...
}

The high-level programming language is called high-level for a good reason. It blocks all the noise you otherwise need for the lower-level APIs, registers, system calls, interrupts and signals, etc.

And I haven’t seen a performance problem solely because the code has too many levels of abstraction. I see code that is hard to read and test, and that takes extra days and weeks to modify: adding even small features is difficult, and fixing a defect can be even harder without breaking other “unrelated” stuff.

If you’re interested in maintainability, I’ve created a cheat sheet about more patterns and tips on the subject, and you can print it out if you like.

But performant is also important, right?

Let’s take a little bit about performance. A good design tends to make your code more performant. With a good abstraction and an extra layer to block the lower-level details, it’s much easier (again, since the code is more readable) to find the bottleneck and fix the performance issue correspondingly.

And I’ve discussed the relationship between good design and performance in another article Did someone say composition? — performance issues, in many cases, are caused by bad design.

Premature optimization is the root of all evil.
— Donald Knuth

It’s hard to foresee which part could cause performance problems, and guessing is not a good habit to have in coding. As you have seen above, these additional abstractions aren’t causing big issues. So my approach is don’t optimise your code upfront. Instead, you should establish a suitable measurement mechanism and monitor the numbers during the continuous delivery progress.

Conclusion

In this blog post, I weigh the importance of clean code versus performant code. Clean code is preferable as it provides a clearer and more maintainable codebase. I argue that a singular focus on performance can result in intricate designs that are challenging to keep up in the long term. Ultimately, I contend that putting clean code first results in not only better performance but also easier maintenance in the future.

References

I hope you found this blog post informative and helpful. If you’re interested in diving deeper into the subject, I’ve created an online course that covers all the topics discussed in this post and more. This course is designed to be interactive, engaging and packed with practical tips and techniques.

Clean Code Takes Priority: Choosing Readability Over Speed was originally published in ITNEXT on Medium, where people are continuing the conversation by highlighting and responding to this story.