Reduce stack depth in StateT #1466
Conversation
|
looks like a flakey test (Futures), restarting. |
Current coverage is 92.23% (diff: 100%)@@ master #1466 diff @@
==========================================
Files 242 242
Lines 3619 3646 +27
Methods 3549 3577 +28
Messages 0 0
Branches 70 69 -1
==========================================
+ Hits 3337 3363 +26
- Misses 282 283 +1
Partials 0 0
|
| @@ -70,9 +77,12 @@ final class StateT[F[_], S, A](val runF: F[S => F[(S, A)]]) extends Serializable | |||
| * Like [[map]], but also allows the state (`S`) value to be modified. | |||
| */ | |||
| def transform[B](f: (S, A) => (S, B))(implicit F: Monad[F]): StateT[F, S, B] = | |||
There was a problem hiding this comment.
this only needs Functor[F] in this formulation.
| @@ -12,25 +12,32 @@ import cats.syntax.either._ | |||
| final class StateT[F[_], S, A](val runF: F[S => F[(S, A)]]) extends Serializable { | |||
|
|
|||
| def flatMap[B](fas: A => StateT[F, S, B])(implicit F: Monad[F]): StateT[F, S, B] = | |||
There was a problem hiding this comment.
this only needs FlatMap[F] in this formulation, but I didn't want to change binary compatibility for a negligible gain.
There was a problem hiding this comment.
I think that makes sense for now. Maybe add a comment to that effect? If we ever want to tighten things up later that will be handy.
| fas(a).run(s) | ||
| } | ||
| }) | ||
| } | ||
| }) | ||
|
|
||
| def flatMapF[B](faf: A => F[B])(implicit F: Monad[F]): StateT[F, S, B] = |
There was a problem hiding this comment.
only needs FlatMap[F] in the current formulation.
| } | ||
| ) | ||
| }) | ||
|
|
||
| def map[B](f: A => B)(implicit F: Monad[F]): StateT[F, S, B] = |
There was a problem hiding this comment.
if we fix transform to only use Functor[F] then this only needs Functor[F], which is fitting for map.
|
These changes seem positive to me. I think we should either document all the places where type class bounds could be tightened, or just do the change now. While breaking compatibility is a drag, better now than at 1.0. 👍 whichever way you go. |
|
Okay, I lowered everything about as much as I think you can. Nice to only need |
|
👍 (EDIT: We had a test timeout so I restarted them.) |
|
@adelbertc what do you think? |
|
LGTM 👍 |
I hit some stack overflows using traverse with StateT. I don't think this will always solve the issue, but I tried to minimize stack depth in a number of cases.