Update documentation of StreamIterator, improve tests.

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 1655 matching lines @@
 /**
  * An [Iterable] like interface for the values of a [Stream].
  *
  * This wraps a [Stream] and a subscription on the stream. It listens
  * on the stream, and completes the future returned by [moveNext] when the
  * next value becomes available.
  */
 abstract class StreamIterator<T> {
 
   /** Create a [StreamIterator] on [stream]. */
-  factory StreamIterator(Stream<T> stream)
-      // TODO(lrn): use redirecting factory constructor when type
-      // arguments are supported.
-      => new _StreamIteratorImpl<T>(stream);
+  factory StreamIterator(Stream<T> stream) = _StreamIteratorImpl<T>;
 
   /**
    * Wait for the next stream value to be available.
    *
-   * It is not allowed to call this function again until the future has
-   * completed. If the returned future completes with anything except `true`,
+   * The function must not be called again until the future returned by a

[Søren Gjesse, 2015/07/03 11:06:52]

"must not": What happens if that is violated? StateError?

[Lasse Reichstein Nielsen, 2015/07/03 11:21:17]

Yes, it throws StateError. We don't document that.

+   * previous call is completed.
+   * When the returned future completes with `true`, a new event is available
+   * and can be read from [current].
+   *
+   * If the returned future completes with anything except `true`
+   * (whether with another value or with an error),

[Søren Gjesse, 2015/07/03 11:06:52]

When can "another value" be anything but false?

[Lasse Reichstein Nielsen, 2015/07/03 11:21:17]

It *should* not, but this is an interface, but we can't guarantee that an
implementation won't do something else.

But let's state that it must complete with true or false, then it's the
implementation's fault if it doesn't.

    * the iterator is done, and no new value will ever be available.
    *
    * The future may complete with an error, if the stream produces an error.
    */
   Future<bool> moveNext();
 
   /**
    * The current value of the stream.
    *
-   * Only valid when the future returned by [moveNext] completes with `true`
-   * as value, and only until the next call to [moveNext].
+   * Is `null` before the first call to [moveNext] and after a call to
+   * `moveNext` completes with a `false` result or an error.
+   *
+   * When a `moveNext` call completes with `true`, the `current` field holds
+   * the most recent event of the stream, and it stays like that until the next
+   * call to `moveNext`.
+   * Between a call to `moveNext` and when its returned future completes,
+   * the value is unspecified.
    */
   T get current;
 
   /**
    * Cancels the stream iterator (and the underlying stream subscription) early.
    *
    * The stream iterator is automatically canceled if the [moveNext] future
    * completes with either `false` or an error.
    *
-   * If a [moveNext] call has been made, it will complete with `false` as value,
-   * as will all further calls to [moveNext].
-   *
    * If you need to stop listening for values before the stream iterator is
    * automatically closed, you must call [cancel] to ensure that the stream
    * is properly closed.
    *
+   * If [moveNext] has been called when the iterator is cancelled,
+   * its returned future will complete with `false` as value,
+   * as will all further calls to [moveNext].

[Søren Gjesse, 2015/07/03 11:06:52]

So moveNext cannot distinguish between end of stream and cancel. Should there be
a canceled exception in this case?

[Lasse Reichstein Nielsen, 2015/07/03 11:21:17]

I don't think it's worth it.
The code calling cancel should also be the code calling moveNext, so it should
be able to coordinate that. It should not need to introduce an error that you
have to handle as well. That means that any error would come from the stream.

+   *
    * Returns a future if the cancel-operation is not completed synchronously.
    * Otherwise returns `null`.
    */
   Future cancel();
 }
 
 
 /**
  * Wraps an [_EventSink] so it exposes only the [EventSink] interface.
  */
 class _ControllerEventSinkWrapper<T> implements EventSink<T> {
   EventSink _sink;
   _ControllerEventSinkWrapper(this._sink);
 
   void add(T data) { _sink.add(data); }
   void addError(error, [StackTrace stackTrace]) {
     _sink.addError(error, stackTrace);
   }
   void close() { _sink.close(); }
 }