Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Making a stream transformer involves a lot of boilerplate #27740

Closed
Hixie opened this issue Nov 3, 2016 · 6 comments
Closed

Making a stream transformer involves a lot of boilerplate #27740

Hixie opened this issue Nov 3, 2016 · 6 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. area-documentation Prefer using 'type-documentation' and a specific area label. core-a docs-api library-async type-enhancement A request for a change that isn't a bug

Comments

@Hixie
Copy link
Contributor

Hixie commented Nov 3, 2016

Edit: this comment is wrong, see below.

As far as I can tell, the simplest way to write a StreamTransformer is something like this (untested):

class CloudBitStreamParser extends StreamTransformer<String, dynamic> {
  @override
  Stream<dynamic> bind(Stream<String> stream) {
    Stream<dynamic> output;
    StreamSubscription<String> input;
    output = StreamController<dynamic>(
      onListen: () {
        input = stream.listen(
          onData: (String event) {
            if (event.isEmpty)
              return;
            if (event.startsWith('data:')) {
              output.add(JSON.decode(event.substring(5))); // 5 is the length of the string 'data:'
              return;
            }
            output.addError('Unexpected data from CloudBit stream: "$event"');
          },
          onError: (dynamic exception, StackTrace stack) {
            output.addError(exception, stack);
          },
          onDone: () {
            output.close();
          },
        );
      },
      onPause: () {
        input.pause();
      },
      onResume: () {
        input.resume();
      },
      onCancel: () {
        input.cancel();
      },
    );
    return output;
  }
}

It seems like the above could be simplified to:

class CloudBitStreamParser extends FooStreamTransformer<String, dynamic> {
  @override
  void convert(String event, StreamSubscription<String> input, Stream<dynamic> output) {
    if (event.isEmpty)
      return;
    if (event.startsWith('data:')) {
      output.add(JSON.decode(event.substring(5))); // 5 is the length of the string 'data:'
      return;
    }
    output.addError('Unexpected data from CloudBit stream: "$event"');
  }
}

...where FooStreamTransformer is a subclass of StreamTransformer that does all the boilerplate logic.

(Alternatively, if the code above is wrong, then the dartdocs for StreamTransformer should be expanded to explain how to actually implement a StreamTransformer.)

@lrhn
Copy link
Member

lrhn commented Nov 3, 2016

You can use StreamTransformer.fromHandlers to easily create transformers that just convert input events to output events.

Example:

new StreamTransformer.fromHandlers(handleData: (String event, EventSink output) {
  if (event.startsWith('data:')) {
    output.add(JSON.decode(event.substring('data:'.length)));
  } else if (event.isNotEmpty) {
    output.addError('Unexpected data from CloudBit stream: "$event"');
  }
});

@Hixie
Copy link
Contributor Author

Hixie commented Nov 3, 2016

Turns out the code I have in the original comment won't work at all because you just can't extend StreamTransformer at all.

The documentation for StreamTransformer:

  • Should explain that you can't extend it and should just implement it instead.
  • Should explain why you can't extend it.
  • Should explain how to implement a StreamTransformer, e.g. pointing to its fromHandlers constructor.

The docs for fromHandlers should explain:

  • What the default behaviour for errors on the incoming stream is.
  • What it handles for you (e.g. forwarding paused state, cancellation).
  • How to cancel the stream in response to an event.

@lrhn
Copy link
Member

lrhn commented Nov 3, 2016

That is reasonable. It's clearly under-documented.
The "why" you can't extend it is simple - it's an interface, it has no implementation to inherit, so there is no reason to extend it.

(The StreamTransformer class is really a mistake - it's a single-method-interface so it should just have been a function type, but too late to fix that now).

@Hixie
Copy link
Contributor Author

Hixie commented Nov 3, 2016

Ah, yes, I was confused by the other classes around here and thought this one had some implementation logic you would want to inherit.

@anders-sandholm anders-sandholm added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-async area-documentation Prefer using 'type-documentation' and a specific area label. labels Nov 3, 2016
@Hixie
Copy link
Contributor Author

Hixie commented Nov 6, 2016

(The StreamTransformer class is really a mistake - it's a single-method-interface so it should just have been a function type, but too late to fix that now).

It's cleaner to use a class when you have long-lived state (the alternative is jumping through hoops to get the closure to capture the state you want to keep, which can be somewhat ugly). In fact in doing the work I'm doing now, I rather wish there was a class that implemented the fromHandlers stuff so I could inherit from that rather than using fromHandlers...

@Hixie
Copy link
Contributor Author

Hixie commented Nov 6, 2016

...though I guess since StreamTransformer wants to be re-usable, that doesn't really work here anyway.

@floitschG floitschG removed the core-m label Aug 21, 2017
@lrhn lrhn added the type-enhancement A request for a change that isn't a bug label Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. area-documentation Prefer using 'type-documentation' and a specific area label. core-a docs-api library-async type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants