Washing your code: don't make me think

You’re reading an excerpt from my book on clean code, “Washing your code.” Available as PDF, EPUB, and as a paperback and Kindle edition. Get your copy now.

Clever code is something we may see in job interview questions or language quizzes, when they expect us to know how a language feature, which we probably have never seen before, works. My answer to all these questions is: “it won’t pass code review”.

Some folks confuse brevity with clarity. Short code (brevity) isn’t always the clearest code (clarity), often it’s the opposite. Striving to make your code shorter is a noble goal, but it should never come at the expense of readability.

There are many ways to express the same idea in code, and some are easier to understand than others. We should always aim to reduce the cognitive load of the next developer who reads our code. Every time we stumble on something that isn’t immediately obvious, we waste our brain’s resources.

Info: I “stole” the name of this chapter from Steve Krug’s book on web usability with the same name.

Dark patterns of JavaScript

Let’s look at some examples. Try to cover the answers and guess what these code snippets do. Then, count how many you guessed correctly.

Example 1:

const percent = 5;
const percentString = percent.toString().concat('%');

This code only adds the % sign to a number and should be rewritten as:

const percent = 5;
const percentString = `${percent}%`;
// → '5%'

Example 2:

const url = 'index.html?id=5';
if (~url.indexOf('id')) {
  // Something fishy here…
}

The ~ symbol is called the bitwise NOT operator. Its useful effect here is that it returns a falsy value only when indexOf() returns -1. This code should be rewritten as:

const url = 'index.html?id=5';
if (url.includes('id')) {
  // Something fishy here…
}

Example 3:

const value = ~~3.14;

Another obscure use of the bitwise NOT operator is to discard the fractional portion of a number. Use Math.floor() instead:

const value = Math.floor(3.14);
// → 3

Example 4:

if (dogs.length + cats.length > 0) {
  // Something fishy here…
}

This one is understandable after a moment: it checks if either of the two arrays has any elements. However, it’s better to make it clearer:

if (dogs.length > 0 && cats.length > 0) {
  // Something fishy here…
}

Example 5:

const header = 'filename="pizza.rar"';
const filename = header.split('filename=')[1].slice(1, -1);

This one took me a while to understand. Imagine we have a portion of a URL, such as filename="pizza". First, we split the string by = and take the second part, "pizza". Then, we slice the first and the last characters to get pizza.

I’d likely use a regular expression here:

const header = 'filename="pizza.rar"';
const filename = header.match(/filename="(.*?)"/)[1];
// → 'pizza'

Or, even better, the URLSearchParams API:

const header = 'filename="pizza.rar"';
const filename = new URLSearchParams(header)
  .get('filename')
  .replaceAll(/^"|"$/g, '');
// → 'pizza'

These quotes are weird, though. Normally we don’t need quotes around URL parameters, so talking to the backend developer could be a good idea.

Example 6:

const percent = 5;
const percentString = percent.toString().concat('%');

In the code above, we add a property to an object when the condition is true, otherwise we do nothing. The intention is more obvious when we explicitly define objects to destructure rather than relying on destructuring of falsy values:

const percent = 5;
const percentString = `${percent}%`;
// → '5%'

I usually prefer when objects don’t change shape, so I’d move the condition inside the value field:

const url = 'index.html?id=5';
if (~url.indexOf('id')) {
  // Something fishy here…
}

Example 7:

const url = 'index.html?id=5';
if (url.includes('id')) {
  // Something fishy here…
}

This wonderful one-liner creates an array filled with numbers from 0 to 9. Array(10) creates an array with 10 empty elements, then the keys() method returns the keys (numbers from 0 to 9) as an iterator, which we then convert into a plain array using the spread syntax. Exploding head emoji…

We can rewrite it using a for loop:

const value = ~~3.14;

As much as I like to avoid loops in my code, the loop version is more readable for me.

Somewhere in the middle would be using the Array.from() method:

const value = Math.floor(3.14);
// → 3

The Array.from({length: 10}) creates an array with 10 undefined elements, then using the map() method, we fill the array with numbers from 0 to 9.

