scala-library-next icon indicating copy to clipboard operation
scala-library-next copied to clipboard

Add `IterableOps.groupFlatMap` method

Open ashleymercer opened this issue 3 years ago • 2 comments

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:

  • we can avoid an extra function call to the reducer "wrapper" function for each element
  • we can use a mutable buffer internally to build up the list of values

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.

ashleymercer avatar Oct 17 '22 11:10 ashleymercer

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

BalmungSan avatar Oct 17 '22 13:10 BalmungSan

@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)

ashleymercer avatar Oct 17 '22 14:10 ashleymercer