Misusing `map`

Unfortunately, since late 2019 or early 2020 or so I'm increasingly seeing people (for instance, on Stack Overflow) misusing map like this:

// ANTIPATTERN
someArray.map(element => {
    // ...do something with `element`...
});

Note that:

  • The map callback doesn't return anything
  • The return value of map isn't used

This is an antipattern.

Sadly, it seems like someone, somewhere, is doing their students/followers the disservice of teaching it. I don't think all of these people spontaneously started doing this on their own the last year and a half. (An alternative explanation is people being confused by JSX, seeing map inside a JSX {____} expression like {something.map(x => <Component x={x} />)} and somehow generalizing from that usage (which is correct, the callback returns something, and the new array is used) into thinking they should use it for loops in general, even when not returning anything or using the new array. While I'd prefer that confusion explanation, sadly I think the person teaching poor practices is more likely.)

What makes this an antipattern?

  1. Semantics: map is for a mapping operation, each A in the array becomes a B in the result array, transformed by the callback. If you're not doing mapping operation, using map is like saying "purple" when you meant to say "fruit."
  2. map creates and returns a new array (whether you use it or not). So if you get in the habit of using map instead of for-of or forEach or similar, you're getting in the habit of creating lots of unnecessary, short-lived array objects. This increases the amount of memory churn in your program, for no benefit whatsoever. It's a rare program where it will matter, but still, it's silly to add memory churn for no reason.

But semantics (loosely, the meaning of words/sentences) is the main thing. If I'm doing code maintenance and I see a map where nothing is using the result array, I see a bug — and I'll waste time looking for what was supposed to be done with that result array.

What should we do instead?

Use for-of:

for (const element of someArray) {
    // ...do something with `element`...
}

...or forEach

someArray.forEach(element => {
    // ...do something with `element`...
});

...or any of your other options for looping through an array.

Happy coding!

Have a question or comment about this post? Ping me on Mastodon at @tjcrowdertech@hachyderm.io!