What s wrong with Array.reduce ?

We use XO for our code linting. Recently I upgraded to its latest version and suddenly I had lots of errors as soon as I tried to commit (**).

What was wrong?

Well. It seems that there is a new trend out there.

Array.reduce is the new most hated guy.

It is disliked so much that a new ESLint rule was added to prevent - or reduce its usage.

What the heck!

I remember that when I started using it 3 years ago, it took me some time to understand the use case and find it cool and useful. And now, even though I don´t use it so often, it generally makes the code look quite nice and smart. Until now, I guess.

When I found all these eslint errors I was quite pissed, first because they were unexpected and I did not want to spend time fixing my code, nor cluttering it with eslint-disable comments to ignore it. But I was also quite intrigued by the reasons behind this opinionated choice from AVA contributors.

I read some of the comments in the thread and started reconsidering the snippets in our repository that contain Array.reduce.

Let´s consider this simplified example, where we have a list of records and we want to validate them and aggregate all the valid and invalid ones.

const isValid = (record) => // run some validation logic over the record props and return true or false


module.exports.analyzeResults = (records = []) => {
     return records.reduce(
         (acc, current) => {
           if (isValid(current)) {
                  acc.valid.push(current)
             } else {
                 acc.invalid.push(current)
             }
             return acc
         },
         {valid: [], invalid: []}
     )
}

With Array.reduce we can achieve it quite nicely, with one iteration only over the list and returning 2 new arrays.

What would be the alternative without Array.reduce and using Array.filter and Array.map instead, to still be as functional as possible?

