Lets say i have a list and i want to add some values into it using a mapping function:
const arr = [1, 2, 3, 4, 5];
const anotherArr = [];
I do this using a functional approach:
arr.map((item) => anotherArr.push(item);
Is this an antipattern / bad logic - that is, not using the mapping operation return value for anything? Are there any good resources on this?
(i know this logic is silly and i can just copy the list - that is not the point of my question)
Yes, this is an anti-pattern. The .map
method has clear usage - you transform the contents of one array into another. You do that by supplying a mapping function that expresses the relationship of each elements, e.g, to transforming a letter to its position in the alphabet can be expressed via the function:
function letterToPositionInAlphabet(letter) {
return letter.toUpperCase().charCodeAt(0) - 64;
}
So mapping an array of letters via this function will give you an array with each of their positions:
function letterToPositionInAlphabet(letter) {
return letter.toUpperCase().charCodeAt(0) - 64;
}
const letters = ["a", "b", "c"];
console.log(letters.map(letterToPositionInAlphabet));
The mapping operation is idiomatic and part of understanding the code. If you see someArr.map(someFn)
sets up expectations and it's easy to understand what sort of operation is happening, without needing to known the contents of either the array or the function. When you see letters.map(letterToPositionInAlphabet)
it should be trivial to understand what the intent is - get the positions in the alphabet of some letters.
However, using .map
as .forEach
is breaking that intended meaning and can be confusing to read. Having this
function playerToPlaceInRankList(player) {
const position = lookupPlayerRank(player);
positionsArr.push(position);
}
/* many lines later */
players.map(playerToPlaceInRankList);
/* more code */
The line which seems like it performs mapping also immediately looks wrong because the return value is ignored. Either that line is not needed, or you have to lookup what playerToPlaceInRankList
does in order to find out what is actually happening here. That's unnecessary mental load for just reading what should be a straight forward line and self-documenting line of code.
The same applies to using other methods like .filter
, .find
, .every
, .some
, etc. Don't use those just because they iterate over the array, if what you want is not what they are intended to do.