scala / scala-library-next

backwards-binary-compatible Scala standard library additions
Apache License 2.0
67 stars 17 forks source link

Add `IterableOps.groupFlatMap` method #135

Open ashleymercer opened 1 year ago

ashleymercer commented 1 year ago

We already have IterableOps.groupMap and groupMapReduce methods, does it make sense to add a groupFlatMap method as well?

In the simplest case this could just be an alias for groupMapReduce(f)(g)(_ ++ _) but if implemented separately it can be more efficient:

Indeed groupMapReduce already states in its doc that it exists (solely?) because it's more efficient than rolling the behaviour by hand using groupBy(f).map(...). It also somehow "feels" like this is missing from the API since we generally expect map and flatMap to come as a pair.

BalmungSan commented 1 year ago

It could be added as part of my proposal here: https://github.com/scala/scala-library-next/pull/53

ashleymercer commented 1 year ago

@BalmungSan looks like there's some overlap of ideas here, but your proposal is more complex since you're changing the target type of collection. I would expect groupFlatMap to be much simpler - I already have a draft implementation which I'm using locally - so hopefully much easier to get it accepted / merged.

For example, I'd expect to keep the existing behaviour of flatMap that you end up with the source type of collection, for example:

val as = Seq(List(1), List(2))
val bs = as.flatMap(identity)

The resulting type is bs: Seq[Int] rather than bs: List[Int], so I'd expect groupFlatMap to behave similarly (and obviously changing this for regular flatMap would be a whole separate discussion).

Of course, at some point you could propose adding groupFlatMapTo as well :)

(Also updated issue title and description to reflect the fact that this should really live on IterableOps, not on Iterable)