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

MonadError#adaptError could be on ApplicativeError instead #2685

Closed
bplommer opened this issue Jan 7, 2019 · 8 comments · Fixed by #3148
Closed

MonadError#adaptError could be on ApplicativeError instead #2685

bplommer opened this issue Jan 7, 2019 · 8 comments · Fixed by #3148

Comments

@bplommer
Copy link
Contributor

bplommer commented Jan 7, 2019

MonadError#adaptError is defined as

def adaptError[A](fa: F[A])(pf: PartialFunction[E, E]): F[A] =
  flatMap(attempt(fa))(_.fold(e => raiseError(pf.applyOrElse[E, E](e, _ => e)), pure))

But it could be defined on ApplicativeError simply as

def adaptError[A](fa: F[A])(pf: PartialFunction[E, E]): F[A] =
  recoverWith(fa)(pf.andThen(raiseError))

I haven't opened a PR because this would be a breaking change.

@kailuowang
Copy link
Contributor

I think we can at least change the implementation in MonadError and add it to ApplicativeError syntax

@kailuowang kailuowang added this to the 1.6 milestone Jan 8, 2019
@bplommer
Copy link
Contributor Author

bplommer commented Jan 8, 2019

@kailuowang I'm happy to pick this up. I suppose it will need a different name in the ApplicativeError syntax to prevent ambiguity - maybe applicativeAdaptError?

@kailuowang
Copy link
Contributor

@bplommer thanks, yeah, that's a good catch. applicativeAdaptError sounds too long. how about adaptErr or modifyError with a comment on why it can't be the same as the one in MonadError for now? we shall be able to change this in 2.0.

@kailuowang kailuowang modified the milestones: 1.6.0-RC1, 1.7 Jan 22, 2019
@kailuowang kailuowang modified the milestones: 1.7, 2.1 Feb 5, 2019
@kubukoz
Copy link
Member

kubukoz commented Apr 26, 2019

Can this be closed? :)

@kailuowang
Copy link
Contributor

I still like to move this method in the type class in 2.1

@kubukoz
Copy link
Member

kubukoz commented Apr 26, 2019

On the >2.11 branch, right?

@kailuowang
Copy link
Contributor

Right, which is master.

@travisbrown
Copy link
Contributor

For what it's worth it should have been possible to fix the syntax in 2.0 by making the MonadErrorOps adaptError package private, but I just tried it for 2.1 and ran into a compiler bug: scala/bug#11787

(private or private[MonadErrorOps] avoid the bug but break bincompat.)

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

Successfully merging a pull request may close this issue.

5 participants