Add `byteCollector` stream transformer and `collectBytes` function.

Add `byteCollector` stream transformer and `collectBytes` function.

BUG= https://github.com/dart-lang/typed_data/issues/2
R=nweiz@google.com

Committed: https://github.com/dart-lang/async/commit/4a55a88cd178e51a533a1c440eae233d0047d95b

[Lasse Reichstein Nielsen, 2017-01-25 12:18:50]

Description was changed from

==========
Add `byteCollector` stream transformer and `collectBytes` function.

BUG= https://github.com/dart-lang/typed_data/issues/2#issuecomment-274958794
==========

to

==========
Add `byteCollector` stream transformer and `collectBytes` function.

BUG= https://github.com/dart-lang/typed_data/issues/2
==========

[Lasse Reichstein Nielsen, 2017-01-25 12:19:37]

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

[nweiz, 2017-01-25 23:00:40]

https://codereview.chromium.org/2649233006/diff/1/CHANGELOG.md
File CHANGELOG.md (right):

https://codereview.chromium.org/2649233006/diff/1/CHANGELOG.md#newcode3
CHANGELOG.md:3: * Add `byteCollector` stream transformer and `collectBytes`
function.
"Add a", "and a"

https://codereview.chromium.org/2649233006/diff/1/CHANGELOG.md#newcode4
CHANGELOG.md:4: These collects list-of-byte events into a single byte list.
"collects" -> "collect"

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

https://codereview.chromium.org/2649233006/diff/1/lib/src/byte_collector.dart...
lib/src/byte_collector.dart:19: const StreamTransformer<List<int>,
Uint8List>(_collectionTransformer);
I'm not sure I see the use of a transformer here. It's only ever going to
produce a stream with a single value—in what circumstance would I use it rather
than collectBytes()?

https://codereview.chromium.org/2649233006/diff/1/lib/src/byte_collector.dart...
lib/src/byte_collector.dart:31: List<List<int>> byteLists = [];
"var byteLists = <List<int>>[]"

Also "var" for assignments below.