We can write it shorter by using Array.from()’s map callback:

if (dogs.length + cats.length > 0) {
  // Something fishy here…
}

Explicit map() is slightly more readable, and we don’t need to remember what the second argument of Array.from() does. Additionally, Array.from({length: 10}) is slightly more readable than Array(10). Though only slightly.

So, what’s your score? I think mine would be around 3/7.

Gray areas

Some patterns tread the line between cleverness and readability.

For example, using Boolean to filter out falsy array elements (null and 0 in this example):

if (dogs.length > 0 && cats.length > 0) {
  // Something fishy here…
}

I find this pattern acceptable; though it requires learning, it’s better than the alternative:

const header = 'filename="pizza.rar"';
const filename = header.split('filename=')[1].slice(1, -1);

However, keep in mind that both variations filter out falsy values, so if zeros or empty strings are important, we need to explicitly filter for undefined or null:

const header = 'filename="pizza.rar"';
const filename = header.match(/filename="(.*?)"/)[1];
// → 'pizza'

Make differences in code obvious

When I see two lines of tricky code that appear identical, I assume they differ in some way, but I don’t see the difference yet. Otherwise, a programmer would likely create a variable or a function for the repeated code instead of copypasting it.

For example, we have a code that generates test IDs for two different tools we use on a project, Enzyme and Codeception:

const header = 'filename="pizza.rar"';
const filename = new URLSearchParams(header)
  .get('filename')
  .replaceAll(/^"|"$/g, '');
// → 'pizza'

It’s difficult to immediately spot any differences between these two lines of code. Remember those pairs of pictures where you had to find ten differences? That’s what this code does to the reader.

While I’m generally skeptical about extreme code DRYing, this is a good case for it.

Info: We talk more about the Don’t repeat yourself principle in the Divide and conquer, or merge and relax chapter.

const percent = 5;
const percentString = percent.toString().concat('%');

Now, there’s no doubt that the code for both test IDs is exactly the same.

Let’s look at a trickier example. Suppose we use different naming conventions for each testing tool:

const percent = 5;
const percentString = `${percent}%`;
// → '5%'

The difference between these two lines of code is hard to notice, and we can never be sure that the name separator (- or _) is the only difference here.

In a project with such a requirement, this pattern will likely appear in many places. One way to improve it is to create functions that generate test IDs for each tool:

const url = 'index.html?id=5';
if (~url.indexOf('id')) {
  // Something fishy here…
}

This is already much better, but it’s not perfect yet — the repeated code is still too large. Let’s fix this too:

const url = 'index.html?id=5';
if (url.includes('id')) {
  // Something fishy here…
}

This is an extreme case of using small functions, and I generally try to avoid splitting code this much. However, in this case, it works well, especially if there are already many places in the project where we can use the new getTestIdProps() function.

Sometimes, code that looks nearly identical has subtle differences:

const value = ~~3.14;

The only difference here is the parameter we pass to the function with a very long name. We can move the condition inside the function call:

const value = Math.floor(3.14);
// → 3

This eliminates the similar code, making the entire snippet shorter and easier to understand.

Whenever we encounter a condition that makes code slightly different, we should ask ourselves: is this condition truly necessary? If the answer is “yes”, we should ask ourselves again. Often, there’s no real need for a particular condition. For example, why do we even need to add test IDs for different tools separately? Can’t we configure one of the tools to use test IDs of the other? If we dig deep enough, we might find that no one knows the answer, or that the original reason is no longer relevant.

Consider this example:

if (dogs.length + cats.length > 0) {
  // Something fishy here…
}

This code handles two edge cases: when assetsDir doesn’t exist, and when assetsDir isn’t an array. Also, the object generation code is duplicated. (And let’s not talk about nested ternaries…) We can get rid of the duplication and at least one condition:

if (dogs.length > 0 && cats.length > 0) {
  // Something fishy here…
}

I don’t like that Lodash’s castArray() method wraps undefined in an array, which isn’t what I’d expect, but still, the result is simpler.

Avoid shortcuts

