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

Poll<T, E> = Result<Async<T>, E> vs Async<Result<T,E>> #141

Closed
Matthias247 opened this issue Sep 11, 2016 · 6 comments
Closed

Poll<T, E> = Result<Async<T>, E> vs Async<Result<T,E>> #141

Matthias247 opened this issue Sep 11, 2016 · 6 comments

Comments

@Matthias247
Copy link
Contributor

Hi,
why was the type of poll to be chosen to be Result<Async<T>,E> instead of Async<Result<T,E>>.
Imho the latter signature matches better to the typical definition of a future, which is it's either

  • not resolved
  • or resolved with either an error or a value

It also matches better the fact that the thing is a boxed (in the sense of 'monadic', not) version of the actual Result of the call.

@Stebalien
Copy link
Contributor

In one word: try!. This way, one can try!(something.poll()) (or, in when it stabilizes, something.poll()?) and then handle the non-error cases. Also, semantically, returning an error doesn't mean that the value has resolved. It could have, e.g., timed out.

Note, this could* possibly be changed in the future depending on what the final form of the Carrier trait looks like. That is, it may eventually become possible to apply the ? operator to Async<Result<T, E>>.

*I'm not a dev; I'm just speculating.

@alexcrichton
Copy link
Member

Thanks for the report @Matthias247! This actually was indeed what we used to do, but after the discussion on #108 we decided to settle on what we've got today. @Stebalien hit the nail on the head with the primary rationale being to support the use of try!.

@alexcrichton
Copy link
Member

Closing due to previous discussion on this topic.

@agausmann
Copy link

agausmann commented Aug 18, 2018

I do think this design decision should be reconsidered once the try_trait feature gets stabilized. Async<Result<T, E>> is much easier to read - either a deferred computation has terminated or it hasn't. If it has, then we check if it succeeded.

@Nemo157
Copy link
Member

Nemo157 commented Aug 18, 2018

With futures 0.3 removing the error type from the base Future type this has basically happened, TryFuture<Item = T, Error = E> is equivalent to a Future<Output = Result<T, E>> so the return type of Future::poll is Poll<Result<T, E>> (in 0.3 Poll is now what was called Async, so this is similar to 0.1's Async<Result<T, E>>).

There are also some Try implementations included for Poll now.

@agausmann
Copy link

That's also a great step forward! I'm glad to see that, and a bit ashamed I didn't see that. Looking forward to the 0.3 rewrite then.

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

5 participants