Stop abusing .map()!

Every once in a while when I do code review or visit StackOverflow I stumble upon code snippets that look like this:

const fruitIds = ['apple', 'oragne', 'banana'];
fruitIds.map((id) => {
   document.getElementById(`fruit-${id}`).classList.add('active');
});

So as you can see it's just a simple iteration where for every element in the fruitIds array we add active class to a certain HTML element in a DOM.

Many programmers (especially new) wouldn't notice anything wrong with the code above. However, there is a one major issue here - the usage of .map(). Let me explain.

What is wrong with .map()?

Well, there is completely nothing wrong with this particular array method. In fact, I think it is very handy and beautifully wraps one of the iteration patterns - mapping.

In simple words, mapping is an operation which applies a function to every element of a collection and returns a new collection with elements changed by the mentioned function. For example, if we have an array of numbers const nums = [1, 2, 3, 4]; and would like to receive a new array of doubled numbers, we could map the original array to a new one like this (in JavaScript):

const biggerNums = nums.map((n) => n * 2);
// >> [2, 4, 6, 8];

The biggerNums array would consist of numbers from the original nums array multiplied by 2.

Notice how .map() is used - we assigned the result of this method to a new variable called biggerNums. I have also mentioned earlier that mapping is an operation that returns a new collection of elements. And this is the very reason the code snippet showed at the beginning of this article is wrong. The .map() returns a new array - always - and if we don't need that array, we shouldn't use .map() in the first place. In this particular case (simple iteration) a different array method should be used - .forEach() - which is specifically designed for such cases. It doesn't return a new collection, it simply walks through an array and invokes a callback function for every element allowing you to do something for each of them.

So the correct version of the mentioned snippet should look like this:

// good way
const fruitIds = ['apple', 'oragne', 'banana'];
fruitIds.forEach((id) => {
   document.getElementById(`fruit-${id}`).classList.add('active');
});

We don't need a new array so we simply iterate over the fruitIds array and add the active class to an HTML element for each of the array items.

Okay, but why should I care? .map() is shorter and easier to write than .forEach(). What could possibly go wrong?

Consequences of abusing .map()

One of the worst consequences of abusing .map() is the fact that it returns a new redundant array. To be more specific - it returns a new array of the same size as the one this method was called on. It means that if we have an array of 1000 elements, .map() will return a new array of 1000 elements - every time.

In JavaScript, all functions return a value. Even if we don't use the return keyword, the function will return undefined implicitly. That's how the language has been designed. This rule also applies to callbacks - they are functions too.

Having said that, let's get back to the original example:

// wrong way
const fruitIds = ['apple', 'oragne', 'banana'];
fruitIds.map((id) => {
   document.getElementById(`fruit-${id}`).classList.add('active');
});

What happens here? An array of fruit IDs is created and then it's mapped to another array of the same size. Even though the array returned by .map() is not used, it does take place in memory. This new (unused) array looks like this:

[undefined, undefined, undefined]

It's because the callback passed to the .map() method does not have the return keyword and as we know, if there is no return, undefined is returned implicitly.

How bad is it? Very bad. In this particular example it won't bring any serious consequences - there are only three items in the array so creating another three-element array won't cause any problems. However, the problem arises when we deal with big arrays of complex data. If we want to iterate over an array of five thousand objects and we abuse .map(), we create another array of five thousand elements - undefineds. So we end up storing 10 000 elements in memory from which a whole half is redundant. It is a very non-optimal practice and in some scenarios it may even lead to the application overload. This is why we should pick right methods to the right tasks.

Summary

There are many practices that are essentialy bad, but the negative consequences will start to be visible only when dealing with bigger datasets. One of such practices is abuse of .map(). When operating on small arrays, it won't cause any hurt. But when we make this mistake with a bigger array it will start overloading our application and it may be quite hard to debug.

This is why we should never let it pass by and whenever we see this abuse, we should take care of it. I hope now you understand why.