Stack-safe Coyoneda#1602
Conversation
| /** The transformer function, to be lifted into `F` by `run`. */ | ||
| val k: Pivot => A | ||
| /** The transformer functions, to be lifted into `F` by `run`. */ | ||
| val k: List[Any => Any] |
There was a problem hiding this comment.
Given its signature should this be exposed? Seems like we should hide it.
There was a problem hiding this comment.
I would say so, but the user should have the power to concatenate the lists IMO. Perhaps a scarier name?
There was a problem hiding this comment.
Never mind, that doesn't sound useful. Making it private[cats] just in case we change it in future.
|
|
||
| /** Like `lift(fa).map(_k)`. */ | ||
| def apply[F[_], A, B](fa: F[A])(k0: A => B): Aux[F, B, A] = | ||
| /** unsafe mang */ |
There was a problem hiding this comment.
Maybe a better comment than this ;)
There was a problem hiding this comment.
Omg I was removing it you fast person
edmundnoble
force-pushed
the
coyo-stack-safe
branch
from
283877a to
5e82854
Compare
Codecov Report
@@ Coverage Diff @@
## master #1602 +/- ##
==========================================
+ Coverage 93.88% 93.88% +<.01%
==========================================
Files 246 246
Lines 4088 4090 +2
Branches 154 154
==========================================
+ Hits 3838 3840 +2
Misses 250 250
Continue to review full report at Codecov.
|
edmundnoble
force-pushed
the
coyo-stack-safe
branch
from
5e82854 to
29e23ea
Compare
| /** Creates a `Coyoneda[F, A]` for any `F`, taking an `F[A]` | ||
| * and a list of [[Functor.map]]ped functions to apply later | ||
| */ | ||
| def unsafeApply[F[_], A, B](fa: F[A])(k0: List[Any => Any]): Aux[F, B, A] = |
There was a problem hiding this comment.
I don't think it (or anything with Any in it) should be public.
There was a problem hiding this comment.
My bad, I was on my way to making this private. Not because of Any, but because it fixes List.
edmundnoble
force-pushed
the
coyo-stack-safe
branch
from
29e23ea to
8d6073e
Compare
| private[cats] val k: List[Any => Any] | ||
|
|
||
| import Coyoneda.{Aux, apply} | ||
| def execute(a: Pivot): A = |
There was a problem hiding this comment.
Does this one need to be public?
There was a problem hiding this comment.
Good question: it should in my opinion be public because a) it's as safe as exposing k previously and b) k was public before, which means users have had time to create use-cases that require using the function inside a Coyo on other values.
|
I wonder if we want to just add code like: sealed trait StackSafeFn[A, B] {
def apply(a: A): B = StackSafeFn.run(this, a)
}
object StackSafeFn {
private case class FromScala[A, B](fn: A => B) extends StackSafeFn[A, B]
private case class Compose[A, B, C](first: StackSafeFn[A, B], second: StackSafeFn[B, C]) extends StackSafeFn[A, C]
private def run[A, B](fn: StackSafeFn[A, B], a: A): B = ...
}
|
|
@johnynek What we need is |
| /** The transformer function, to be lifted into `F` by `run`. */ | ||
| val k: Pivot => A | ||
| /** The transformer functions, to be lifted into `F` by `run`. */ | ||
| private[cats] val k: List[Any => Any] |
There was a problem hiding this comment.
what if we called this ks and keep def k: Pivot => A = execute _ so we don't break binary compatibility here?
There was a problem hiding this comment.
👍 Done, got it down as final def k and added a doc.
| final def toYoneda(implicit F: Functor[F]): Yoneda[F, A] = | ||
| new Yoneda[F, A] { | ||
| def apply[B](f: A => B): F[B] = F.map(fi)(k andThen f) | ||
| def apply[B](f: A => B): F[B] = F.map(fi)(execute _ andThen f) |
There was a problem hiding this comment.
if we keep k we can not change this line.
edmundnoble
force-pushed
the
coyo-stack-safe
branch
from
8d6073e to
19c6561
Compare
|
👍 |
|
merging with 3 sign-offs |
7 participants
Makes Coyoneda's
mapstack-safe, by keeping a list of functions and reversing it before applying them.Breaking change.