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.

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 FreeT.inject helper function (#1534) #2169

Merged
merged 3 commits into from
Mar 16, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
19 changes: 19 additions & 0 deletions free/src/main/scala/cats/free/FreeT.scala
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,25 @@ object FreeT extends FreeTInstances {

def foldMap[S[_], M[_]: Monad](fk: FunctionK[S, M]): FunctionK[FreeT[S, M, ?], M] =
λ[FunctionK[FreeT[S, M, ?], M]](f => f.foldMap(fk))

/**
* This method is used to defer the application of an InjectK[F, G]
* instance. The actual work happens in
* `FreeTInjectKPartiallyApplied#apply`.
*
* 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] =
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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can F[_] be inferred too?
Would the name inject be a bit confusing here? What aboutliftInject or injectLift?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 👍 .

Copy link
Contributor

Choose a reason for hiding this comment

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

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"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedbacks / help. I've updated to PR accordingly 👍

def apply[A](fa: F[A])(implicit I: InjectK[F, G], m: Applicative[M]): FreeT[G, M, A] =
FreeT.liftF[G, M, A](I.inj(fa))
}
}

private[free] sealed abstract class FreeTInstances extends FreeTInstances0 {
Expand Down
62 changes: 62 additions & 0 deletions free/src/test/scala/cats/free/FreeTSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,68 @@ class FreeTSuite extends CatsSuite {
}
}

sealed trait Test1Algebra[A]

case class Test1[A](value : Int, f: Int => A) extends Test1Algebra[A]

object Test1Algebra {
implicit def test1AlgebraAFunctor: Functor[Test1Algebra] =
new Functor[Test1Algebra] {
def map[A, B](a: Test1Algebra[A])(f: A => B): Test1Algebra[B] = a match {
case Test1(k, h) => Test1(k, x => f(h(x)))
}
}

implicit def test1AlgebraArbitrary[A](implicit seqArb: Arbitrary[Int], intAArb : Arbitrary[Int => A]): Arbitrary[Test1Algebra[A]] =
Arbitrary(for {s <- seqArb.arbitrary; f <- intAArb.arbitrary} yield Test1(s, f))
}

sealed trait Test2Algebra[A]

case class Test2[A](value : Int, f: Int => A) extends Test2Algebra[A]

object Test2Algebra {
implicit def test2AlgebraAFunctor: Functor[Test2Algebra] =
new Functor[Test2Algebra] {
def map[A, B](a: Test2Algebra[A])(f: A => B): Test2Algebra[B] = a match {
case Test2(k, h) => Test2(k, x => f(h(x)))
}
}

implicit def test2AlgebraArbitrary[A](implicit seqArb: Arbitrary[Int], intAArb : Arbitrary[Int => A]): Arbitrary[Test2Algebra[A]] =
Arbitrary(for {s <- seqArb.arbitrary; f <- intAArb.arbitrary} yield Test2(s, f))
}

type T[A] = EitherK[Test1Algebra, Test2Algebra, A]

object Test1Interpreter extends FunctionK[Test1Algebra,Id] {
override def apply[A](fa: Test1Algebra[A]): Id[A] = fa match {
case Test1(k, h) => h(k)
}
}

object Test2Interpreter extends FunctionK[Test2Algebra,Id] {
override def apply[A](fa: Test2Algebra[A]): Id[A] = fa match {
case Test2(k, h) => h(k)
}
}

val eitherKInterpreter: FunctionK[T,Id] = Test1Interpreter or Test2Interpreter

test(".inject") {
forAll { (x: Int, y: Int) =>
def res[F[_]]
(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))
} yield a + b
}
(res[T] foldMap eitherKInterpreter) == (x + y) should ===(true)
}
}

test("== should not return true for unequal instances") {
val a = FreeT.pure[List, Option, Int](1).flatMap(x => FreeT.pure(2))
val b = FreeT.pure[List, Option, Int](3).flatMap(x => FreeT.pure(4))
Expand Down