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

feat: +EitherT.liftRedeemK #4441

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

Conversation

GreyPlane
Copy link

@GreyPlane GreyPlane commented May 19, 2023

I found it is very handy for example lift from Future to EitherT but want to get rid of Throwable, just like attempt and redeem

Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

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

Thanks!

Bikeshedding: the name "redeem" bothers me a bit, because the traditional redeem takes two functions. This is closer to adaptError, but that doesn't can't the type of the error like this does. It's also similar to leftMap, but then it loses its connection to error handling. So my ideas aren't quite right either.

I think I like it, but I wonder if anyone else has a better idea for a name.

TheRealEugen
TheRealEugen previously approved these changes May 21, 2023
@TheRealEugen
Copy link

I found it is very handy for example lift from Future to EitherT but want to get rid of Throwable, just like attempt and redeem

@TheRealEugen
Copy link

Details

@GreyPlane
Copy link
Author

GreyPlane commented May 21, 2023

Thanks!

Bikeshedding: the name "redeem" bothers me a bit, because the traditional redeem takes two functions. This is closer to adaptError, but that doesn't can't the type of the error like this does. It's also similar to leftMap, but then it loses its connection to error handling. So my ideas aren't quite right either.

I think I like it, but I wonder if anyone else has a better idea for a name.

Ye, I also try to add that extra function that maps A => B, but that's still kind of different from original redeem since it only has one result type. though maybe it is still worth it since it will be more similar to the original one and keep the naming consistent.

@satorg
Copy link
Contributor

satorg commented May 21, 2023

Maybe liftRedeemErrorK? It is more verbose, but since it deals with errors only, the name could be more sounding.

Copy link
Member

@danicheg danicheg left a comment

Choose a reason for hiding this comment

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

Sorry for bringing yet another variant for naming, but I like Ross's idea of proceeding with some form of adaptError 🤷🏻

@GreyPlane
Copy link
Author

Sorry for bringing yet another variant for naming, but I like Ross's idea of proceeding with some form of adaptError 🤷🏻

I think i'd prefer liftAdaptErrorK, the difference between them looks intuitive to reason about since you're adapting Error into Either rather than abstract F.

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.

6 participants