Add StreamChannel and MultiChannel.

Add StreamChannel and MultiChannel.

These are exact copies of the classes and tests from the test package.

R=rnystrom@google.com, kevmoo@google.com

Committed: https://github.com/dart-lang/stream_channel/commit/21d98c9a548a6e343ebfe820da64db024b23d2b4

[Bob Nystrom, 2016-01-21 00:06:30]

rnystrom@google.com changed reviewers:
+ rnystrom@google.com

[Bob Nystrom, 2016-01-21 00:06:30]

lgtm

https://codereview.chromium.org/1610443002/diff/1/lib/src/multi_channel.dart
File lib/src/multi_channel.dart (right):

https://codereview.chromium.org/1610443002/diff/1/lib/src/multi_channel.dart#...
lib/src/multi_channel.dart:73: VirtualChannel virtualChannel([id]);
How about just calling these "channels"? It's not like there are non-virtual
ones and "channel" isn't already a claimed term, is it?

Also can id be typed?

https://codereview.chromium.org/1610443002/diff/1/lib/src/multi_channel.dart#...
lib/src/multi_channel.dart:129: var _nextId = 1;
"var" -> "int"?

I know it's obvious from the initializer, but I feel like we generally only rely
on that for final fields.

https://codereview.chromium.org/1610443002/diff/1/lib/src/multi_channel.dart#...
lib/src/multi_channel.dart:200: // closed.
Move this comment down.

[nweiz, 2016-01-21 00:34:35]

https://codereview.chromium.org/1610443002/diff/1/lib/src/multi_channel.dart
File lib/src/multi_channel.dart (right):

https://codereview.chromium.org/1610443002/diff/1/lib/src/multi_channel.dart#...
lib/src/multi_channel.dart:73: VirtualChannel virtualChannel([id]);
On 2016/01/21 00:06:30, Bob Nystrom wrote:
> How about just calling these "channels"? It's not like there are non-virtual
> ones and "channel" isn't already a claimed term, is it?

"channel" is used to describe stream channels in general, so I think that would
sound weird. Also, from the user's perspective, they're more virtual than the
MultiChannel itself, because they need to have an explicit exchange of IDs to be
established.

> Also can id be typed?

I'd prefer not to make any guarantees about the type of IDs we use in the API.
From the user's perspective, the only important thing about them is that they're
JSON-safe.

https://codereview.chromium.org/1610443002/diff/1/lib/src/multi_channel.dart#...
lib/src/multi_channel.dart:129: var _nextId = 1;
On 2016/01/21 00:06:30, Bob Nystrom wrote:
> "var" -> "int"?
> 
> I know it's obvious from the initializer, but I feel like we generally only
rely
> on that for final fields.

Really? I do this pretty frequently. I guess I'd expect "dynamic" or "Object" to
be written explicitly if the type could vary beyond the initializer.

https://codereview.chromium.org/1610443002/diff/1/lib/src/multi_channel.dart#...
lib/src/multi_channel.dart:200: // closed.
On 2016/01/21 00:06:30, Bob Nystrom wrote:
> Move this comment down.

I'm not sure why or where. All of the logic in the method body relates to
shutting down the virtual channel.

[Bob Nystrom, 2016-01-21 00:38:30]

https://codereview.chromium.org/1610443002/diff/1/lib/src/multi_channel.dart
File lib/src/multi_channel.dart (right):

https://codereview.chromium.org/1610443002/diff/1/lib/src/multi_channel.dart#...
lib/src/multi_channel.dart:73: VirtualChannel virtualChannel([id]);
On 2016/01/21 00:34:35, nweiz wrote:
> > Also can id be typed?
> 
> I'd prefer not to make any guarantees about the type of IDs we use in the API.
> From the user's perspective, the only important thing about them is that
they're
> JSON-safe.

In _MultiChannel.virtualChannel(), don't you assume it's an int if it isn't
null?

https://codereview.chromium.org/1610443002/diff/1/lib/src/multi_channel.dart#...
lib/src/multi_channel.dart:129: var _nextId = 1;
On 2016/01/21 00:34:35, nweiz wrote:
> On 2016/01/21 00:06:30, Bob Nystrom wrote:
> > "var" -> "int"?
> > 
> > I know it's obvious from the initializer, but I feel like we generally only
> rely
> > on that for final fields.
> 
> Really? I do this pretty frequently. I guess I'd expect "dynamic" or "Object"
to
> be written explicitly if the type could vary beyond the initializer.

<shrug>

https://codereview.chromium.org/1610443002/diff/1/lib/src/multi_channel.dart#...
lib/src/multi_channel.dart:200: // closed.
On 2016/01/21 00:34:35, nweiz wrote:
> On 2016/01/21 00:06:30, Bob Nystrom wrote:
> > Move this comment down.
> 
> I'm not sure why or where. All of the logic in the method body relates to
> shutting down the virtual channel.

I meant down to the _innerSink.add() call. Isn't that where you're sending a
message without data?

[nweiz, 2016-01-21 01:02:15]

Code review changes

[nweiz, 2016-01-21 01:02:24]

Description was changed from

==========
Add StreamChannel and MultiChannel.

These are exact copies of the classes and tests from the test package.

R=kevmoo@google.com
==========

to

==========
Add StreamChannel and MultiChannel.

These are exact copies of the classes and tests from the test package.

R=rnystrom@google.com, kevmoo@google.com

Committed:
https://github.com/dart-lang/stream_channel/commit/21d98c9a548a6e343ebfe820da...
==========

[nweiz, 2016-01-21 01:02:24]

Committed patchset #2 (id:20001) manually as
21d98c9a548a6e343ebfe820da64db024b23d2b4 (presubmit successful).

[nweiz, 2016-01-21 01:05:55]

https://codereview.chromium.org/1610443002/diff/1/lib/src/multi_channel.dart
File lib/src/multi_channel.dart (right):

https://codereview.chromium.org/1610443002/diff/1/lib/src/multi_channel.dart#...
lib/src/multi_channel.dart:73: VirtualChannel virtualChannel([id]);
On 2016/01/21 00:38:30, Bob Nystrom wrote:
> On 2016/01/21 00:34:35, nweiz wrote:
> > > Also can id be typed?
> > 
> > I'd prefer not to make any guarantees about the type of IDs we use in the
API.
> > From the user's perspective, the only important thing about them is that
> they're
> > JSON-safe.
> 
> In _MultiChannel.virtualChannel(), don't you assume it's an int if it isn't
> null?

Yes, but that's still an implementation detail. The user knows where the ID will
be in a message, and they provide whatever's there regardless of its type to
this method. Internally, we know it'll be an int, but we don't guarantee that to
the user.

https://codereview.chromium.org/1610443002/diff/1/lib/src/multi_channel.dart#...
lib/src/multi_channel.dart:200: // closed.
On 2016/01/21 00:38:30, Bob Nystrom wrote:
> On 2016/01/21 00:34:35, nweiz wrote:
> > On 2016/01/21 00:06:30, Bob Nystrom wrote:
> > > Move this comment down.
> > 
> > I'm not sure why or where. All of the logic in the method body relates to
> > shutting down the virtual channel.
> 
> I meant down to the _innerSink.add() call. Isn't that where you're sending a
> message without data?

Done.