Add capture/release for results in package:async

Add capture/release for results in package:async

R=floitsch@google.com

Committed: https://code.google.com/p/dart/source/detail?r=36548

[Lasse Reichstein Nielsen, 2014-04-29 08:08:52]

RFC: Should we have something like this?
Should it be named differently (I prefer Reflect/Reifly, but that's probably too
confusing)?
Should it be in dart:async instead of package:async. 
It would allow having Future<T>.materialize -> Future<Result<T>> (but no good
alternative for the reverse).

[Anders Johnsen, 2014-04-29 08:23:01]

On 2014/04/29 08:08:52, Lasse Reichstein Nielsen wrote:
> RFC: Should we have something like this?

I like it :)

> Should it be named differently (I prefer Reflect/Reifly, but that's probably
too
> confusing)?

Dunno, materialize is quite a mouthful, but it's not bad.

> Should it be in dart:async instead of package:async. 

I don't think so. dart:async is very polluted as is.

> It would allow having Future<T>.materialize -> Future<Result<T>> (but no good
> alternative for the reverse).

I don't think we should put anything more onto the Future, unless we have a
_really_ good use case.

[Sean Eagan, 2014-04-30 18:05:55]

On 2014/04/30 18:00:46, Sean Eagan wrote:

Result could be a synchronous concept as well:

class Result<T> {

  factory Result(T f()) {
    try {
      return new ValueResult<T>(f());
    } catch(e, s) {
      return new ErrorResult<T>(e, s);
    }
  }

  //...

}

and could reuse "result" and avoid a new term like "materialize":

ResultFuture
FromResultFuture
ResultStream
FromResultStream

or similar.

[Lasse Reichstein Nielsen, 2014-04-30 19:40:08]

On 2014/04/30 18:05:55, Sean Eagan wrote:
> On 2014/04/30 18:00:46, Sean Eagan wrote:
> 
> Result could be a synchronous concept as well:

If we did that, it should have its own package (package:result or something).

> 
> class Result<T> {
> 
>   factory Result(T f()) {
>     try {
>       return new ValueResult<T>(f());
>     } catch(e, s) {
>       return new ErrorResult<T>(e, s);
>     }
>   }

That looks like a synchronous version of Future (is that a "Now"?)

> 
>   //...
> 
> }
> 
> and could reuse "result" and avoid a new term like "materialize":

I don't particularly like "materialize" - it's too vague.

> 
> ResultFuture
> FromResultFuture
> ResultStream
> FromResultStream
> 
> or similar.

That describes the result type, but not really the operation that creates it.
I want a verb for the transformation.

[Lasse Reichstein Nielsen, 2014-04-30 19:40:09]

On 2014/04/30 18:05:55, Sean Eagan wrote:
> On 2014/04/30 18:00:46, Sean Eagan wrote:
> 
> Result could be a synchronous concept as well:

If we did that, it should have its own package (package:result or something).

> 
> class Result<T> {
> 
>   factory Result(T f()) {
>     try {
>       return new ValueResult<T>(f());
>     } catch(e, s) {
>       return new ErrorResult<T>(e, s);
>     }
>   }

That looks like a synchronous version of Future (is that a "Now"?)

> 
>   //...
> 
> }
> 
> and could reuse "result" and avoid a new term like "materialize":

I don't particularly like "materialize" - it's too vague.

> 
> ResultFuture
> FromResultFuture
> ResultStream
> FromResultStream
> 
> or similar.

That describes the result type, but not really the operation that creates it.
I want a verb for the transformation.

[Sean Eagan, 2014-05-07 14:23:46]

On 2014/04/30 19:40:08, Lasse Reichstein Nielsen wrote:
> On 2014/04/30 18:05:55, Sean Eagan wrote:
> > On 2014/04/30 18:00:46, Sean Eagan wrote:
> > 
> > Result could be a synchronous concept as well:
> 
> If we did that, it should have its own package (package:result or something).

Yeah, I don't really have any use cases for a sync result, I just wanted to
point out that it conceptually makes sense.

