Skip to content

Commit

Permalink
Reversed order of evaluation of Applicative.ap - will reverse order o…
Browse files Browse the repository at this point in the history
…f params to ap in 1.0
  • Loading branch information
mpilquist committed Nov 2, 2016
1 parent 1edf3fd commit 38f31cd
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
2 changes: 1 addition & 1 deletion core/shared/src/main/scala/fs2/util/Monad.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ trait Monad[F[_]] extends Applicative[F] {
def flatMap[A,B](a: F[A])(f: A => F[B]): F[B]

def map[A,B](a: F[A])(f: A => B): F[B] = flatMap(a)(f andThen pure)
def ap[A,B](fa: F[A])(f: F[A => B]): F[B] = flatMap(f)(map(fa))
def ap[A,B](fa: F[A])(f: F[A => B]): F[B] = flatMap(fa)(a => map(f)(_(a)))
}

object Monad {
Expand Down
14 changes: 14 additions & 0 deletions core/shared/src/test/scala/fs2/util/TraverseSpec.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package fs2
package util

class TraverseSpec extends Fs2Spec {

"Traverse" - {
"evaluate effects in left-to-right order" in {
var acc = collection.mutable.ListBuffer[Int]()
val result = Traverse[List].traverse((1 to 5).toList)(n => Task.delay(acc += n))
result.unsafeRunSync()
acc.toList shouldBe List(1, 2, 3, 4, 5)
}
}
}

2 comments on commit 38f31cd

@pchlupacek
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to me + 1. Also think you have to fix Task.traverse before merging this, otherwise that will result to reversed order now

@pchiusano
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mpilquist I would probably leave the order of ap as is and modify traverse instead, for the reasons I gave here about it 'working out nicely' to run function's effects first.

Please sign in to comment.