CSS has shorthand properties, and developers often overuse them. The idea is that a single property can define multiple properties at the same time. Here’s a good example:

const header = 'filename="pizza.rar"';
const filename = header.split('filename=')[1].slice(1, -1);

Which is the same as:

const header = 'filename="pizza.rar"';
const filename = header.match(/filename="(.*?)"/)[1];
// → 'pizza'

One line of code instead of four, and it’s still clear what’s happening: we set the same margin on all four sides of an element.

Now, look at this example:

const percent = 5;
const percentString = percent.toString().concat('%');

To understand what they do, we need to know that:

• when the margin property has four values, the order is top, right, bottom, left;
• when it has three values, the order is top, left/right, bottom;
• and when it has two values, the order is top/bottom, left/right.

This creates an unnecessary cognitive load, and makes code harder to read, edit, and review. I avoid such shorthands.

Another issue with shorthand properties is that they can set values for properties we didn’t intend to change. Consider this example:

const percent = 5;
const percentString = `${percent}%`;
// → '5%'

This declaration sets the Helvetica font family, the font size of 2rem, and makes the text italic and bold. What we don’t see here is that it also changes the line height to the default value of normal.

My rule of thumb is to use shorthand properties only when setting a single value; otherwise, I prefer longhand properties.

Here are some good examples:

const url = 'index.html?id=5';
if (~url.indexOf('id')) {
  // Something fishy here…
}

And here are some examples to avoid:

const url = 'index.html?id=5';
if (url.includes('id')) {
  // Something fishy here…
}

While shorthand properties indeed make the code shorter, they often make it significantly harder to read, so use them with caution.

Write parallel code

Eliminating conditions isn’t always possible. However, there are ways to make differences in code branches easier to spot. One of my favorite approaches is what I call parallel coding.

Consider this example:

const value = ~~3.14;

It might be a personal pet peeve, but I dislike when the return statements are on different levels, making them harder to compare. Let’s add an else statement to fix this:

const value = Math.floor(3.14);
// → 3

Now, both return values are at the same indentation level, making them easier to compare. This pattern works when none of the condition branches are handling errors, in which case an early return would be a better approach.

Info: We talk about early returns in the Avoid conditions chapter.

Here’s another example:

if (dogs.length + cats.length > 0) {
  // Something fishy here…
}

In this example, we have a button that behaves like a link in the browser and shows a confirmation modal in an app. The reversed condition for the onPress prop makes this logic hard to see.

Let’s make both conditions positive:

if (dogs.length > 0 && cats.length > 0) {
  // Something fishy here…
}

Now, it’s clear that we either set onPress or link props depending on the platform.

We can stop here or take it a step further, depending on the number of Platform.OS === 'web' conditions in the component or how many props we need to set conditionally

We can extract the conditional props into a separate variable:

const header = 'filename="pizza.rar"';
const filename = header.split('filename=')[1].slice(1, -1);

Then, use it instead of hardcoding the entire condition every time:

const header = 'filename="pizza.rar"';
const filename = header.match(/filename="(.*?)"/)[1];
// → 'pizza'

I also moved the target prop to the web branch because it’s not used by the app anyway.

When I was in my twenties, remembering things wasn’t much of a problem for me. I could recall books I’d read and all the functions in a project I was working on. Now that I’m in my forties, that’s no longer the case. I now value simple code that doesn’t use any tricks; I value search engines, quick access to the documentation, and tooling that help me to reason about the code and navigate the project without keeping everything in my head.

We shouldn’t write code for our present selves but for who we’ll be a few years from now. Thinking is hard, and programming demands a lot of it, even without having to decipher tricky or unclear code.

Start thinking about:

• When you feel smart and write some short, clever code, think if there’s a simpler, more readable way to write it.
• Whether a condition that makes code slightly different is truly necessary.
• Whether a shortcut makes the code shorter but still readable, or just shorter.

If you have any feedback, mastodon me, tweet me, open an issue on GitHub, or email me at artem@sapegin.ru. Get your copy.

The above is the detailed content of Washing your code: don't make me think. For more information, please follow other related articles on the PHP Chinese website!