> 
> > 
> > class Result<T> {
> > 
> >   factory Result(T f()) {
> >     try {
> >       return new ValueResult<T>(f());
> >     } catch(e, s) {
> >       return new ErrorResult<T>(e, s);
> >     }
> >   }
> 
> That looks like a synchronous version of Future (is that a "Now"?)

I was thinking of Result as the result of a computation, regardless of whether
it was completed synchronously or asynchronously.

> I want a verb for the transformation.

Maybe putting them in the Result namespace would help clarify.  How about
"capture" / "release":

class Result {

  //...

  static Future<Result> capture(Future f);
  static Future release(Future<Result> f);
  static Stream<Result> captureStream(Stream s);
  static Stream releaseStream(Stream<Result> s);

}

[floitsch, 2014-05-07 14:30:54]

LGTM.
definitely in the package.

[Lasse Reichstein Nielsen, 2014-05-20 08:03:25]

On 2014/05/07 14:23:46, Sean Eagan wrote:
> 
> Maybe putting them in the Result namespace would help clarify.  How about
> "capture" / "release":
> 
> class Result {
> 
>   //...
> 
>   static Future<Result> capture(Future f);
>   static Future release(Future<Result> f);
>   static Stream<Result> captureStream(Stream s);
>   static Stream releaseStream(Stream<Result> s);
> 
> }

I like that.

[Lasse Reichstein Nielsen, 2014-05-20 08:32:28]

Now, I'm getting doubts about this code.
The Result class looks suspiciously like a Future.
Is this becoming the "completed future with direct access to the value" that we
removed a long time ago?
At least it is explicit.

If we didn't disallow Future<Future>, would that have been the correct approach
instead?

[floitsch, 2014-05-20 14:36:20]

On 2014/05/20 08:32:28, Lasse Reichstein Nielsen wrote:
> Now, I'm getting doubts about this code.
> The Result class looks suspiciously like a Future.
> Is this becoming the "completed future with direct access to the value" that
we
> removed a long time ago?
> At least it is explicit.
> 
> If we didn't disallow Future<Future>, would that have been the correct
approach
> instead?

My use-case for this class was to send errors in the value lane.

[Lasse Reichstein Nielsen, 2014-05-21 10:13:49]

Ok, then I'll stand by the current version. PTAL and see if it still lgty after
the renamings.

[floitsch, 2014-05-22 12:15:21]

LGTM with tests.

https://codereview.chromium.org/255123002/diff/40001/pkg/async/lib/async.dart
File pkg/async/lib/async.dart (right):

https://codereview.chromium.org/255123002/diff/40001/pkg/async/lib/async.dart...
pkg/async/lib/async.dart:8: export "materialize.dart";
I think we should start thinking of having different libraries in the async
package.
I would like the async-package to become a collection of useful libraries (in a
well organized way).

https://codereview.chromium.org/255123002/diff/40001/pkg/async/lib/materializ...
File pkg/async/lib/materialize.dart (right):

https://codereview.chromium.org/255123002/diff/40001/pkg/async/lib/materializ...
pkg/async/lib/materialize.dart:67: * Converts the [Result] value of the given
[future] to a result or error
to a value or error completion.

[Lasse Reichstein Nielsen, 2014-05-23 07:14:24]

https://codereview.chromium.org/255123002/diff/40001/pkg/async/lib/async.dart
File pkg/async/lib/async.dart (right):

https://codereview.chromium.org/255123002/diff/40001/pkg/async/lib/async.dart...
pkg/async/lib/async.dart:8: export "materialize.dart";
How would that differ from what we are currently doing?
The async library itself is just exporting all the different libraries in the
async package.

https://codereview.chromium.org/255123002/diff/40001/pkg/async/lib/materializ...
File pkg/async/lib/materialize.dart (right):

https://codereview.chromium.org/255123002/diff/40001/pkg/async/lib/materializ...
pkg/async/lib/materialize.dart:67: * Converts the [Result] value of the given
[future] to a result or error
On 2014/05/22 12:15:21, floitsch wrote:
> to a value or error completion.

Done.

[Lasse Reichstein Nielsen, 2014-05-23 07:14:51]

I'll rename the library from "materialize" to "result".

[Lasse Reichstein Nielsen, 2014-05-23 08:41:19]

Committed patchset #5 manually as r36548 (presubmit successful).