mediamonks / frontend-coding-standards

Media.Monks - Frontend Coding Standards
60 stars 23 forks source link

How and when we should use certain array methods and loops #63

Open psimk opened 3 years ago

psimk commented 3 years ago

JS gives us a lot of freedom in how to iterate over arrays. We can use existing for loops in the same situations as array methods. However, the readability, safety and performance can greatly differ depending on what you are trying to achieve and what method/for loop you use to do that.

for ... of ...

for (const foo of foos) {
  // do something with foo
}

and

for(;;;)

for (let index = 0; index < foos.length; index++) {
 const foo = foos[index];
  // do something with foo
}

:arrow_up: are roughly equal to :arrow_down:

[].forEach(...)

foos.forEach((element) => {
  // do something with foo
});

Even though the above loops are have the exact same goals - loop through an array of elements, in certain scenarios it can better to pick one over another.

Example:

for(;;;)

Usually referred to as the traditional for loop, it provides the most flexibility, but are the most verbose and that verbosity can be a cause for errors. The index if used is mutable, you must define your own condition on "how many iterations should this loop for" and with last expression you define "how this loop will iterate".

However, if you require performance and the least possible iterations over the array, then this is your best choice.

for ... of ...

Usually referred to as the iteration loop as it can loop through any iterable. It's also usually seen as an improvement over the traditional loop as it removes all of the verbosity that's usually not needed. The fact that it loops through iterables also makes it a perfect choice for custom data structures.

The obvious downside is that you don't have access to the index of the current element, but you can in fact access the index by using the [].entries() method on some data structures.

for (const [index, foo] of foos.entries()) {
  // do something with foo and it's index
}

[].forEach(...)

This came before for ... of ..., but can be seen as the same thing, but only for arrays, iterables would need to be converted to arrays first. It's callback based iteration makes it a good choice wanting to pass all elements through some side effect function. Because it's an array method, it can be chained together with other array methods to create very readable code.

Things get even more complicated when taking into account all of the array methods.

The downside for the main array methods filter, map and reduce is that they can't return early. Also none of the array methods can use use break or continue. Returning early and the extra keywords if used poorly can negatively impact the readability of a standard for loop, so not being able to use them at all in array methods can be seen as an advantage in some cases.

Moreover, the methods which allow passing a callback are also in danger of being passed a generic function which accepts more than a single element. Example:

["1", "2", "3"].map(parseInt);

This returns [1, NaN, NaN] instead of the intended [1, 2, 3] as parseInt's second argument denotes the numeral system parseInt should be using to parse the value. So in cases like this, the function must be wrapped in another inline function to prevent this.

["1", "2", "3"].map(item => parseInt(item));

It should be made clear that e.g. you should use includes if you are matching primitives or values with the same references or use some over find when you are matching more complex values and only care if the array has that item, not what that item is.

I hope the above does justice in explaining the ambiguous nature of looping in JS and that it's worth while writing out some guidelines on when what should be used.

There's also an ongoing question of for ... of ... vs reduce vs map + find/filter


Personally I am really against using reduce for tasks more complex than e.g. counting, as it is generally very unclear what the reduce is reducing. map and filter have solid definitions, one maps values to something and one filters values based on something. Reduce is just reducing, which may also not be correct as the output can be anything and not just an array.

My thoughts on the matter are to either chain existing array methods to achieve the same, as each chain can be seen as a step, or to use a for ... of ... loop.

A really often use case for reduce is to map an array to an object and I think it's a great example on what I personally don't like about reduce.

[1, 2, 3].reduce((accum, number) => {
  accum[String(number)] = number;
  return accum;
}, {} as Record<string, number>);

I am used to reading code and everything by that matter, sequentially from top to bottom. With reduce I first parse the callback parameter names, then I need to go to the end to see the default value of accum and only after that I can dive into the callback and try and figure out what it's doing. This would also be exaggerated if the callback contained more complex logic.

Whereas using for ... of ..., I can simply go top to bottom and not have to worry about reading it in a certain way.

const obj: Record<string, number> = {};

for (const number of [1, 2, 3]) {
  obj[String(number)] = number;
}

However, there's another approach to do the above, using modern object methods. Which is shorter, more concise and retains the immutability of reduce thanks to map. Additionally, in TS, you don't have to deal with types as with Object.fromEntries they are already inferred.

const obj = Object.fromEntries([1, 2, 3].map((number) => [String(number), number]));

I would create the PR explaining all of these oddities, but I need more opinions on the matter:

mmnathan commented 3 years ago

maybe address for..in and (do) while, while you're at it