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 recover either #506

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

geirolz
Copy link
Contributor

@geirolz geirolz commented Jul 19, 2024

Add methods to AnyF to recover errors to Either where an instance of MonadError[F] is provided.

New methods:

  • recoverEither // not sure if put the as or not
  • recoverAsLeft
  • recoverAsRight

@satorg
Copy link

satorg commented Jul 19, 2024

Thank you for your contribution!

However, let me chime in and share my thoughts about some of the PR details please.

  1. MonadError does not seem necessary here. ApplicativeError should be just enough (unless I missed something out).
  2. recoverEither with all the syntax enabled seems to be a shortcut for:
    fa.recoverEither(pf) <=> fa.mapAsRight.recover(pf)
    
    I wonder is there a benefit from introducing a function that can be represented as a chain of other two existing functions without adding any new functionality of its own?
  3. recoverAsRight might seem more sophisticated but in fact it can be expressed with the same two functions but chained in the reverse order:
    fa.recoverAsRight(pf) <=> fa.recover(pf).mapAsRight
    
    But the latter requires less transformations to be applied - is it correct?

Note that I cannot find a simple replacement for recoverAsLeft. Therefore it looks like the most worthwhile addition to me (even though it could be a quite narrow use case).

@geirolz
Copy link
Contributor Author

geirolz commented Jul 20, 2024

Thank you @satorg for the detailed review!

  1. MonadError does not seem necessary here. ApplicativeError should be just enough (unless I missed something out).

You are right! Updated the PR

  1. recoverEither with all the syntax enabled seems to be a shortcut

Right, I added it for completeness mainly - the true need was just recoverAsLeft. Btw many operators implementation are just shortcut so I wouldn't care to much about it. The scope of mouse is to provide a more easily and convenient syntax

  1. recoverAsRight might seem more sophisticated but in fact it can be expressed with the same two functions but chained in the reverse order.

Didn't notice that - you are right!

recoverAsLeft is the main reason for this PR. Happens that you have an effect that contains the errors, some of them are failures for you and you want to move them in the left branch of the Either.
In this way is like an attempt but only for some cases defined by the partial function.

val ioa: IO[Unit] = ???
ioa.recoverAsLeft { 
 case ex: MyLogicalFailure => ex
}

// or when you have to lift to Either only some errors and map them basically 
// In this case the errors not mapped will raise an exception and will be treated as 500 ( internal server error ) - the others // are properly treated with different error codes 
ioa.recoverAsLeft { 
 case ex: MyLogicalFailure => HttpError(400, ex.getMessage)
}

Comment on lines +58 to +59
def recoverEither[E, L](pf: PartialFunction[E, Either[L, A]])(implicit F: ApplicativeError[F, E]): F[Either[L, A]] =
F.recover(mapAsRight[L])(pf)
Copy link
Member

Choose a reason for hiding this comment

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

When #508 is published, It can be used as a counterpart:

val fa: F[Unit] = ???

fa.attempt.leftFlatMapOrKeepIn(pf)

But if we take allocations into consideration, it's probably better to have a distinct syntax for that.

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.

3 participants