Be Careful with Array Mutators

Photo by Corinne Kutz on Unsplash

A while ago, I wrote a linting rule to highlight an error that I’d made on a few occasions. Let’s look at the error — and its variations — and then at the rule that prevents it.

The error

If you are used to working with fluent APIs — perhaps with D3 or lodash — it feels natural to write Array-based code like this:

const developers = employees
  .filter(e => e.role === "developer")
  .map(e => e.name)
  .sort(); 

Which is fine. There’s no error in that code.

However, in situations in which you don’t need to filter or map, you might write something like this:

const sorted = names.sort();

It feels natural, but there’s a potential error in there. And it’s an error that can effect hard-to-find bugs. It’s also an error that’s easy to overlook in code reviews.

The sort method mutates the array. It sorts the array in place and returns a reference to the array itself. It does not return a sorted copy of the array. So the sorted and names variables both reference the same, sorted array. And it means that — unless the array was already sorted — any code using the names variable will be dealing with an array in which the elements that been rearranged.

So why is the code in the first snippet okay? It’s because sort is applied to the array returned by map — a transient array to which there are no references.

The rule

The fundamental problem is the use of the return value when the sort method is applied to an array that is referenced elsewhere in the program.

If the return value is not used, it’s fine — as the intent is clear:

names.sort();

And it’s also fine if the sort method is applied to an array that has no referencing variable:

const names = ["bob", "alice"].sort();

It is a potential problem if the result is assigned to another variable:

const sorted = names.sort();

Or assigned to a property:

const data = {
  names: names.sort()
};