Washing your code: avoid loops

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.

Traditional loops, like for or while, are too low-level for common tasks. They are verbose and prone to off-by-one error. You have to manage the index variable yourself, and I always make typos with lenght. They don’t have any particular semantic value except that you’re doing some operation probably more than once.

Replacing loops with array methods

Modern languages have better ways to express iterative operations. JavaScript has may useful methods to transform and iterate over arrays, like .map() or .find().

For example, let’s convert an array of strings to kebab-case with a for loop:

const names = ['Bilbo Baggins', 'Gandalf', 'Gollum'];
for (let i = 0; i < names.length; i++) {
  names[i] = _.kebabCase(names[i]);
}

And now with the .map() method:

const names = ['Bilbo Baggins', 'Gandalf', 'Gollum'];
const kebabNames = names.map(name => _.kebabCase(name));

We can shorten it even more if our processing function accepts only one argument, and kebabCase from Lodash does:

const names = ['Bilbo Baggins', 'Gandalf', 'Gollum'];
const kebabNames = names.map(_.kebabCase);

But this may be a bit less readable than the expanded version, because we don’t see what exactly we’re passing to a function. ECMAScript 6’s arrow functions made callbacks shorter and less cluttered, compared to the old anonymous function syntax:

const names = ['Bilbo Baggins', 'Gandalf', 'Gollum'];
const kebabNames = names.map(function(name) {
  return _.kebabCase(name);
});

Or let’s find an element in an array with a for loop:

const names = ['Bilbo Baggins', 'Gandalf', 'Gollum'];
let foundName;
for (let i = 0; i < names.length; i++) {
  if (names[i].startsWith('B')) {
    foundName = names[i];
    break;
  }
}

And now with the .find() method:

const names = ['Bilbo Baggins', 'Gandalf', 'Gollum'];
const foundName = names.find(name => name.startsWith('B'));

In both cases I much prefer versions with array methods than with for loops. They are shorter and we’re not wasting half the code on iteration mechanics.

Implied semantics of array methods

Array methods aren’t just shorter and more readable; each method has its own clear semantic:

• .map() says we’re transforming an array into another array with the same number of elements;
• .find() says we’re finding a single element in an array;
• .some() says we’re testing that the condition is true for some array elements;
• .every() says we’re testing that the condition is true for every array element.

Traditional loops don’t help with understanding what the code is doing until you read the whole thing.

We’re separating the “what” (our data) from the “how” (how to loop over it). More than that, with array methods we only need to worry about our data, which we’re passing in as a callback function.

When you use array methods for all simple cases, traditional loops signal to the code reader that something unusual is going on. And that’s good: you can reserve brain resources for better understanding the unusual, more complex cases.

Also don’t use generic array methods like .map() or .forEach() when more specialized array methods would work, and don’t use .forEach() when .map() would work:

const names = ['Bilbo Baggins', 'Gandalf', 'Gollum'];
const kebabNames = [];
names.forEach(name => {
  kebabNames.push(_.kebabCase(name));
});

This is a more cryptic and less semantic implementation of .map(), so better use .map() directly like we did above:

const names = ['Bilbo Baggins', 'Gandalf', 'Gollum'];
const kebabNames = names.map(name => _.kebabCase(name));

This version is much easier to read because we know that the .map() method transforms an array by keeping the same number of items. And unlike .forEach(), it doesn’t require a custom implementation nor mutate an output array. Also the callback function is now pure: it doesn’t access any variables in the parent function, only function arguments.

Dealing with side effects

Side effects make code harder to understand because you can no longer treat a function as a black box: a function with side effects doesn’t just transform input to output, but can affect the environment in unpredictable ways. Functions with side effects are also hard to test because you’ll need to recreate the environment before each test and verify it after.

All array methods mentioned in the previous section, except .forEach(), imply that they don’t have side effects, and that only the return value is used. Introducing any side effects into these methods would make code easy to misread since readers wouldn’t expect to see side effects.

.forEach() doesn’t return any value, and that’s the right choice for handling side effects when you really need them:

errors.forEach(error => {
  console.error(error);
});

for of loop is even better:

• it doesn’t have any of the problems of regular for loops, mentioned in the beginning of this chapter;
• we can avoid reassignments and mutations, since we don’t have a return value;
• it has clear semantics of iteration over all array elements, since we can’t manipulate the number of iterations, like in a regular for loop. (Well, almost, we can abort the loops with break.)

Let’s rewrite our example using for of loop:

for (const error of errors) {
  console.error(error);
}

Sometimes loops aren’t so bad

Array methods aren’t always better than loops. For example, a .reduce() method often makes code less readable than a regular loop.

Let’s look at this code:

