-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add state method to MonadState #1651
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a) apply
on a trait is a recipe for name collisions. Call it state
maybe, to be consistent with reader
on MonadReader
?
b) It's not consistent to have this on MonadState
and have no method from Either
on MonadError
.
@edmundnoble |
To clarify and correct myself, in |
@edmundnoble done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@@ -17,6 +17,9 @@ package cats | |||
* }}} | |||
*/ | |||
trait MonadState[F[_], S] extends Monad[F] { | |||
def state[A](f: S => (S, A)): F[A] = | |||
flatMap(get)(s => f(s) match { case (s, a) => inspect(_ => a)}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this use set
to set the S
resulting from f(s)
? As is this will just return the initial S
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peterneyens Right. Forgot to update the state ( Fixed this!
We might want to add some doctests. |
@kailuowang done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.