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 and recoverWith #41

Closed
Thomvis opened this issue May 12, 2015 · 5 comments
Closed

Add recover and recoverWith #41

Thomvis opened this issue May 12, 2015 · 5 comments

Comments

@Thomvis
Copy link
Member

Thomvis commented May 12, 2015

Would you be open to adding recover and recoverWith to Result. They're similar to map and flatMap except that they are given closures that run when the Result failed and then provide an opportunity to make it succeed. The implementation of recover would actually be identical to the ?? operation.

If you are favorable of this change, I'd like to make a PR.

For more context, see Thomvis/BrightFutures#49, where we're discussing to replace a custom Result implementation with this library and identify the functions discussed here as the biggest missing piece.

The naming comes from Scala (http://www.scala-lang.org/api/2.10.1/index.html#scala.concurrent.Future), at least that's where I got it from.

@robrix
Copy link
Contributor

robrix commented May 12, 2015

Thanks for filing this!

I’m of two minds. Against:

  • I really like reinforcing use of the extant ?? operator—overloading is very much in Swift’s idiom, and the analogy between Result’s ?? and Optional’s is clear, and supportive of general use of the language. Adding recover/recoverWith dilutes that somewhat, and so I’m a bit reluctant.

For:

  • ?? doesn’t cover recoverWith, and an ?? overload taking a function on the right hand side feels like it’s too distinct in usage from the extant overloads to be reasonably named ??, and I don’t much want to define new custom operators.
  • recover and recoverWith are pretty decent names with some precedent, and names are searchable where operators are not; for which reason (in part) we have flatMap as well as >>-.

On the balance, I think it’d make a fine addition. My only request is that the documentation for recover mention the equivalence with ??.

Thank you kindly!

@robrix
Copy link
Contributor

robrix commented May 13, 2015

Fixed by #42. I’ll tag in a moment.

@robrix robrix closed this as completed May 13, 2015
@Thomvis
Copy link
Member Author

Thomvis commented May 13, 2015

@robrix Thanks for the fast response & thanks @nvh for the PR 🌟

@nvh
Copy link
Contributor

nvh commented May 13, 2015

🎉 ❤️

@robrix
Copy link
Contributor

robrix commented May 13, 2015

You’re very welcome 😄

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

No branches or pull requests

3 participants