FR: UInt8Buffer.bufferStream()

[matanlurey]

As discussed offline:

abstract class UInt8Buffer {
    static Future<UInt8Buffer> bufferStream(Stream<List<int>> byteStream, {int length});
}

/cc @nex3 @lrhn

[lrhn]

Not where I would want it - I don't think a stream operation belongs on Uint8Buffer. It's a matter of separation of concerns, and of abstraction levels. Uint8Buffer has one job, it shouldn't be combined with unrelated (or semi-related) things. It shouldn't introduce asynchrony into byte operations - which are basically the bottom of the abstraction hierarchy. To be precise, I don't want the typed_data package to even import dart:async.

I would be more welcoming to putting the helper method somewhere in package:async instead.

[matanlurey]

To be clear I'd be fine with it in pkg/async as a top-level bufferStream (or the sorts), but that means pkg/async will be importing this one. If you're both OK with it I'll move it over.

[nex3]

@lrhn I don't understand the resistance to importing dart:async. Asynchronous operations are part of the core language and library, and collecting data from a byte stream is the most common use of UInt8Buffer (and the typed_data package in general). Adding support there feels very similar to me to methods like, for example, Codec.encodeStream().

[lrhn]

It's my (perhaps idiosyncratic) notion of abstractions and abstractions levels.

Classes and libraries should have cleanly delimited jobs to handle and abstractions that they work on. The typed_data library should only worry about typed data, not try to "interface" with other concepts just because it might be convenient, and at least not more complex concepts. Convenience is a good thing in many cases, but not the most important part of API design - I think separation of concerns is much more important. Otherwise you just end up with a tight dependency graph instead of clean composable single-purpose APIs.

When I write operations that work on byte streams, I typically write them as operations on bytes and then provide a way for streams to interact with that.

So, dart:typed_data is not about asynchrony at all. It's not a concern for that library, and to separate concerns, it shouldn't start worrying about asynchrony, futures or streams at all. That's for another library.

The async library is a better location because the stream is the important part here - using Uint8Buffer is an implementation choice for the method we are talking about, not a part of its API. If we had a library specially for byte streams, then it would fit even better there. It's not an operation on Stream itself because not all streams are byte streams. It could be a static method, but that's not necessary, and I would still prefer to reserve those for operations on all streams.

So, my preferred solution is a top-level method in package:async.

(I'm sure we have gone against these principles in many other cases, and in quite a few of them, I have come to regret that later, so I'm quite disinclined to do it again).

If the Stream<T> interface had a Future<R> collect(EventCollector<T, R> collector) with class EventCollector<T, R> implements EventSink<T> { Future<R> close(); } (that is, an event-sink where the close operation can return a value, not just null), then we could just define a class ByteCollector implements EventCollector<List<int>, <List<int>> { ... } and use that, but our Stream API doesn't really allow that because EventSink.cancel returns void and StreamSink.cancel should only return null. (Maybe we should reconsider that choice).

[matanlurey]

Thanks @lrhn we can close this.