jashkenas / underscore

JavaScript's utility _ belt
https://underscorejs.org
MIT License
27.33k stars 5.53k forks source link

still recommend flatMap or concatMap #452

Closed rebcabin closed 12 years ago

rebcabin commented 12 years ago

with reference to issue 436, I agree that documenting the second argument to flatten is a good start, but it didn't quite close the issue for me. I would still recommend adding a "flatMap" or "concatMap" primitive to the base library so that we get proper chain composability. Adding this new primitive would not break anything, but would make chains of array-valued maps composable. For instance, imagine the following expression, in pseudo-C#, that represents the line items for high-value orders from a list of customers: Customers .Where(customer => customer.Orders .SelectMany(order => order.LineItems) .Sum() > 1000) .SelectMany(customer => customer.Orders) .SelectMany(order => order.LineItems) ;

to write this in underscore with a new flatMap primitive, the "SelectMany"s become "flatMaps."

I implemented a proposal for flatmap in a fork of underscore's source, along with a few unit tests for your consideration. I don't have docco running and I didn't produce a "min" version of the javascript, so it's just the javascript and the unit tests. https://github.com/rebcabin/underscore/commit/4f90bab75d8f69df8d8d0317354f9f5981e2fedb

jashkenas commented 12 years ago

That's great -- when sending pull requests, you should never change docs or build a minified version.

That said, for chaining, I think that our current explicit methods are already superior... (pardon my CoffeeScript)

_.chain(Customers)
  .map((customer) -> customer.orders()).flatten()
  .map((order) -> order.lineItems()).flatten()
  .value()
rebcabin commented 12 years ago

Well, ok, if what you meant was calls of flatten WITH the newly documented second argument set to "true." :), then you have composable chains, all goodness. Any particular reason why the fact that Haskell's bind is isomorphic to concatMap doesn't increase the appeal of concatMap to you? Bind is Hugely general, definitional, foundational to all monads. If you ever choose to cover other monads, like the continuation monad (hello, callbacks) you will end up implementing something just like this pattern over and over again. Map and flattenOnce go together like coffee and cream. Sure, you can do them separately (now), but you will end up finding them used over and over again in the same combination. DRY would argue "time to abstract"' no?

jashkenas commented 12 years ago

Maybe, but I'll have to see it in real world code first ;)

Perhaps because JavaScript isn't Haskell, I can't recall the last time I've ever seen a sequence of maps and flattens chained together.

rebcabin commented 12 years ago

ok. I will work on an example using the Reactive Framework -- this will be a chain of transforms like map and filter, but over a collection of observables rather than over a list in memory. I'm going to try to make something that is pretty painful without concatMap :) "I'll be back :)"

inca commented 11 years ago

+1 for flatMap. Tons of scenarios, where it can be used to achieve the "map-and-filter" in a single one-liner. Sadly that it didn't make it into the library, especially when you include lots of functions which can be expressed using another ones (like filter and reject, every and some, etc.).

acjay commented 10 years ago

+1 for flatMap. I'm emulating it in a case where I'm automatically creating additional methods for each property in a given object.

arcseldon commented 10 years ago

+1 for flatMap. as rebcabin pointed out, am using the Reactive Extensions where using flatMap becomes critical to avoiding ugly map, flatten pairs all over the place. Underscore is a great library but incomplete without this facility. So easy to implement too, jashkenas, for once you let us all down. :(

inca commented 10 years ago

Just in case someone can find it useful. Some time ago I became fond of native reduce method as a drop-in replacement for flatMap.

Assume the array of users:

var users = [
  { name: 'Alice', phones: ['123456789'] },
  { name: 'Bob', phones: ['23542341', '231421342'] },
  { name: 'Bill' }
];

You want an array of phone numbers. Here's how you would do this with flatMap (if we had one):

users.flatMap(function(user) { return user.phones });

Here's how you would do this with reduce:

users.reduce(function(phones, user) {
  return user.phones ? phones.concat(user.phones) : phones
}, []);

Yup, it's a bit longer, but OTOH it is far more universal and, just like flatMap, traverses the initial array only once.

arcseldon commented 10 years ago

inca - thank you very much for sharing that pattern. Am familiar with using reduce for this kind of purpose, and in situations where there are only a limited number of flatMap() type operations, this alternative makes a lot of sense. The reactive extensions (reactive patterns in general) often get to multiple depths of nesting where a). flatMap by just tells you what the code is doing with clarity, b). you don't get all the noise of the reduce() approach with dangling [] args etc etc. flatMap() would also keep the API consistent with the naming frequently used in other implementations hence further assisting readability. Again, thank you for posting the reduce alternative, certainly better than the map(), flatten() pairing I have in place atm.

