Add FreeT.inject helper function (#1534) #2169
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2169 +/- ##
==========================================
+ Coverage 94.66% 94.79% +0.13%
==========================================
Files 328 332 +4
Lines 5548 5960 +412
Branches 213 207 -6
==========================================
+ Hits 5252 5650 +398
- Misses 296 310 +14
Continue to review full report at Codecov.
|
| /** | ||
| * Uses the [[http://typelevel.org/cats/guidelines.html#partially-applied-type-params Partially Applied Type Params technique]] for ergonomics. | ||
| */ | ||
| private[free] final class FreeTInjectKPartiallyApplied[F[_], M[_], G[_]](val dummy: Boolean = true ) extends AnyVal { |
There was a problem hiding this comment.
Can F[_] be inferred too?
Would the name inject be a bit confusing here? What aboutliftInject or injectLift?
There was a problem hiding this comment.
As far as I understand, F[_] cannot be inferred as it's an implicit parameter of the apply method.
This work is an adaption of the snippet in the Free file:
def inject[F[_], G[_]]: FreeInjectKPartiallyApplied[F, G] =
new FreeInjectKPartiallyApplied`
private[free] final class FreeInjectKPartiallyApplied[F[_], G[_]](val dummy: Boolean = true ) extends AnyVal {
def apply[A](fa: F[A])(implicit I: InjectK[F, G]): Free[G, A] =
Free.liftF(I.inj(fa))
}
and similarly F[_] couldn't be inferred either.
I agree that liftInject or injectLift are clearer, but I would argue to keep it like this for consistency. Don't mind change it though to one of the suggested name though 👍 .
There was a problem hiding this comment.
It can infer the type with a caveat.
diff --git a/free/src/main/scala/cats/free/FreeT.scala b/free/src/main/scala/cats/free/FreeT.scala
index fba272d..aa42ba4 100644
--- a/free/src/main/scala/cats/free/FreeT.scala
+++ b/free/src/main/scala/cats/free/FreeT.scala
@@ -195,14 +195,14 @@ object FreeT extends FreeTInstances {
* This method exists to allow the `F`, `M` and `G` parameters to be
* bound independently of the `A` parameter below.
*/
- def inject[F[_], M[_], G[_]]: FreeTInjectKPartiallyApplied[F, M, G] =
+ def inject[M[_], G[_]]: FreeTInjectKPartiallyApplied[M, G] =
new FreeTInjectKPartiallyApplied
/**
* Uses the [[http://typelevel.org/cats/guidelines.html#partially-applied-type-params Partially Applied Type Params technique]] for ergonomics.
*/
- private[free] final class FreeTInjectKPartiallyApplied[F[_], M[_], G[_]](val dummy: Boolean = true ) extends AnyVal {
- def apply[A](fa: F[A])(implicit I: InjectK[F, G], m: Applicative[M]): FreeT[G, M, A] =
+ private[free] final class FreeTInjectKPartiallyApplied[M[_], G[_]](val dummy: Boolean = true ) extends AnyVal {
+ def apply[F[_], A](fa: F[A])(implicit I: InjectK[F, G], m: Applicative[M]): FreeT[G, M, A] =
FreeT.liftF[G, M, A](I.inj(fa))
}
}
diff --git a/free/src/test/scala/cats/free/FreeTSuite.scala b/free/src/test/scala/cats/free/FreeTSuite.scala
index 4f04edf..ddf18e4 100644
--- a/free/src/test/scala/cats/free/FreeTSuite.scala
+++ b/free/src/test/scala/cats/free/FreeTSuite.scala
@@ -170,8 +170,8 @@ class FreeTSuite extends CatsSuite {
(implicit I0: Test1Algebra :<: F,
I1: Test2Algebra :<: F): FreeT[F, Id, Int] = {
for {
- a <- FreeT.inject[Test1Algebra, Id, F](Test1(x, identity))
- b <- FreeT.inject[Test2Algebra, Id, F](Test2(y, identity))
+ a <- FreeT.inject[Id, F]((Test1(x, identity): Test1Algebra[Int]))
+ b <- FreeT.inject[Id, F]((Test2(y, identity): Test2Algebra[Int]))
} yield a + b
}
(res[T] foldMap eitherKInterpreter) == (x + y) should ===(true)I did have to explicitly pass the type Test1(x, identity):Test1Algebra[Int]), because scalac won't cast up from Test1[_] to Test1Algebra[_], however it wouldn't be problem if there is a helper method.
def test1[A](value: Int, f: Int => A): Test1Algebra[A] = Test1(value, f)Given that the helper method is useful in other situations ( like a simple for loop that only involves Test1Algebra) and a common pattern I still favor the version that I proposed.
I actually propose we rename this one to liftInject and add an liftInject to Free that also infers on F[_] as well. We can't deprecate the old inject but I guess we can comment it as "to be deprecated / removed in cats 2.0"
There was a problem hiding this comment.
Thanks for the feedbacks / help. I've updated to PR accordingly 👍
|
thanks very much @valydia ! |
| /** | ||
| * Uses the [[http://typelevel.org/cats/guidelines.html#partially-applied-type-params Partially Applied Type Params technique]] for ergonomics. | ||
| */ | ||
| private[free] final class FreeLiftInjectKPartiallyApplied[G[_]](val dummy: Boolean = true ) extends AnyVal { |
There was a problem hiding this comment.
I'm surprised that it's not a problem to use this as a public return type when the class is package private. It looks like this is the pattern that we are using elsewhere, though. I guess since it's a value class that gets erased it ends up not mattering?
No description provided.