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 recoverF and mapF to EitherT #1643

Closed
leandrob13 opened this issue Apr 30, 2017 · 13 comments
Closed

Add recoverF and mapF to EitherT #1643

leandrob13 opened this issue Apr 30, 2017 · 13 comments

Comments

@leandrob13
Copy link
Contributor

leandrob13 commented Apr 30, 2017

I have been dealing with use cases where there is an EitherT[Future, X, Y] and there is the need to recover from a failed future.

def someFuture(x: String): Future[Int] = Future(x.toInt)
def fe(s: String): Future[Either[String, Int]] = someFuture(s).map { x =>
  if (x == 0) Right(x)
  else Left("Error")
}

EitherT.right[Future, String, Int](someFuture("1")) //EitherT(Future(Success(Right(1))))
EitherT.right[Future, String, Int](someFuture("not an int")) //EitherT(Future(Failure(java.lang.NumberFormatException: For input string: "not an int")))
EitherT(fe("0")) //EitherT(Future(Success(Right(0))))
EitherT(fe("1")) //EitherT(Future(Success(Left(Error))))
EitherT(fe("not an int")) //EitherT(Future(Failure(java.lang.NumberFormatException: For input string: "not an int")))

If I want to recover from the future that failed and keep the EitherT context, would have to do:

EitherT(EitherT(fe("not an int")).value.recoverWith { ... })

I want to be able to just do EitherT(fe("not an int")).recoverWith { ... } obviously not only for futures but with any F that has a ApplicativeError instance.

Is it a good idea to add a recoverF and recoverFWith functions to EitherT? I have been considering adding a mapF also.

@edmundnoble
Copy link
Contributor

Sounds good to me so far. 👍 to making a PR.

@leandrob13
Copy link
Contributor Author

@edmundnoble I'm on it.

@djspiewak
Copy link
Member

I take it that the MonadError instance for EitherT[F, L, ?] where F also has a MonadError is not sufficient to implement this?

@leandrob13
Copy link
Contributor Author

leandrob13 commented Apr 30, 2017

@djspiewak It has a Monad instance, not a MonadError for F so there is no recovery for F. I just checked:

implicit def catsDataMonadErrorForEitherT[F[_], L](implicit F0: Monad[F]): MonadError[EitherT[F, L, ?], L] =
    new EitherTMonadError[F, L] { implicit val F = F0 }

private[data] trait EitherTMonadError[F[_], L] extends MonadError[EitherT[F, L, ?], L] with EitherTMonad[F, L] {
  def handleErrorWith[A](fea: EitherT[F, L, A])(f: L => EitherT[F, L, A]): EitherT[F, L, A] =
    EitherT(F.flatMap(fea.value) {
      case Left(e) => f(e).value
      case r @ Right(_) => F.pure(r)
    })
  override def handleError[A](fea: EitherT[F, L, A])(f: L => A): EitherT[F, L, A] =
    EitherT(F.flatMap(fea.value) {
      case Left(e) => F.pure(Right(f(e)))
      case r @ Right(_) => F.pure(r)
    })
  def raiseError[A](e: L): EitherT[F, L, A] = EitherT.left(F.pure(e))
  override def attempt[A](fla: EitherT[F, L, A]): EitherT[F, L, Either[L, A]] = EitherT.right(fla.value)
  override def recover[A](fla: EitherT[F, L, A])(pf: PartialFunction[L, A]): EitherT[F, L, A] =
    fla.recover(pf)
  override def recoverWith[A](fla: EitherT[F, L, A])(pf: PartialFunction[L, EitherT[F, L, A]]): EitherT[F, L, A] =
    fla.recoverWith(pf)
}

@djspiewak
Copy link
Member

Oh, yeah that's basically not what we want. :-D I guess there are actually two possible MonadError derivations for EitherT: one which derives from a MonadError[F], and the other which derives from the outer either.

@leandrob13
Copy link
Contributor Author

@djspiewak I had been looking around in code and I had some use cases for EitherT and Kleisli where there was a need to recover from F. I had a failed PR on Kleisli about this (#1601). The only catch about the solution to recover Kleislis is that you have to define a type alias for the ApplicativeError instance to get selected:
type K[A] = Kleisli[Either[String, ?], String, A]

Since that idea stuck in my mind and considering that for both cases there is no way to recover from F, maybe this proposal can point us to a proper solution. Is it worth the try then @djspiewak?

@djspiewak
Copy link
Member

@leandrob13 Did you still have to alias even with -Ypartial-unification?

@leandrob13
Copy link
Contributor Author

@djspiewak In the test that I did after the guys pointed out that alternative, yes. The point is that I wanted Kleisli to recover without the work around.

@djspiewak
Copy link
Member

Well, -Ypartial-unification isn't really considered a workaround anymore. :-) Cats has literally removed all of the machinery that would otherwise be necessary to make things work without that compiler switch.

@leandrob13
Copy link
Contributor Author

leandrob13 commented Apr 30, 2017

@djspiewak I didn't mean the -Ypartial-unification, I meant aliasing and including the ApplicativeError syntax that can enable a kleisli instance k to do k.recover { ... } . I saw the recent pr on that, tough work!

@djspiewak
Copy link
Member

Oh, I see what you mean. I think that probably stems from the fact that -Ypartial-unification unifies strictly left-to-right, but monad transformers (and MonadError/ApplicativeError) are variant to the left, requiring either right-to-left or "inside-out" unification.

@leandrob13
Copy link
Contributor Author

@djspiewak now I know why, thanks! And I forgot that by the time I did the failed kleisli pr unapply was not removed yet. I will finish my proposal for EitherT and maybe I could have another shot at Kleisli recoverF

@leandrob13
Copy link
Contributor Author

Issue addressed in #1644

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

No branches or pull requests

3 participants