jdalton commented 10 years ago

:-1: on this. I added it to my implementation of _.flatten and have regretted it:

 _.flatten(users, function(user) { return user.phones; });

or

_.flatten(users, 'phones');

I'm much more in favor of just doing something like _(users).chain().pluck('phones').flatten() (basically what my flatten is doing internally anyways)

arcseldon commented 10 years ago

jdalton, very interested to hear your thoughts here. Have to agree that your recommended alternative looks good. Have to ask however, why would you rather do things long hand? In your view, is it simpler, more readable etc? As we are all no doubt aware, having nested flatMaps (or some equivalent means) with a final map is a frequently used idiom, a sort of functional equivalent to doing nested forEach statements. Extending our simple example just a bit further - Imagine here that we extended Array prototype to have the relevant methods (rather than be library specific):

return interestGroups.flatMap(function (interestGroup)) {
    return interestGroup.forums.flatMap(function (forum) {
        return forum.users.flatMap(function (user) {
            return user.phones.filter(function (phone) {
                               return phone.areaCode === '001';
                         }).map(function (phone) {
                               return { id: interestGroup.id, title: forum.title, name: user.name, phone: phone.number};
                     });
                });
    });
}

Why would you rather do that with pluck and flatten separately. What is it about flatten specifically that you have regretted? Thanks for your insights.

jdalton commented 10 years ago

Have to agree that your recommended alternative looks good. Have to ask however, why would you rather do things long hand?

I added map support to _.flatten to match API like _.uniq & _.sortBy. However, it didn't really align as those other methods used the callback as the criteria to perform an action and didn't use the callback return value as part of the final result. Also mapping didn't translate to deep flattens (recursive). I'm literally calling _.map before passing the result to baseFlatten under the hood of _.flatten. This increases its dependencies (when built as modules) for functionality that is not the common case.

Underscore is great at giving devs the small bits they need to compose their own flat-maps, filter-maps, etc.

arcseldon commented 10 years ago

Thank you very much for explaining this. Clearly there are considerations I was unaware of. Am using Underscore all the time on projects, very versatile, and it was only recently I started to question the omission of flatMap. Agreed, the building blocks are all there so it isn't really any limitation as such.

inca commented 10 years ago