https://codereview.chromium.org/2649233006/diff/1/lib/src/byte_collector.dart...
lib/src/byte_collector.dart:33: await for (var list in source) {
I'm pretty sure "await for" is still very slow on most implementations. It's
fairly likely that this will be used by performance-critical networking
applications, so we should probably use .listen() directly.

https://codereview.chromium.org/2649233006/diff/1/test/byte_collection_test.dart
File test/byte_collection_test.dart (right):

https://codereview.chromium.org/2649233006/diff/1/test/byte_collection_test.d...
test/byte_collection_test.dart:14: Future<Uint8List> result =
collectBytes(streamOf([[0], [1], [2], [256]]));
"var", also below.

https://codereview.chromium.org/2649233006/diff/1/test/byte_collection_test.d...
test/byte_collection_test.dart:15: var expected = new Uint8List.fromList([0, 1,
2, 0]);
Just compare against a plain list—test doesn't validate that the types match
exactly.

https://codereview.chromium.org/2649233006/diff/1/test/byte_collection_test.d...
test/byte_collection_test.dart:87: 
Extra empty lines.

https://codereview.chromium.org/2649233006/diff/1/test/byte_collection_test.d...
test/byte_collection_test.dart:96: streamOf(Iterable<List<int>> lists) async* {
Why not just use new Stream.fromIterable()?

[Lasse Reichstein Nielsen, 2017-01-26 09:43:06]

https://codereview.chromium.org/2649233006/diff/1/CHANGELOG.md
File CHANGELOG.md (right):

https://codereview.chromium.org/2649233006/diff/1/CHANGELOG.md#newcode3
CHANGELOG.md:3: * Add `byteCollector` stream transformer and `collectBytes`
function.
On 2017/01/25 23:00:40, nweiz wrote:
> "Add a", "and a"

Done.

https://codereview.chromium.org/2649233006/diff/1/CHANGELOG.md#newcode4
CHANGELOG.md:4: These collects list-of-byte events into a single byte list.
On 2017/01/25 23:00:40, nweiz wrote:
> "collects" -> "collect"

Acknowledged.

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

https://codereview.chromium.org/2649233006/diff/1/lib/src/byte_collector.dart...
lib/src/byte_collector.dart:19: const StreamTransformer<List<int>,
Uint8List>(_collectionTransformer);
It might be a little too speculative :)
I was thinking you might want it because it allows chaining.
  someStream.map(..).where(..).transform(byteCollector).first.then(...)
instead of.
  collectBytes(someStream.map(..).where(..)).then(...)

On the other hand, a future is likely to be used with `await` so there
won't be nothing after the call. 

I was also considering having a version that collected up to a specific size
only, so you could collect a sequence of byte lists of length 5000 , but that
would be a different API anyway, and not this const value.

So, good point, I'll just remove it again.

https://codereview.chromium.org/2649233006/diff/1/lib/src/byte_collector.dart...
lib/src/byte_collector.dart:31: List<List<int>> byteLists = [];
Done.
(Even though I was actually deliberately creating a dynamic list in spec mode,
it's probably not important in practice :)

https://codereview.chromium.org/2649233006/diff/1/lib/src/byte_collector.dart...
lib/src/byte_collector.dart:33: await for (var list in source) {
Argh.
On one hand, I'm convinced it will never be made fast unless we have important
code that is slowed down by it.
On the other hand, I don't want to make it the user's problem (any more than it
already is).
So, done with listen.

https://codereview.chromium.org/2649233006/diff/1/test/byte_collection_test.dart
File test/byte_collection_test.dart (right):

https://codereview.chromium.org/2649233006/diff/1/test/byte_collection_test.d...
test/byte_collection_test.dart:14: Future<Uint8List> result =
collectBytes(streamOf([[0], [1], [2], [256]]));
On 2017/01/25 23:00:40, nweiz wrote:
> "var", also below.

Done.

https://codereview.chromium.org/2649233006/diff/1/test/byte_collection_test.d...
test/byte_collection_test.dart:15: var expected = new Uint8List.fromList([0, 1,
2, 0]);
On 2017/01/25 23:00:40, nweiz wrote:
> Just compare against a plain list—test doesn't validate that the types match
> exactly.

Done.

https://codereview.chromium.org/2649233006/diff/1/test/byte_collection_test.d...
test/byte_collection_test.dart:87: 
On 2017/01/25 23:00:40, nweiz wrote:
> Extra empty lines.

Done.

https://codereview.chromium.org/2649233006/diff/1/test/byte_collection_test.d...
test/byte_collection_test.dart:96: streamOf(Iterable<List<int>> lists) async* {
Too easy!
I had something more complicated here originally, but now you are right, I could
just use Stream.fromIterable.

[nweiz, 2017-01-26 19:07:47]

lgtm

Before you release this, consider another CL that adds a
collecyBytesCancelable() function which does the same thing but returns a
CancelableOperation that can cancel the underlying StreamSubscription.

[Lasse Reichstein Nielsen, 2017-01-27 11:48:27]

Description was changed from

==========
Add `byteCollector` stream transformer and `collectBytes` function.

BUG= https://github.com/dart-lang/typed_data/issues/2
==========

to

==========
Add `byteCollector` stream transformer and `collectBytes` function.

BUG= https://github.com/dart-lang/typed_data/issues/2
R=nweiz@google.com

Committed:
https://github.com/dart-lang/async/commit/4a55a88cd178e51a533a1c440eae233d004...
==========

[Lasse Reichstein Nielsen, 2017-01-27 11:48:28]

Committed patchset #2 (id:20001) manually as
4a55a88cd178e51a533a1c440eae233d0047d95b (presubmit successful).

[Lasse Reichstein Nielsen, 2017-01-27 11:48:34]

Good idea.