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 Result Extraction Helpers #286

Merged
merged 5 commits into from
Jan 8, 2021

Conversation

rolandpeelen
Copy link

@rolandpeelen rolandpeelen commented Jan 8, 2021

As discussed in: #285

This PR adds the following three functions:

getOrElseBy: ('e => 'a, result('a, 'e)) => 'a
getErrorOrElse: ('e, result('a, 'e)) => 'e)
getErrorOrElseBy: ('a => 'e, result('a, 'e)) => 'e

Since these are a bit niche, the examples are a bit niche as well. Given the rest of the examples they might be a bit verbose. Please let me know if that's the case, then I'll try and make it less so.

The PR can be reviewed commit-by-commit.

@rolandpeelen rolandpeelen marked this pull request as ready for review January 8, 2021 09:59
Result.getErrorOrElse("Handled", Ok({id, payload})) == "Handled";
]}
*/
let getErrorOrElse: 'e 'e. ('e, t('a, 'e)) => 'e =
Copy link
Member

Choose a reason for hiding this comment

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

Did you want the universal types to be 'a 'e. here?

Copy link
Author

Choose a reason for hiding this comment

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

Good catch! not sure how those crept in. Fixed in next commit :)

src/Relude_Result.re Outdated Show resolved Hide resolved
Copy link
Member

@andywhite37 andywhite37 left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for adding these. I'll give @mlms13 a chance to look, but I'm good to merge

@mlms13
Copy link
Member

mlms13 commented Jan 8, 2021

This looks great to me! Thanks for adding tests and all the doc comments as well!.

@mlms13 mlms13 merged commit f939b42 into reazen:master Jan 8, 2021
@rolandpeelen
Copy link
Author

No worries, thank you guys for putting in all the effort to build and maintain this!

@andywhite37
Copy link
Member

I can push out a new release this evening

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