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.

Sign up for GitHub

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

Jump to bottom

replicateA_, replicateM, replicateM_ #3705

Closed
wants to merge 7 commits into from
Closed

replicateA_, replicateM, replicateM_ #3705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2127562
Add replicateA_ to Applicative
s5bug Dec 6, 2020
203ed45
Add replicateM_ to Monad, stub replicateM
s5bug Dec 6, 2020
2f9a258
Add Ops for Monad and Applicative
s5bug Dec 6, 2020
aa7694d
Implement replicateM in a way that's stack-safe
s5bug Dec 6, 2020
221ccf6
Merge remote-tracking branch 'upstream/master'
s5bug Jan 29, 2021
c695ff8
Stack-safe replicateA_ implementation
s5bug Jan 29, 2021
f8f1ef8
Fix documentation comment
s5bug Jan 29, 2021
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
2 changes: 2 additions & 0 deletions core/src/main/scala-2.x/src/main/scala/cats/syntax/MonadOps.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,6 @@ final class MonadOps[F[_], A](private val fa: F[A]) extends AnyVal {
def untilM_(p: F[Boolean])(implicit M: Monad[F]): F[Unit] = M.untilM_(fa)(p)
def iterateWhile(p: A => Boolean)(implicit M: Monad[F]): F[A] = M.iterateWhile(fa)(p)
def iterateUntil(p: A => Boolean)(implicit M: Monad[F]): F[A] = M.iterateUntil(fa)(p)
def replicateM(n: Int)(implicit M: Monad[F]): F[List[A]] = M.replicateM(n, fa)
def replicateM_(n: Int)(implicit M: Monad[F]): F[Unit] = M.replicateM_(n, fa)
}
2 changes: 2 additions & 0 deletions core/src/main/scala-3.x/src/main/scala/cats/syntax/MonadOps.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,6 @@ final class MonadOps[F[_], A](private val fa: F[A]) extends AnyVal {
def untilM_(using M: Monad[F])(p: F[Boolean]): F[Unit] = M.untilM_(fa)(p)
def iterateWhile(using M: Monad[F])(p: A => Boolean): F[A] = M.iterateWhile(fa)(p)
def iterateUntil(using M: Monad[F])(p: A => Boolean): F[A] = M.iterateUntil(fa)(p)
def replicateM(using M: Monad[F])(n: Int): F[List[A]] = M.replicateM(n, fa)
def replicateM_(using M: Monad[F])(n: Int): F[Unit] = M.replicateM_(n, fa)
}
26 changes: 26 additions & 0 deletions core/src/main/scala/cats/Applicative.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,32 @@ import scala.annotation.implicitNotFound
def replicateA[A](n: Int, fa: F[A]): F[List[A]] =
Traverse[List].sequence(List.fill(n)(fa))(this)

/**
* Given `fa` and `n`, apply `fa` `n` times but discard the results.
*
* Example:
* {{{
* scala> import cats.data.State
*
* scala> type Counter[A] = State[Int, A]
* scala> val increment: Counter[Unit] = State.modify(_ + 1)
* scala> val get: Counter[Int] = State.get
* scala> val increment5AndGet: Counter[Int] = Applicative[Counter].productR(
* | Applicative[Counter].replicateA_(5, increment),
* | get
* | )
* scala> increment5AndGet.run(0).value
* res0: (Int, Int) = (5,5)
* }}}
*/
def replicateA_[A](n: Int, fa: F[A]): F[Unit] =
def go(x: Int, step: F[Unit]): F[Unit] = {
if(x == 0) step
else go(x - 1, this.productR(fa, step))
}
go(n, this.pure(()))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: pure(()) is the same as unit.

nit: if we keep this implementation, I'd like to add the tailrec annotation.

instead of using fa can we do make: val fvoid = void(fa) outside of def go, and use that?

I still want to express this function in a stack safe way if we can.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking about adding @tailrec but didn't know if it was used elsewhere. Will do.

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Stray closing brace:

Suggested change
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I'm missing an opening brace. I don't know how I didn't catch that.


/**
* Compose an `Applicative[F]` and an `Applicative[G]` into an
* `Applicative[λ[α => F[G[α]]]]`.
Expand Down
9 changes: 9 additions & 0 deletions core/src/main/scala/cats/Monad.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,15 @@ import scala.annotation.implicitNotFound

tailRecM(branches.toList)(step)
}

def replicateM[A](n: Int, fa: F[A]): F[List[A]] =
Copy link
Contributor

Choose a reason for hiding this comment

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

for any lawful monad, these are exactly the same as replicateA aren't they? I am opposed to adding a function that is only different when the monad is not lawful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but it does not have the overhead of traverseViaChain/Eval/etc., if that would make a difference (would it?)

Copy link
Contributor

Choose a reason for hiding this comment

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

People can override for efficiency but my opinion is that we should not have duplicative functions that are equivalent in general.

Secondly, generally I think we should not trade stack safety for performance since lack of stack safety makes the library randomly fail when sizes get a bit larger.

I think we should have it be as fast as possible that is also stack safe, IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Should I remove replicateM or just alias it?

Should I override replicateM with this version in StackSafeMonad?

if (n == 0) this.pure(Nil)
else this.flatMap(fa)(head => this.flatMap(replicateM(n - 1, fa))(tail => head :: tail))

def replicateM_[A](n: Int, fa: F[A]): F[Unit] =
if (n == 0) this.pure(())
else this.flatMap(fa)(_ => replicateM_(n - 1, fa))

}

object Monad {
Expand Down
1 change: 1 addition & 0 deletions core/src/main/scala/cats/syntax/applicative.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ final class ApplicativeIdOps[A](private val a: A) extends AnyVal {

final class ApplicativeOps[F[_], A](private val fa: F[A]) extends AnyVal {
def replicateA(n: Int)(implicit F: Applicative[F]): F[List[A]] = F.replicateA(n, fa)
def replicateA_(n: Int)(implicit F: Applicative[F]): F[Unit] = F.replicateA_(n, fa)
def unlessA(cond: Boolean)(implicit F: Applicative[F]): F[Unit] = F.unlessA(cond)(fa)
def whenA(cond: Boolean)(implicit F: Applicative[F]): F[Unit] = F.whenA(cond)(fa)
}