module.exports.analyzeResults = (records = []) => {
    const valid = records.filter(r => isValid(r))
    const invalid = records.filter(r => !isValid(r))
    return {valid, invalid}
}
```

I know already what you are going to say:

> Ehi, but you are iterating over the list twice!!

True.

But the code is undoubtedly simpler and nicer to read.
So to some extent is the same objection many devs still say when it comes to use {% raw %}`
array.map(simplifyDataStructure).filter(bySomeProp).map(extractOnlySomething).filter(whatIwant)`
against doing everything in one single For Loop.

> Readability and Testability of the single operations

So unless you have a very very big dataset, it is really better to favour readability or simplicity rather than stuffing everything in a _complex_ reduced method.

I am not entirely sold on the new trend. And I am not going to rewrite all my methods using Array.reduce,  but this discussion really tickled my interest and helped me question my stance and coding.

What do you think?
---

(**)
> ProTip:  Use [Husky](https://www.npmjs.com/package/husky) to create a git hook so that whenever you try to commit XO is run and your code is linted.  that really helps to enforce coding standards in your team, and prevent unnecessary pickiness during code reviews.

Comments

[Brad]

This seems like a very personal choice. I personally am a fan of reduce as it forces me to identity what I'll be "managing" over the loop. I also don't usually use many of the for loop alternatives, even though they are essentially just as capable.

A perk of JS is its flexibility, so there are multiple ways to do the same thing. As a side-effect, people will always have opinions on which is the right way.

If you like reduce, keep using reduce. If someone else hates reduce that isn't on your project, don't worry about it. If they are on your project, then decide the best way forward together :)

[Heiker]

There is nothing wrong with reduce, it's only sin is to be slightly more difficult to understand than a for loop. The arguments against reduce can be reduced to this phrase.

I can do this with a for loop.

That's it. Anything that you can do with reduce you can do with a for loop, but since the for loop is the "simpler more readable" choice then it must be the right one.

[Davide de Paolis]

mmm. but this is true also for array.foreach and array.map etc. and i find those much nicer and simpler. I love reduce. but i kind of agree with the rant in the tweet to some extent

[Heiker]

I'm just repeating the only argument against reduce I think it make sense. By some misfortune reduce doesn't seem to "click" with people in the same way map, filter or forEach do.

[Jon Bristow]

which is a shame, because map, filter, and forEach are just reduce in disguise.

[Alain Van Hout]

Though, for completeness, all of those are just loops in disguise (which isn't an argument for or against it).

[Jon Bristow]

Only if you consider loops and tail recursion to be the same thing, which is debatable.

Also, it's only the same when applied to lists.

Other monadic structures may not act as loops at all, when called with reduce aka fold

[Andreas Møller]

In this case simpler and more readable is very subjective.

[Davide de Paolis]

Agree. Readability is anyway mostly a matter of being used to read such code. :-)

[Heiker]

That's why it's wrapped in quotes.

[Chidiebere Ogujeiofor]

Is it not safe for us to say that once you understand how the reduce function works, then reading/understanding it becomes quite easy.

I agree that the loops are more readable though but I don't think it's enough to tell folks 'never' to use reduce

[Andreas Møller]

Nothing. Most people are more familiar with imperativ or object orientated programming, so so functional programming practices are unfamiliar. Unfortunately for some developers unfamiliar automatically means it's wrong.

[David]

This about sums it up.

Once people are familiar with reduce, it looks just like any other boiler-plate. The callback function used is the "reducer". I would find it weird if I saw React people preferring the old for-loop.

I think the test-ability argument is a moot point in the thread, because if there are many conditions and a really fat reducer function, that needs testing, not the reducer in the context of a reduce invocation.

An argument can be made on both grounds. Functional AND declarative code is typically preferred. A lot of people have already used this example but in a less readable fashion, but let's really illustrate declarative code:

// simple to unit test this reducer
function maximum(max, num) { return Math.max(max, num); }

// read as: 'reduce to a maximum' 
let numbers = [5, 10, 7, -1, 2, -8, -12];
let max = numbers.reduce(maximum);

[Costin Manda]

I like this approach (plus I finally understood where reducer comes from :D - in .NET it's called Aggregate). However, I also think that if one is not going to (re)use multiple types of reducers, there is no need to use reduce to emulate a forEach or even a normal loop.

But congratulations on the conciseness of your code!

[David]

Yeap! I've never done C#, but the analogs are listed in this SO thread: stackoverflow.com/questions/428798...

Yeah, it was a contrived example from many people in this thread. I also don't use JS regularly at all anymore which is why I often default to ES5 syntax.

I typically end up using reduce for "partitioning" or performing only a single iteration through the collection to include/exclude/filter into a different data type, like a set, map, etc to keep things atomic... we've all seen people declare combination of multiple arrays, array and object, array and map, etc at the TOP of the file and the actual for-loop is modifying it 100 lines down, and some other code that modifies it in between... that's really really really bad and shitty for anyone needing to debug it. These functional facilities over collections make them atomic and you know where to look if there's a bug -- the reducer (hopefully unit-tested)...

I've used reduce for a lot of this (changing data-types, and accumulation to a different collection type)

let numStrings = ['1', '2', '3'];
numStrings.reduce((map, num) => {
  const num = parseInt(num);
  return num % 2 === 0 ? map.evens.push(num) : map.odds.push(num);
}, {evens: [], odds: []});

// RESULT (strings are converted to integers in a nice map/object of evens and odds)
// {evens: [2], odds: [1, 3]}

Some people call this "partitioning". The above would be difficult to express with just a filter call or for-loop without scattering variables. The thing that gets most people is where the "initial value" {evens: [], odds: []} is. People are just very used to seeing it right above an iteration

[Costin Manda]

I see what you are saying, but unless you are going to reduce that function, this can just as well be expressed by a forEach or a loop. One declaration of a variable above it doesn't bother me. Cheers!

[David]

forEach just abstracts away the iterator variable boilerplate, but could still leave the collection variables scattered about, like at the top of the file and make it possible for someone to mutate them in between... but yah!

[Joseph]

Pardon for jumping in but fold are very well studied functions, as you know :) Sorry for dropping a link, but it speaks much better than I could: Fold Higher Order Function

[Hutch]

@wulymammoth your code is slightly wrong. You have to return the accumulator (the object of arrays, this case). You're instead returning whatever push returns (which is the length of the array after pushing). (Also, you're re-declaring the parameter num as a const, which isn't allowed, but that's a simpler fix.)

[David]

All good catches

[Joseph]

Calculating lcm of more than two numbers also becomes impressively simple and beautiful. For example my AoC day 12 solution.

function gcd(a, b) {
  if (!b) return b === 0 ? a : NaN;
  return gcd(b, a % b);
}

function lcm(a, b) {
  return (a / gcd(a, b)) * b;
}
const systemPeriod = periods.reduce(lcm)

[David]

☝️

[Peter Vivo]

let max = Math.max(...[5, 10, 7, -1, 2, -8, -12])

[David]

Yeap! Just re-using the maximum example that others have used for illustration purposes only. JS already has this available in the standard library :)

[Joseph]

Exactly! I reckon anything overused is bad. However to straight up deface the enormous power a fold function gives is just wrong itself. Structural transformations are a thing, and there's a tool for them, period.

[Daniel Ziltener]

There is definitely nothing wrong with reduce. And I'm sorry, but someone who finds reduce "unreadable" should not be programming linters.

[Nathan Kallman]

I agree your example is one where reduce is better expressed in other ways. But I think there's cases where reduce is the correct metaphor and the clearest construction:

const max = (numbers) => numbers.reduce((max, number) => number > max ? number : max);

I will never be convinced that a for loop is more readable than that (and map and filter aren't options for this type of computation)

[Pacharapol Withayasakpunt]

I almost always to write first argument as prev or acc; therefore,

numbers.reduce((prev, current) => current > prev ? current : prev)

About readability, I guess it depends on how you visualize.

[Peyton McGinnis]

The JavaScript community continues to follow the trend of syntactical additions being requested, added, praised, and then criticized because of unreadability and mental overhead. If you want to use reduce because it fits your context and provides clarity, then use it. The problem is that too many people misunderstand why the convention was initially introduced — not to provide a "swiss-army knife" replacement for for loops, but to alleviate complexity and provide more purity when writing functional code.

[Timo Zimmermann]

My personal rule of thumb is to use a for-loop if the reduce expression doesn’t fit in one line. That doesn’t mean that reduce functions that take up multiple lines are in general hard to read

[John Kazer]

The problem with reduce is that there are two things happening in parallel. What happens to the accumulator and the function operating on the list. This situation always makes reasoning harder, at least until you've practised a bit.

I guess you can say a for loop has same issue with the iterator and the operations carried out but they are sort of separate things.

The reduce example in the article added a third piece of action, the choice between valid and invalid. To me that is too many moving parts for a functional approach and the latter example using two filters is better.

Yes it is slower with the two list iterations but if you need speed then use a for loop.

[Davide de Paolis]

I really appreciate how you articulated your thoughts. This is how I see it too. Thanks

[leob]

Goes too far that a linter (just a tool) is trying to force its questionable opinion on the developer ... that tool can't be reconfigured to just take this rule out?

[Davide de Paolis]

of course it can. as they mention in the Repo: XO is a Opinionated but configurable ESLint wrapper :-)

[leob]

Well problem solved then :-)

Anyway yes, I do see a potential problem with Array.reduce - it's so generic that it can be misused ... but for a linter to tell me not to use it, that feels a bit patronizing.

[Sdu]

but linters also suggest not to use many other practices when programming. And as a Junior I appreciate such as should I be able to justify the use of something blocked by a linter, I will then adjust linter rules to say I know what I'm doing

[gyorgygutai]

Another prime example of the JS community getting bored.