Warn when adding something to a closed sink and improve documentation

sdk/lib/async/stream.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of dart.async;
 
 // -------------------------------------------------------------------
 // Core Stream types
 // -------------------------------------------------------------------
 
@@ skipping 1445 matching lines @@
    * In case of an error the subscription will automatically cancel (even
    * when it was listening with `cancelOnError` set to `false`).
    *
    * In case of a `done` event the future completes with the given
    * [futureValue].
    */
   Future<E> asFuture<E>([E futureValue]);
 }
 
 /**
- * An interface that abstracts creation or handling of [Stream] events.
+ * A [Sink] that supports adding errors and closing.

[Lasse Reichstein Nielsen, 2017/04/26 08:26:18]

The [Sink] already supports closing, so it's redundant to mention here, so just
"... that supports adding errors."

Perhaps also say that this makes it suitable for capturing the results of
asynchronous *computations* (which can complete with a value *or* an error).

[floitsch, 2017/05/01 16:46:25]

Done.

  */
 abstract class EventSink<T> implements Sink<T> {

[Lasse Reichstein Nielsen, 2017/04/26 08:26:19]

The name "EventSink" comes from streams, so I'm not sure removing the reference
to streams in the documentation is really necessary.
If it is, maybe we should rename this sink to AsyncSink (which we won't because
it's breaking, but still!).
So, make some sort of documentation about what an "event" is in the class
documentation, because it's needed as context for the name (and the "event" uses
below).

[floitsch, 2017/05/01 16:46:24]

Added a paragraph:
 * The [EventSink] has been designed to handle asynchronous events from
 * [Stream]s. See, for example, [Stream.eventTransformed] which uses
 * `EventSink`s to transform events.

I agree, that the name is not great, but I don't think we should change the name
to `AsyncSink`. That's a misnomer as well.

-  /** Send a data event to a stream. */
+  /**
+   * Puts a data [event] into the sink.

[Lasse Reichstein Nielsen, 2017/04/26 08:26:19]

Puts .. into -> Adds .. to

We generally use "add" about what we do to sinks.
Dittos below.

[floitsch, 2017/05/01 16:46:25]

Done.

+   *
+   * Must not be called on a closed sink.
+   */
   void add(T event);
 
-  /** Send an async error to a stream. */
+  /**
+   * Puts an [errorEvent] into the sink.
+   *
+   * Must not be called on a closed sink.
+   */
   void addError(Object errorEvent, [StackTrace stackTrace]);

[Lasse Reichstein Nielsen, 2017/04/26 08:26:19]

Hmm, errorEvent -> error.
It's suspicious that the error is an event and the stackTrace isn't. And it's
not clear what event is (unless you write it somewhere, in which case you
probably don't need to repeat it here anyway).

[floitsch, 2017/05/01 16:46:25]

Done.

 
-  /** Close the sink. No further events can be added after closing. */
+  /**
+   * Closes the sink.
+   *
+   * No further events can be added after closing.

[Lasse Reichstein Nielsen, 2017/04/26 08:26:19]

Can you close more than once?

What is an event? (that should be documented in the class doc, as stated above).

[floitsch, 2017/05/01 16:46:24]

   * Calling this method more than once is allowed, but does nothing.
   *
   * Neither [add] nor [addError] must be called after this method.

done.

+   */
   void close();
 }
 
 /** [Stream] wrapper that only exposes the [Stream] interface. */
 class StreamView<T> extends Stream<T> {
   final Stream<T> _stream;
 
   const StreamView(Stream<T> stream)
       : _stream = stream,
         super._internal();
@@ skipping 12 matching lines @@
   }
 }
 
 /**
  * Abstract interface for a "sink" accepting multiple entire streams.
  *
  * A consumer can accept a number of consecutive streams using [addStream],
  * and when no further data need to be added, the [close] method tells the
  * consumer to complete its work and shut down.
  *
- * This class is not just a [Sink<Stream>] because it is also combined with
- * other [Sink] classes, like it's combined with [EventSink] in the
- * [StreamSink] class.
- *
  * The [Stream.pipe] accepts a `StreamConsumer` and will pass the stream
  * to the consumer's [addStream] method. When that completes, it will
  * call [close] and then complete its own returned future.
  */
 abstract class StreamConsumer<S> {
   /**
    * Consumes the elements of [stream].
    *
    * Listens on [stream] and does something for each event.
    *
@@ skipping 21 matching lines @@
    * Returns a future which is completed when the consumer has shut down.
    * If cleaning up can fail, the error may be reported in the returned future,
    * otherwise it completes with `null`.
    */
   Future close();
 }
 
 /**
  * A object that accepts stream events both synchronously and asynchronously.
  *
- * A [StreamSink] unifies the asynchronous methods from [StreamConsumer] and
- * the synchronous methods from [EventSink].
+ * A [StreamSink] combines the methods from [StreamConsumer] and [EventSink].
  *
  * The [EventSink] methods can't be used while the [addStream] is called.
  * As soon as the [addStream]'s [Future] completes with a value, the
  * [EventSink] methods can be used again.
  *
  * If [addStream] is called after any of the [EventSink] methods, it'll
  * be delayed until the underlying system has consumed the data added by the
  * [EventSink] methods.
  *
  * When [EventSink] methods are used, the [done] [Future] can be used to
@@ skipping 225 matching lines @@
   }
 
   void addError(error, [StackTrace stackTrace]) {
     _sink.addError(error, stackTrace);
   }
 
   void close() {
     _sink.close();
   }
 }