const tableData = [];
if (props.item && props.item.details) {
  for (const client of props.item.details.clients) {
    for (const config of client.errorConfigurations) {
      tableData.push({
        errorMessage: config.error.message,
        errorLevel: config.error.level,
        usedIn: client.client.name
      });
    }
  }
}

My first reaction would be to rewrite it with .reduce() to avoid loops:

const tableData =
  props.item &&
  props.item.details &&
  props.item.details.clients.reduce(
    (acc, client) => [
      ...acc,
      ...client.errorConfigurations.reduce(
        (inner, config) => [
          ...inner,
          {
            errorMessage: config.error.message,
            errorLevel: config.error.level,
            usedIn: client.client.name
          }
        ],
        []
      )
    ],
    []
  );

But is it really more readable?

After a cup of coffee and a chat with a colleague, I’ve ended up with a much cleaner code:

const tableData =
  props.item &&
  props.item.details &&
  props.item.details.clients.reduce((acc, client) =>
    acc.concat(
      ...client.errorConfigurations.map(config => ({
        errorMessage: config.error.message,
        errorLevel: config.error.level,
        usedIn: client.client.name
      }))
    ),
    []
  );

I think I still prefer the double for version, but I’ll be happy with both versions, the original and the second rewrite, if I had to review such code.

(Though tableData is a really bad variable name.)

Iterating over objects

There are many ways to iterate over objects in JavaScript. I equally dislike them all, so it’s hard to choose the best one. Unfortunately there’s no .map() for objects, though Lodash does have three methods for object iteration, so it’s a good option if you’re already using Lodash in your project.

const allNames = {
  hobbits: ['Bilbo', 'Frodo'],
  dwarfs: ['Fili', 'Kili']
};
const kebabNames = _.mapValues(allNames, names =>
  names.map(name => _.kebabCase(name))
);

If you don’t need the result as an object, like in the example above, Object.keys(), Object.values() and Object.entries() are also good:

const allNames = {
  hobbits: ['Bilbo', 'Frodo'],
  dwarfs: ['Fili', 'Kili']
};
Object.keys(allNames).forEach(race =>
  console.log(race, '->', allNames[race])
);

Or:

const allNames = {
  hobbits: ['Bilbo', 'Frodo'],
  dwarfs: ['Fili', 'Kili']
};
Object.entries(allNames).forEach(([race, value]) =>
  console.log(race, '->', names)
);

I don’t have a strong preference between them. Object.entries() has more verbose syntax, but if you use the value (names in the example above) more than once, the code would be cleaner than Object.keys(), where you’d have to write allNames[race] every time or cache this value into a variable at the beginning of the callback function.

If I stopped here, I’d be lying to you. Most of the articles about iteration over objects have examples with console.log(), but in reality you’d often want to convert an object to another data structure, like in the example with _.mapValues() above. And that’s where things start getting uglier.

Let’s rewrite our example using .reduce():

const kebabNames = Object.entries(allNames).reduce(
  (newNames, [race, names]) => {
    newNames[race] = names.map(name => _.kebabCase(name));
    return newNames;
  },
  {}
);

With .forEach():

const allNames = {
  hobbits: ['Bilbo', 'Frodo'],
  dwarfs: ['Fili', 'Kili']
};
const kebabNames = {};
Object.entries(allNames).forEach(([race, names]) => {
  kebabNames[race] = names.map(name => name.toLowerCase());
});

And with a loop:

const kebabNames = {};
for (let [race, names] of Object.entries(allNames)) {
  kebabNames[race] = names.map(name => name.toLowerCase());
}

And again .reduce() is the least readable option.

In later chapters I’ll urge you to avoid not only loops but also reassigning variables and mutation. Like loops, they often lead to poor code readability, but sometimes they are the best choice.

But aren’t array methods slow?

You may think that using functions is slower than loops, and likely it is. But in reality it doesn’t matter unless you’re working with millions of items.

Modern JavaScript engines are very fast and optimized for popular code patterns. Back in the day we used to write loops like this, because checking the array length on every iteration was too slow:

var names = ['Bilbo Baggins', 'Gandalf', 'Gollum'];
for (var i = 0, namesLength = names.length; i < namesLength; i++) {
  names[i] = _.kebabCase(names[i]);
}

It’s not slow anymore. And there are other examples where engines optimize for simpler code patterns and make manual optimization unnecessary. In any case, you should measure performance to know what to optimize, and whether your changes really make code faster in all important browsers and environments.

Also .every(), .some(), .find() and .findIndex() will short circuit, meaning they won’t iterate over more array elements than necessary.

Start thinking about:

• Replacing loops with array methods, like .map() or .filter().
• Avoiding side effects in functions.

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