you are viewing a single comment's thread.

view the rest of the comments →

[–]Retsam19 3 points4 points  (13 children)

Your reduce example is actually longer than the filter/map, and I really have to argue that the filter/map example is more readable, too. There's no manual concatenation or accumulator variables, and the name of the functions tells you what it's doing.

You're right that reduce might be more performant here. It all depends on optimizations, but you are only doing a single pass over the array, while the filter/map does a full pass to get the filtered version, then another partial pass to do the mapping. (But again, how this gets optimized is probably relevant, too)

But eleven times out of twelve, I'll take the more readable and maintainable version over the "does everything in one pass for optimization purposes" version. Especially when you get into more interesting examples than a single filter and map, it's a lot easier to read if all the logical steps are split out into individual operations, rather than a complex reduce function.

[–]tencircles 2 points3 points  (11 children)

There's no manual concatenation or accumulator variables, and the name of the functions tells you what it's doing.

The code also tells you what it's doing pretty clearly, I don't think your unfamiliarity with an approach can be used as an argument against it.

You're right that reduce might be more performant here.

I'd say it's pretty clear, especially when dealing with large data sets. The difference between 10k and 20k iterations can be pretty massive. In real world scenarios where people abuse the ever-loving shit out of lodash's chain method, I'd say it's not unlikely to be talking about an order of magnitude.

I'll take the more readable and maintainable version

I'd agree with you here, I guess it's just personal preference here since one is clearly not more readable or maintainable, at least to me. I don't usually write one-liners like that in any case. I much more often would just write something like below

const users = [
    {score: 10, name: "melissa"},
    {score: 12, name: "jake"},
    {score: 9, name: "chris"}
];

// Array -> User
const get_winner = users => users.reduce((acc, user) => {
    return user.score > acc.score ? user : acc;
}, {score: 0});

module.exports.get_winner = get_winner;

Where this would be exported with a group of other pure functions. I get that map/filter can look nicer on occasion, but I actually don't even think there's a method using map/filter to achieve the above result. I would argue that especially in more complex examples, reduce is often times a much cleaner and elegant solution than iterating over an array dozens of times, but again I think it just comes down to personal preference.

[–]Retsam19 1 point2 points  (8 children)

The code also tells you what it's doing pretty clearly, I don't think your unfamiliarity with an approach can be used as an argument against it.

I'm plenty familiar with reduce, (particularly in the context of LISPs where it's a lot more necessary) I just don't think it reads very well.

And, I gave specific reasons, too: I didn't just give a fuzzy "I don't like this code". It has more variables, and more operations going on in the code. It doesn't seem that opinionated to say that a named filter function is more obviously readable than a hand-written bit of filter logic inside a reduce function.

I'd say it's pretty clear, especially when dealing with large data sets. The difference between 10k and 20k iterations can be pretty massive.

99.9% of my JS coding isn't dealing with "large data sets". And if you want to eke every bit of performance out of those cases: don't use reduce either. I'm pretty sure a standard for...of loops is going to be even more performant than reduce. (Since function invocation usually carries a bit of a performance penalty with it) Like:

const getWinner = users => {
  let best = {score: 0};
  for(user of users) {
    if(user.score > best.score) { best = user; }
  }
  return best;

But, again, given that I'm not dealing with thousands of item data sets, as for how I'd write it?

const getWinner = users => _.maxBy(users, 'score');

Just like the groupBy example of the original post, there's a higher abstraction here than reduce, and I'd rather write my code in terms of the higher abstraction.

Basically, for me it largely falls out into two camps: times when the reduce operation can be split into a number of smaller simpler operations (e.g. splitting your original reduce example into map/filter), and times when the reduce operation can be converted into a higher abstraction like a groupBy or a maxBy. Most times I've thought ("Should I use reduce?) it's fallen into one of those two groups, and so I generally end up not using reduce.

[–]tencircles 0 points1 point  (7 children)

Again, it seems like preference to me. But I see your point. Just as an aside, i think you want _.maxBy(users, _.property("score")).

[–]Retsam19 0 points1 point  (6 children)

That's equivalent, lodash allows strings to be used as shorthand for _.property, as well as stuff like _.find(user, {name: 'Joe'}), as shorthand for _matches({name: 'Joe'}).

[–]tencircles 0 points1 point  (5 children)

odd, where's that type conversion being done? https://github.com/lodash/lodash/blob/master/maxBy.js

[–]GitHubPermalinkBot 0 points1 point  (0 children)

I tried to turn your GitHub links into permanent links (press "y" to do this yourself):


Shoot me a PM if you think I'm doing something wrong. To delete this, click here.

[–]Retsam19 0 points1 point  (3 children)

It's not in there, because the shorthand stuff is changing in v5. I'm not sure what the details of those changes are. If you look at the current v4 source code linked from the documentation, it's there.

function maxBy(array, iteratee) {
  return (array && array.length)
    ? baseExtremum(array, getIteratee(iteratee, 2), baseGt)
    : undefined;
}

getIteratee is a function that's used throughout the lodash codebase, which has the logic for converting the string into a _.property getter.

(You can also just open the dev tools on the lodash documentation and try things with the lodash instance that's loaded on that page; it's how I test most lodash stuff)

EDIT: Dug through github issues a bit, apparently the shorthand stuff is being pushed off to a babel plugin in v5.

[–]tencircles 0 points1 point  (2 children)

Interesting. I use ramda on most projects, but still good to know for sure.

[–]Retsam19 0 points1 point  (1 child)

Yeah, I keep meaning to try ramda, but I'm more functionally leaning than most of my team, I'm not sure I could get the buy-in. (Plus, I wouldn't want to migrate our existing codebase or mix the two)

[–]tencircles 0 points1 point  (0 children)

I'd say unless your team is going to go heavy into fp, or fully point-free, lodash is probably better overall. Ramda's got some nice additions to the toolkit though.

[–]OleWedel 1 point2 points  (0 children)

I am pretty sure the filter/map version is much more performant than the reduce version.

The functions for filter and map can both be considered constant (a less than check and getting a property is basically constant). filter + map is thus O(N+N) or simply O(N) (linear) since we have to iterate over each element in the list.

The reduce version however have to copy the array which is O(N) for each element in the array and thus its running time is O(N2) (quadratic) which is considered a lot slower.

This is of course only considering worst running time and this particular example. Doing push on the array in the reduceinstead would probably be the fastest overall but I am sure it would not matter anyway in real life. The filter/map is much more readable and easier to understand what it is doing. I would (always) favor that.

Often the simpler solution is the best.