_.flatten should stay the way it is meant to be. flatMap is different. When you chain map and flatten, you traverse results twice (see flatten) while it is possible to do so in a single pass (also, it's actually _(_(users).pluck('phones')).flatten() or _.chain(users).pluck('phones').flatten().values()).

OTOH, when being overly selective about what to include into API and what to leave behind, you risk getting rid of about half of the methods Underscore provides (almost everything can be expressed via reduce). It's just one question the keeps bothering me: why include reject, but continue to refuse so widely spread and anticipated flatMap?

Off the topic

I began my functional programming on Scala, where reactive paradigm is very common. I can still remember how this monadic approach has impressed me and, frankly, I though that map, filter, reduce and flatMap form the very foundation of monads, from where myriads of useful methods can be implemented (I mean really useful, like findIndexOf, not like reject).

I think flatMap is just the top of the iceberg. I really miss findIndexOf, padTo, takeWhile, dropWhile, grouped, minBy, maxBy, etc. (just take a look at Scala's Seq to have a clue of what is possible on linear collections).

I don't suggest you to take the implement everything approach, but I really think that flatMap is very basic and foundational. flatMap is all about coercing null and undefined into a zero-length array, and other non-array values into a single-element arrays. If even strongly typed languages like Scala need this to be able to speak in monadic language, then I guess it would fit into JavaScript with its more relaxed typing even more smoothly (also it is way too easier to implement in JavaScript then you would with statically typed languages).

P.S. Sorry for taking your precious time.

jdalton commented 10 years ago

_.flatten should stay the way it is meant to be. flatMap is different. When you chain map and flatten, you traverse results twice (see flatten) while it is possible to do so in a single pass

Your comment about number of iterations is an implementation detail. If that is your primary concern there are libraries like lazy.js that specialize in lazy evaluation to reduce the number of iterations in a given chain of method calls.

(also, it's actually _(_(users).pluck('phones')).flatten() or _.chain(users).pluck('phones').flatten().values()).

Yes, lets nit pick. I slipped into my own implementation's style of chaining and omitted the .value() call because I figured it got enough of the point across, my bad.

It's just one question the keeps bothering me: why include reject, but continue to refuse so widely spread and anticipated flatMap?

I disagree on the widely spread and anticipated part. The _.reject method is the counter to _.filter as _.omit is the counter to _.pick.

I think flatMap is just the top of the iceberg.

Underscore makes itself easy to extend. Nothing is preventing anyone from adding their own _.mixins to Underscore. That's why the method exists. So devs can add their _.flatMap, _.filterMap, _.rejectMap, _.flatMapDeep, and on and on.

inca commented 10 years ago

@jdalton I didn't mean to offend you or anyone in here. Sorry if anything.

  1. People, flatMap is not and equivalent of .map().flatten(). As I already mentioned above, null and undefined must not be included into the results. Maybe .map().flatten().compact(), but this is three cycles for such a simple task.
  2. The “Nothing is preventing anyone” part always reads like “go and do whatever you want”. In reality nothing stops anyone from binding a method to _, or to Array.prototype, or even from implementing their own library. I guess this is how we ended up with Underscore, Lo-Dash, SugarJS, Agave.JS and myriads of home-made stuff.

    The only way to really measure if the feature is anticipated or not is to employ some kind of a poll and analyse its results. But let's take a look at what we already got: there's a bunch of people here who say: “We need flatMap” and a bunch of other people who say “flatMap can be implemented with existing stuff” (which is a rather weak argument -- or should I open another issue to trim the library off reject, omit and other methods which could be implemented using another ones?).

Anyway, I don't care as much as to continue this conversation, because it all looks like a big can of stubbornness to me (again, no offence).

michaelficarra commented 10 years ago

@jdalton: Not called for.

We're getting into a religious war here, so let's take this conversation elsewhere please.

jdalton commented 10 years ago

@inca

Maybe .map().flatten().compact(), but this is three cycles for such a simple task.

Sure, that will do too. (though underscore-contrib & I have it implemented w/o compacting w/o complaints)

The “Nothing is preventing anyone” part always reads like “go and do whatever you want”.

Underscore can't realistically include all methods so when niche/narrow use case methods can be composed from the existing ones they tend to get lumped into the "nice as a mixin" category. You mentioned combining map, flatten, & compact which is great and can be accomplished as a _.mixin to extend Underscore and its chaining syntax.

But let's take a look at what we already got: there's a bunch of people here

This issue is 2yrs old with no other requests or activity until recently with a total of 3 devs +1'ing. That's not a strong case.

or should I open another issue to trim the library off reject, omit and other methods which could be implemented using another ones

That's totally something you could do. However, there's a strong possibility the issue will be closed as wontfix due to back-compat. Again, _.reject and _.omit complement _.filter and _.pick which is a different boat than _.flatMap.

ms440 commented 10 years ago

@jdalton Yes, you're right - only 3+ devs were raising their voice about this issue. I still think that silent majority would prefer to have flatMap in lodash instead of implementing it by themselves.

There are very few primitives as fundamental for FRP style programming as flatMap. Sure, "go-away-do-it-yourself" is a legit answer/approach, but the usefulness of the framework would be higher with flatMap. I ended up adding it in my code. I believe many people did the same (instead of participating in religious war :).

I

jdalton commented 10 years ago

@ms440 It's not a religious war. Underscore and Lo-Dash promote leveraging function composition when applicable. If you don't dig it, that fine. Both make it easy to extend and customize and provide channels like underscore-contrib and lodash-contib for methods to incubate before adoption in their cores.

spion commented 9 years ago

+1 for flatMap, its not at all uncommon in my code. Needing a recursive flatten is what is uncommon. Almost any language that has FP support has it.

inca commented 9 years ago

@spion As you can see above, we had quite a debate a while ago. It seems that repo maintainers reject feature requests to keep the library small (or maybe for other good reasons). One way or another I have accepted the fact that it's not going to happen. You should, too.

Now, extending Underscore is the last thing one should do: it is almost as bad as extending Array.prototype.

If you are into small single-function modules, just grab flatMap, although it makes code a bit uglier because it doesn't chain with Underscore or Arrays. I guess, for these reasons people should really start asking ECMAscript maintainers to add useful FP stuff to Array API.

eirirlar commented 9 years ago

+1

pfultz2 commented 9 years ago

Well, flatMap is very fundamental, more so than filter and map(especially since they can be implemented using flatMap). I just don't understand the rejection of basic list comprehension.

Needing a recursive flatten is what is uncommon.

Exactly. List comprehension(ie flatMap) is much more common than recursive flatten.

inca commented 9 years ago

I'm with you, my friends :smile:

AlexGalays commented 9 years ago

flatMap is a very basic tool in one FP programmer's tool belt. It wouldn't hurt to peek at what other languages have.

QuotableWater7 commented 8 years ago

+1 to flatMap (though I've stopped caring because lodash provides it)