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

Recover Kleisli from F[_] errors #1601

Closed
wants to merge 3 commits into from

Conversation

leandrob13
Copy link
Contributor

Added functions for recovering from errors from F[_].

The functions recover, recoverWith and recoverWithF need an ApplicativeError instance for F[_].

val k = Kleisli[Either[String, ?], Int, Int] { i =>
  if (i < 0) Left("negative") else Right(i)
}
val r = k.recover[String] { case s => 0 }
val rw = k.recoverWith[String] { case s => Kleisli.lift[Either[String, ?], Int, Int](Right(0)) }
val rwf = k.recoverWithF[String] { case s => Right(0) }
val res = (r.run(-2), rw.run(-2), rwf.run(-2)) // should be (Right(0), Right(0), Right(0))

@kailuowang
Copy link
Contributor

I might missed something but the ApplicativeError instance for Kleisli should've provided these, right?

@leandrob13
Copy link
Contributor Author

leandrob13 commented Apr 10, 2017

@kailuowang you are right. I rewrote it like this:

def recover[E](pf: PartialFunction[E, B])(implicit aek: ApplicativeError[Kleisli[F, A, ?], E]): Kleisli[F, A, B] =
    aek.recover[B](this)(pf)

That should do it. I'll fix the other two.

@peterneyens
Copy link
Collaborator

peterneyens commented Apr 10, 2017

I agree with @kailuowang. You get recover and recoverWith from the ApplicativeError syntax.

You might need to use a Scala version which supports -Ypartial-unification, use the SI 2712 compiler plugin or use a type alias like type K[A] = Kleisli[Either[String, ?], String, A] to be able to get the syntax.

Merging #1583 will definitely help in this case as well.

@codecov-io
Copy link

codecov-io commented Apr 10, 2017

Codecov Report

Merging #1601 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1601      +/-   ##
=========================================
+ Coverage   93.09%   93.1%   +<.01%     
=========================================
  Files         250     250              
  Lines        3983    3986       +3     
  Branches      130     136       +6     
=========================================
+ Hits         3708    3711       +3     
  Misses        275     275
Impacted Files Coverage Δ
core/src/main/scala/cats/data/Kleisli.scala 92.85% <100%> (+0.26%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f73752c...4443f40. Read the comment docs.

@leandrob13
Copy link
Contributor Author

@kailuowang @peterneyens made the changes, didn't have problems running those tests. Just had to add the implicit instances for Kleisli with either and Kleisli with validated.

@kailuowang
Copy link
Contributor

@leandrob13 what we are saying is that the methods you added are redundant, they are already available through ApplicativeError syntax. Did you try enable this sbt plugin (https://github.com/fiadliel/sbt-partial-unification) and see if you can get those methods out of the box (cats 0.9.0) already?

@leandrob13
Copy link
Contributor Author

leandrob13 commented Apr 10, 2017

@kailuowang I just thought that they could be convenient. If you would want to recover a Kleisli you would have to invoke:

ApplicativeError[Kleisli[Either[String, ?], Int, ?], String].recover(k) { ... }

Am I right? I just thought it would be useful to do add a function that does this directly on Kleisli.

k.recover { ... }

Right now I am setting up the test for a use case that may have issues with the partial unification, I will try the plugin you suggest.

@kailuowang
Copy link
Contributor

@leandrob13, you don't have to write ApplicativeError[Kleisli[Either[String, ?], Int, ?], String].recover(k) { ... }

val k = Kleisli((i:Int) => if(i == 0) Left(1) else Right("big") ) 

val res13 = k.recover{ case 1 => "haha" } 

res13.run(3) 
// res14: Either[Int, String] = Right("big")

res13.run(0) 
// res16: Either[Int, String] = Right("haha")

For this to compile, you need partial-unification enabled. But usually the methods on typeclasses are "injected" directly onto the instance through the typeclass's syntax implicit conversion.

@leandrob13
Copy link
Contributor Author

@kailuowang I had no idea you could do that. Thanks, this is going to be of great help! I will close the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants