Skip to content

Adding in ErrorProjection concept#24

Open
cmbaxter wants to merge 2 commits intoHubSpot:masterfrom
cmbaxter:cleanup-functional-methods
Open

Adding in ErrorProjection concept#24
cmbaxter wants to merge 2 commits intoHubSpot:masterfrom
cmbaxter:cleanup-functional-methods

Conversation

@cmbaxter
Copy link
Copy Markdown

@cmbaxter cmbaxter commented Sep 12, 2019

This borrows from the concept of a FailureProjection on the Validation type from scalaz. Adding this ErrorProjection concept allows one to map or flatMap over error cases and then force back to the main Result type.

Also made some updates and deprecations to existing methods:

  • Deprecated mapOk and flatMapOk in favor of just map and flatMap since it should be implied now that mapping and flatMapping should be for the Ok case only.
  • Hooked the pre-existing mapErr into error().map(...) instead now that that exists
  • Deprecated mapErr since it can be expressed now via error().map(...)

Copy link
Copy Markdown

@itscharlieb itscharlieb left a comment

Choose a reason for hiding this comment

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

Cool I think this is nifty, but no real strong preference for this vs flatMapOk, will leave that up to others with strong FP convictions

}

public <RERR_TYPE> ErrorProjection<SUCC_TYPE, RERR_TYPE> flatMap(Function<ERR_TYPE, Result<SUCC_TYPE, RERR_TYPE>> mapper) {
Result<SUCC_TYPE, Result<SUCC_TYPE, RERR_TYPE>> nestedResult = Results.<SUCC_TYPE, ERR_TYPE, Result<SUCC_TYPE, RERR_TYPE>>modErr(mapper).apply(result);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

could this be written more succinctly as

Result<...> nestedResult = result.match(mapper, Result::ok);

Copy link
Copy Markdown
Member

@zklapow zklapow left a comment

Choose a reason for hiding this comment

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

TBH my pref would be to simply leave this API alone. I see how error().map() could maybe read better but this seems to add a bunch of complexity (and leave us with 2 apis?) to achieve that vs just calling mapErr etc which are more discoverable? Maybe I am missing something here about why this API is nicer tho.

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