Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make default reduceMapM lazy if reduceRightTo is lazy #3167

Merged

Conversation

travisbrown
Copy link
Contributor

@travisbrown travisbrown commented Nov 20, 2019

Currently foldMapM and foldMapA will be lazy if the operations they're built on are sufficiently lazy, but reduceMapM isn't.

Take the following stand-alone non-empty stream implementation (I'm not using NonEmptyStream because Stream's reduceRightToOption isn't lazy enough, which should probably be a separate issue):

import cats.{Eval, Reducible}

case class NES[A](h: A, t: Stream[A]) { def toStream: Stream[A] = h #:: t }

object NES {
  implicit val nesReducible: Reducible[NES] = new Reducible[NES] {
    def foldLeft[A, B](fa: NES[A], b: B)(f: (B, A) => B): B = fa.toStream.foldLeft(b)(f)
    def foldRight[A, B](fa: NES[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] =
      fa match {
        case NES(h, Stream()) => f(h, lb)
        case NES(h, th #:: tt) => f(h, Eval.defer(foldRight(NES(th, tt), lb)(f)))
      }
    def reduceLeftTo[A, B](fa: NES[A])(f: A => B)(g: (B, A) => B): B =
      fa.t.foldLeft(f(fa.h))(g)
    def reduceRightTo[A, B](fa: NES[A])(f: A => B)(g: (A, Eval[B]) => Eval[B]): Eval[B] =
      fa match {
        case NES(h, Stream()) => Eval.now(f(h))
        case NES(h, th #:: tt) => g(h, Eval.defer(reduceRightTo(NES(th, tt))(f)(g)))
      }
  }
}

Compare foldMapM, foldMapA, and reduceMapM:

scala> import cats.implicits._
import cats.implicits._

scala> val xs = NES(0, Stream.from(1))
xs: NES[Int] = NES(0,Stream(1, ?))

scala> xs.foldMapM(i => if (i < 10) Right(i) else Left(i))
res0: scala.util.Either[Int,Int] = Left(10)

scala> xs.foldMapA(i => if (i < 10) Right(i) else Left(i))
res1: scala.util.Either[Int,Int] = Left(10)

scala> xs.reduceMapM(i => if (i < 10) Right(i) else Left(i)) // hangs forever

[warn] Canceling execution...
[warn] Run canceled.

With the implementation here it works fine:

scala> xs.reduceMapM(i => if (i < 10) Right(i) else Left(i))
res0: scala.util.Either[Int,Int] = Left(10)

It's worth noting that this new implementation doesn't require FlatMap, which is one of the things #3150 is addressing via a new reduceMapA. If someone has a lazy implementation using the FlatMap that would be more efficient (similar to foldMapM vs. foldMapA), I'd be happy to use it, but I couldn't come up with one.

@travisbrown travisbrown mentioned this pull request Nov 20, 2019
@travisbrown travisbrown added this to the 2.1.0-RC2 milestone Nov 20, 2019
@travisbrown travisbrown changed the title Make default reduceMapM short-circuit if reduceRightTo is lazy enough Make default reduceMapM lazy if reduceRightTo is lazy Nov 21, 2019
@rossabaker rossabaker merged commit ed52874 into typelevel:master Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants