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

Review TryFutureExt combinators #2470

Closed
sidju opened this issue Jul 28, 2021 · 3 comments
Closed

Review TryFutureExt combinators #2470

sidju opened this issue Jul 28, 2021 · 3 comments
Labels
A-future Area: futures::future C-feature-request

Comments

@sidju
Copy link

sidju commented Jul 28, 2021

Currently there isn't a complete selection of sync/async and Result/infallible combinators in TryFutureExt.

- Result<T,E> T (auto wrapped into Result)
sync none .map_ok/.map_err
async .and_then/.or_else none

For the times when one wants to convert an Ok result into an error depending on its value or vice versa sync combinators that return Result would be nicer than using FutureExt::map and only caring about one of the cases.

Likewise when one has an async function that is infallible, such as a web handler that in the worst case renders and returns an error page to treat the same as a successful result, infallible async combinators would be very helpful. Instead of declaring returning with Ok around the return and matching the unused Error type to the TryFuture::Error.

@sidju sidju changed the title Feature request: More TryFutureExt combinators Feature request: Review TryFutureExt combinators Jul 29, 2021
@sidju
Copy link
Author

sidju commented Jul 29, 2021

Upon further consideration I have suggestions for combinator names.

map_ok and map_err have excellent clarity, they obviously map the ok/err case in the chain. To instead of implying mapping a part imply conditionality one can take inspiration from and_then/or_else, arriving at:
.and_map/.else_map
and_then and or_else are on the other hand quite verbose and diffuse to begin with. I would have preferred and_then/else_then, to clearly show their relationship. As such that is where I base this suggestion:
.then_ok/.then_err

I am not truly happy with these names, since they don't really match the meanings in Result, but they are less breaking.
Disregarding breaking my more ideal table would better match result, like this:

- Result<T,E> T (auto wrapped into Result)
sync .and_then/.or_else .map_ok/.map_err
async .async_and_then/.async_or_else .async_map_or/.async_map_err

That change would then include removing .then from FutureExt, to maintain that .then implies returning a Result.

The challenge is that futures-rs used Result as the basis of the future type itself before, meaning that these two corners of the top-post's table that are empty didn't exist. Now that the Future trait has been defined to return not only Results but any type new cases appear and trouble with them. As this means creating entirely new names I feel quite out of my depth.

@sidju
Copy link
Author

sidju commented Jul 29, 2021

The alternate solution would be to wait for async closures to hit stable, remove the option of combining with synchronous functions altogether (as people can just as well use an async closure to wrap the function or make it async) and just run with the same methods as Result (excepting .map to .map_ok).

@sidju sidju changed the title Feature request: Review TryFutureExt combinators Review TryFutureExt combinators Jul 29, 2021
@taiki-e taiki-e added the A-future Area: futures::future label Aug 6, 2021
@taiki-e taiki-e added this to the futures-0.4, futures-core-1.0 milestone Aug 7, 2021
@taiki-e
Copy link
Member

taiki-e commented Jun 18, 2023

Closing in favor of #2755.

@taiki-e taiki-e closed this as completed Jun 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-future Area: futures::future C-feature-request
Projects
None yet
Development

No branches or pull requests

2 participants