Add a bunch of delegates for dart:async types.

Add a bunch of delegates for dart:async types.

A number of these are wrappers I've needed in the past or know I'll need
in the future; some are just for good measure.

R=lrn@google.com

Committed: https://github.com/dart-lang/async/commit/8d5716a22540bb8e790c3999fded524ac7693f88

[nweiz, 2015-07-01 19:30:25]

nweiz@google.com changed reviewers:
+ lrn@google.com

[Lasse Reichstein Nielsen, 2015-07-02 09:38:39]

LGTM except StreamController.

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/event_sink...
File lib/src/delegate/event_sink.dart (right):

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/event_sink...
lib/src/delegate/event_sink.dart:18: : _sink = sink;
Could be on previous line.

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/future.dart
File lib/src/delegate/future.dart (right):

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/future.dar...
lib/src/delegate/future.dart:12: final Future<T> _future;
Consider making this a `Future` (without the <T>).
One use of wrapping can be to change the type parameter, for example if someone
gives you a Future<Fruit> but you know it's going to be an apple and you want to
pass it to something requiring a Future<Apple>.

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/sink.dart
File lib/src/delegate/sink.dart (right):

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/sink.dart#...
lib/src/delegate/sink.dart:11: class DelegatingSink<T> implements Sink<T> {
Technically, Sink is not an async interface, it is from dart:core.

Not a problem, just an observation - Users may not look here for the wrapper.

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/stream_con...
File lib/src/delegate/stream_controller.dart (right):

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/stream_con...
lib/src/delegate/stream_controller.dart:11: /// Simple delegating wrapper around
an [StreamController].
I'm not sure I'd consider StreamController an interface at the same level as the
other ones here. It's more like an implementation class. It's "interface" is
StreamSink, and then it has some convenience methods on top.

For the same reason, I don't think I have made any methods expecting a
StreamController as argument - they always expect a StreamSink, which is the
more general type.

If we treat it as an interface, then other classes might start to implement it,
and I wouldn't encourage that.

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/stream_sin...
File lib/src/delegate/stream_sink.dart (right):

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/stream_sin...
lib/src/delegate/stream_sink.dart:13: class DelegatingStreamSink<T> implements
StreamSink<T> {
If this was in the same package, you could extend DelegatingStreamConsumer
(since a StreamSink is-a StreamConsumer and an EventSink).
Not sure it's worth a reorganization of the code to reuse two methods, though.

[nweiz, 2015-07-06 20:25:02]

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/event_sink...
File lib/src/delegate/event_sink.dart (right):

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/event_sink...
lib/src/delegate/event_sink.dart:18: : _sink = sink;
On 2015/07/02 09:38:39, Lasse Reichstein Nielsen wrote:
> Could be on previous line.

Done.

[nweiz, 2015-07-06 20:40:08]

Code review changes

[nweiz, 2015-07-06 20:40:30]

Not landing yet because I want to get your opinion on StreamController again.

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/future.dart
File lib/src/delegate/future.dart (right):

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/future.dar...
lib/src/delegate/future.dart:12: final Future<T> _future;
On 2015/07/02 09:38:39, Lasse Reichstein Nielsen wrote:
> Consider making this a `Future` (without the <T>).
> One use of wrapping can be to change the type parameter, for example if
someone
> gives you a Future<Fruit> but you know it's going to be an apple and you want
to
> pass it to something requiring a Future<Apple>.

Done.

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/sink.dart
File lib/src/delegate/sink.dart (right):

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/sink.dart#...
lib/src/delegate/sink.dart:11: class DelegatingSink<T> implements Sink<T> {
On 2015/07/02 09:38:39, Lasse Reichstein Nielsen wrote:
> Technically, Sink is not an async interface, it is from dart:core.
> 
> Not a problem, just an observation - Users may not look here for the wrapper.

Yeah, that's true, but there isn't really a better place for it. It's also
probably the case that anything they actually wrap with this will be an async
class.

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/stream_con...
File lib/src/delegate/stream_controller.dart (right):

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/stream_con...
lib/src/delegate/stream_controller.dart:11: /// Simple delegating wrapper around
an [StreamController].
On 2015/07/02 09:38:39, Lasse Reichstein Nielsen wrote:
> I'm not sure I'd consider StreamController an interface at the same level as
the
> other ones here. It's more like an implementation class. It's "interface" is
> StreamSink, and then it has some convenience methods on top.
> 
> For the same reason, I don't think I have made any methods expecting a
> StreamController as argument - they always expect a StreamSink, which is the
> more general type.
> 
> If we treat it as an interface, then other classes might start to implement
it,
> and I wouldn't encourage that.

I don't think of this as a way for otherwise non-controllery classes to
implement StreamController as much as a way for people to implement
StreamControllers with custom functionality. For example, I plan to create a
ForwardingStreamController class that controls a stream while also forwarding
events from another stream, so you can do things like forward a stream but close
it at your discretion.

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/stream_sin...
File lib/src/delegate/stream_sink.dart (right):

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/stream_sin...
lib/src/delegate/stream_sink.dart:13: class DelegatingStreamSink<T> implements
StreamSink<T> {
On 2015/07/02 09:38:39, Lasse Reichstein Nielsen wrote:
> If this was in the same package, you could extend DelegatingStreamConsumer
> (since a StreamSink is-a StreamConsumer and an EventSink).
> Not sure it's worth a reorganization of the code to reuse two methods, though.

Yeah, I thought about doing that, but it seemed more complex than it was worth.

[Lasse Reichstein Nielsen, 2015-07-07 10:32:19]

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/stream_con...
File lib/src/delegate/stream_controller.dart (right):

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/stream_con...
lib/src/delegate/stream_controller.dart:11: /// Simple delegating wrapper around
an [StreamController].
I'm not sure I would encourage an extended StreamController for something like
that. It's still using StreamController as if it is a useful interface, and not
just a hodge-podge of functionality thrown together to solve a single problem -
interacting with a stream subscription.

I'd rather either have a new specialized API for your forwarding stream class
with just the methods that makes sense for that, or something that exposes a
StreamController (or StreamSink) rather than *being* one ("favor composition
over inheritance", etc.). 

I also think it was a mistake to make StreamController be a StreamSink to being
with - it should have had an "asStreamSink()" method that returned a wrapper,
keeping the plain interface simpler.

Basically, some classes are intended as interfaces and others are not, and I
worry about blurring the lines. StreamController is in the not-interface group,
which means that code should generally not use it as an interface.
If it matters that ForwardingStreamController implements StreamController, then
it is being used as an interface. If you care about wrapping it and the result
being a StreamController, then it is being used as an interface.

I'd still prefer not to have the wrapper class.

[nweiz, 2015-07-07 21:51:52]

Code review changes

[nweiz, 2015-07-07 21:51:59]

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/stream_con...
File lib/src/delegate/stream_controller.dart (right):

https://codereview.chromium.org/1219493008/diff/1/lib/src/delegate/stream_con...
lib/src/delegate/stream_controller.dart:11: /// Simple delegating wrapper around
an [StreamController].
On 2015/07/07 10:32:18, Lasse Reichstein Nielsen wrote:
> I'm not sure I would encourage an extended StreamController for something like
> that. It's still using StreamController as if it is a useful interface, and
not
> just a hodge-podge of functionality thrown together to solve a single problem
-
> interacting with a stream subscription.
> 
> I'd rather either have a new specialized API for your forwarding stream class
> with just the methods that makes sense for that, or something that exposes a
> StreamController (or StreamSink) rather than *being* one ("favor composition
> over inheritance", etc.). 
> 
> I also think it was a mistake to make StreamController be a StreamSink to
being
> with - it should have had an "asStreamSink()" method that returned a wrapper,
> keeping the plain interface simpler.
> 
> Basically, some classes are intended as interfaces and others are not, and I
> worry about blurring the lines. StreamController is in the not-interface
group,
> which means that code should generally not use it as an interface.
> If it matters that ForwardingStreamController implements StreamController,
then
> it is being used as an interface. If you care about wrapping it and the result
> being a StreamController, then it is being used as an interface.
> 
> I'd still prefer not to have the wrapper class.

I think there's documentary value in sharing an interface with
StreamController—if a new class would have the same functionality anyway,
sharing an interface makes it very clear that it can be accessed in the same way
as the old class.

That said, you're probably right that a StreamController probably isn't
something people should be wrapping in general, so I'll remove the delegate.

[nweiz, 2015-07-07 21:52:55]

Committed patchset #3 (id:40001) manually as
8d5716a22540bb8e790c3999fded524ac7693f88 (presubmit successful).