Skip to content

Update iterator.dart to fully spec moveNext() #39676

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Update iterator.dart to fully spec moveNext() #39676

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
adcda03
Update iterator.dart
jtlapp Dec 6, 2019
7048fd0
Update iterator.dart
mit-mit Jan 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions sdk/lib/core/iterator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,21 @@ part of dart.core;
* in the [library tour](http://www.dartlang.org/docs/dart-up-and-running/contents/ch03.html)
*/
abstract class Iterator<E> {
/**
* Moves to the next element.
*
* Returns true if [current] contains the next element.
* Returns false if no elements are left.
/**
* Advances the iterator to the next element of the iteration.
*
* Should be called before reading [current]. It the call to `moveNext` returns
* `true`, then [current] will contain the next element of the iteration until
* `moveNext` is called again. If the call returns `false`, there are no further
* elements and [current] should not be used any more.
*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This repeats the behavior mentioned in the class description, but it does make it easier to find this information while only having an Iterator typed variable available.

I'd prefer to phrase it slightly differently (we rarely use "invoke" in documentation, it's too formal). Maybe:

Use the [moveNext] method to move to the next element. This will change the
current value to that of the next element if there is a next element, which is when
[moveNext] returns true. Also use [moveNext] to start the iteration before accessing
the first element.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I misread this as a comment on current.

I'm not sure it's suitable as an addition on moveNext, because the documentation does say that already, but the existing text could probably be rewritten to express the same thing more clearly.

Maybe:

 /**
 * Advances the iterator to the next element of the iteration.
 * 
 * Should be called before reading [current]. It the call to `moveNext` returns
 * `true`, then [current] will contain the next element of the iteration until 
 * `moveNext` is called again. If the call returns `false`, there are no further
 * elements and [current] should not be used any more.
 *
 * It is safe to call [moveNext] after it has already returned `false`, 
 * but it must keep returning `false` and not have any other effect.
 *
 * A call to `moveNext` may throw for various reasons, including a concurrent change
 * to an underlying collection. If that happens, the iterator may be in an inconsistent 
 * state, and any further behavior of the iterator is unspecified, 
 * including the effect of reading [current]. 
 */

How does this read to you?

* It is safe to invoke [moveNext] even when the iterator is already
* positioned after the last element.
* In this case [moveNext] returns false again and has no effect.
* It is safe to call [moveNext] after it has already returned `false`,
* but it must keep returning `false` and not have any other effect.
*
* A call to `moveNext` may throw if iteration has been broken by
* changing the underlying collection.
* A call to `moveNext` may throw for various reasons, including a concurrent change
* to an underlying collection. If that happens, the iterator may be in an inconsistent
* state, and any further behavior of the iterator is unspecified,
* including the effect of reading [current].
*/
bool moveNext();

Expand Down