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

Fix short circuiting behaviour in traverse and traverseFilter #3328

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions alleycats-core/src/main/scala/alleycats/std/set.scala
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package alleycats
package std

import cats.{Applicative, Eval, Foldable, Monad, Monoid, Traverse, TraverseFilter}
import alleycats.compat.scalaVersionSpecific._
import cats.{Always, Applicative, Eval, Foldable, Monad, Monoid, Traverse, TraverseFilter}

import scala.annotation.tailrec
import compat.scalaVersionSpecific._

object set extends SetInstances

Expand Down Expand Up @@ -36,6 +36,14 @@ trait SetInstances {
override def map[A, B](fa: Set[A])(f: A => B): Set[B] = fa.map(f)
def flatMap[A, B](fa: Set[A])(f: A => Set[B]): Set[B] = fa.flatMap(f)

override def map2[A, B, Z](fa: Set[A], fb: Set[B])(f: (A, B) => Z): Set[Z] =
if (fb.isEmpty) Set.empty[Z] // do O(1) work if fb is empty
else fa.flatMap(a => fb.map(b => f(a, b))) // already O(1) if fa is empty

override def map2Eval[A, B, Z](fa: Set[A], fb: Eval[Set[B]])(f: (A, B) => Z): Eval[Set[Z]] =
if (fa.isEmpty) Eval.now(Set.empty[Z]) // no need to evaluate fb
else fb.map(fb => map2(fa, fb)(f))

def tailRecM[A, B](a: A)(f: (A) => Set[Either[A, B]]): Set[B] = {
val bldr = Set.newBuilder[B]

Expand Down Expand Up @@ -74,9 +82,9 @@ trait SetInstances {

def traverse[G[_]: Applicative, A, B](sa: Set[A])(f: A => G[B]): G[Set[B]] = {
val G = Applicative[G]
sa.foldLeft(G.pure(Set.empty[B])) { (buf, a) =>
G.map2(buf, f(a))(_ + _)
}
foldRight[A, G[Set[B]]](sa, Always(G.pure(Set.empty[B]))) { (a, lglb) =>
G.map2Eval(f(a), lglb)((b, set) => set + b)
}.value
}

override def get[A](fa: Set[A])(idx: Long): Option[A] = {
Expand Down Expand Up @@ -123,11 +131,8 @@ trait SetInstances {
val traverse: Traverse[Set] = alleyCatsSetTraverse

def traverseFilter[G[_], A, B](fa: Set[A])(f: A => G[Option[B]])(implicit G: Applicative[G]): G[Set[B]] =
fa.foldLeft(G.pure(Set.empty[B])) { (gSet, a) =>
G.map2(f(a), gSet) {
case (Some(b), set) => set + b
case (None, set) => set
}
}
traverse
.foldRight(fa, Eval.now(G.pure(Set.empty[B])))((x, xse) => G.map2Eval(f(x), xse)((i, o) => i.fold(o)(o + _)))
.value
}
}
5 changes: 4 additions & 1 deletion core/src/main/scala-2.13+/cats/instances/lazyList.scala
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,10 @@ trait LazyListInstances extends cats.kernel.instances.LazyListInstances {
def traverseFilter[G[_], A, B](
fa: LazyList[A]
)(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[LazyList[B]] =
fa.foldRight(Eval.now(G.pure(LazyList.empty[B])))((x, xse) => G.map2Eval(f(x), xse)((i, o) => i.fold(o)(_ +: o)))
traverse
.foldRight(fa, Eval.now(G.pure(LazyList.empty[B])))((x, xse) =>
G.map2Eval(f(x), xse)((i, o) => i.fold(o)(_ +: o))
)
.value

override def filterA[G[_], A](fa: LazyList[A])(f: (A) => G[Boolean])(implicit G: Applicative[G]): G[LazyList[A]] =
Expand Down
8 changes: 8 additions & 0 deletions core/src/main/scala/cats/data/Chain.scala
Original file line number Diff line number Diff line change
Expand Up @@ -774,6 +774,14 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 {
acc
}

override def map2[A, B, Z](fa: Chain[A], fb: Chain[B])(f: (A, B) => Z): Chain[Z] =
if (fb.isEmpty) nil // do O(1) work if fb is empty
else fa.flatMap(a => fb.map(b => f(a, b))) // already O(1) if fa is empty

override def map2Eval[A, B, Z](fa: Chain[A], fb: Eval[Chain[B]])(f: (A, B) => Z): Eval[Chain[Z]] =
if (fa.isEmpty) Eval.now(nil) // no need to evaluate fb
else fb.map(fb => map2(fa, fb)(f))

override def get[A](fa: Chain[A])(idx: Long): Option[A] = fa.get(idx)

def functor: Functor[Chain] = this
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/scala/cats/instances/queue.scala
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ private object QueueInstances {
override def flattenOption[A](fa: Queue[Option[A]]): Queue[A] = fa.flatten

def traverseFilter[G[_], A, B](fa: Queue[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Queue[B]] =
fa.foldRight(Eval.now(G.pure(Queue.empty[B])))((x, xse) => G.map2Eval(f(x), xse)((i, o) => i.fold(o)(_ +: o)))
traverse
.foldRight(fa, Eval.now(G.pure(Queue.empty[B])))((x, xse) => G.map2Eval(f(x), xse)((i, o) => i.fold(o)(_ +: o)))
.value

override def filterA[G[_], A](fa: Queue[A])(f: (A) => G[Boolean])(implicit G: Applicative[G]): G[Queue[A]] =
Expand Down
12 changes: 11 additions & 1 deletion core/src/main/scala/cats/instances/vector.scala
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ trait VectorInstances extends cats.kernel.instances.VectorInstances {
def flatMap[A, B](fa: Vector[A])(f: A => Vector[B]): Vector[B] =
fa.flatMap(f)

override def map2[A, B, Z](fa: Vector[A], fb: Vector[B])(f: (A, B) => Z): Vector[Z] =
if (fb.isEmpty) Vector.empty // do O(1) work if either is empty
else fa.flatMap(a => fb.map(b => f(a, b))) // already O(1) if fa is empty

override def map2Eval[A, B, Z](fa: Vector[A], fb: Eval[Vector[B]])(f: (A, B) => Z): Eval[Vector[Z]] =
if (fa.isEmpty) Eval.now(Vector.empty) // no need to evaluate fb
else fb.map(fb => map2(fa, fb)(f))

def coflatMap[A, B](fa: Vector[A])(f: Vector[A] => B): Vector[B] = {
@tailrec def loop(builder: VectorBuilder[B], as: Vector[A]): Vector[B] =
as match {
Expand Down Expand Up @@ -166,7 +174,9 @@ private[instances] trait VectorInstancesBinCompat0 {
override def flattenOption[A](fa: Vector[Option[A]]): Vector[A] = fa.flatten

def traverseFilter[G[_], A, B](fa: Vector[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Vector[B]] =
fa.foldRight(Eval.now(G.pure(Vector.empty[B])))((x, xse) => G.map2Eval(f(x), xse)((i, o) => i.fold(o)(_ +: o)))
traverse
.foldRight(fa, Eval.now(G.pure(Vector.empty[B])))((x, xse) => G.map2Eval(f(x), xse)((i, o) => i.fold(o)(_ +: o))
)
.value

override def filterA[G[_], A](fa: Vector[A])(f: (A) => G[Boolean])(implicit G: Applicative[G]): G[Vector[A]] =
Expand Down