Use iterator, moveNext(), current.

Use iterator, moveNext(), current.

Committed: https://code.google.com/p/dart/source/detail?r=15019

[floitsch, 2012-11-14 15:09:35]

This is the first version of moving to C#-style iterators.
It still throws when using 'current' outside the boundaries, and as a result
many iterators are even uglier than before.

Also: I only made core and language tests work. I didn't update all other
packages yet.

[Lasse Reichstein Nielsen, 2012-11-15 10:24:37]

LGTM

https://codereview.chromium.org/11410086/diff/1/runtime/lib/array.dart
File runtime/lib/array.dart (right):

https://codereview.chromium.org/11410086/diff/1/runtime/lib/array.dart#newcod...
runtime/lib/array.dart:311: throw new StateError("No more elements");
Wrong message if _pos < 0 and length > 0.

https://codereview.chromium.org/11410086/diff/1/sdk/lib/collection/collection...
File sdk/lib/collection/collections.dart (right):

https://codereview.chromium.org/11410086/diff/1/sdk/lib/collection/collection...
sdk/lib/collection/collections.dart:62: return !iterable.iterator.hasNext;
moveNext().

https://codereview.chromium.org/11410086/diff/1/sdk/lib/core/queue.dart
File sdk/lib/core/queue.dart (right):

https://codereview.chromium.org/11410086/diff/1/sdk/lib/core/queue.dart#newco...
sdk/lib/core/queue.dart:326: if (identical(_currentEntry, _sentinel)) return
false;
The 'null' represents the initial state, right?
Documentation, please!

How about a boolean named '_done' that gets set when you move to the sentinel:
bool moveNext() {
  if (_done) return false;
  _currentEntry = _currentEntry._next;
  _done = identical(_currentEntry, _sentinel);
  return !_done;
}

Do use identical when it's what we really mean (and it is for linked lists and
sentinels in general), so I don't have to wonder what happens if the Entry class
implements operator==.

https://codereview.chromium.org/11410086/diff/1/tests/corelib/list_iterators_...
File tests/corelib/list_iterators_test.dart (right):

https://codereview.chromium.org/11410086/diff/1/tests/corelib/list_iterators_...
tests/corelib/list_iterators_test.dart:10: var elem = it.current;
I'd expect (pun intended) an "Expect.equals(a[i], elem)" here.

[floitsch, 2012-11-16 17:51:58]

https://codereview.chromium.org/11410086/diff/1/runtime/lib/array.dart
File runtime/lib/array.dart (right):

https://codereview.chromium.org/11410086/diff/1/runtime/lib/array.dart#newcod...
runtime/lib/array.dart:311: throw new StateError("No more elements");
On 2012/11/15 10:24:37, Lasse Reichstein Nielsen wrote:
> Wrong message if _pos < 0 and length > 0.

added TODO. this will go away with the new approach.

https://codereview.chromium.org/11410086/diff/1/sdk/lib/collection/collection...
File sdk/lib/collection/collections.dart (right):

https://codereview.chromium.org/11410086/diff/1/sdk/lib/collection/collection...
sdk/lib/collection/collections.dart:62: return !iterable.iterator.hasNext;
On 2012/11/15 10:24:37, Lasse Reichstein Nielsen wrote:
> moveNext().

Done.

https://codereview.chromium.org/11410086/diff/1/sdk/lib/core/queue.dart
File sdk/lib/core/queue.dart (right):

https://codereview.chromium.org/11410086/diff/1/sdk/lib/core/queue.dart#newco...
sdk/lib/core/queue.dart:326: if (identical(_currentEntry, _sentinel)) return
false;
On 2012/11/15 10:24:37, Lasse Reichstein Nielsen wrote:
> The 'null' represents the initial state, right?
> Documentation, please!
> 
> How about a boolean named '_done' that gets set when you move to the sentinel:
> bool moveNext() {
>   if (_done) return false;
>   _currentEntry = _currentEntry._next;
>   _done = identical(_currentEntry, _sentinel);
>   return !_done;
> }
> 
> Do use identical when it's what we really mean (and it is for linked lists and
> sentinels in general), so I don't have to wonder what happens if the Entry
class
> implements operator==.
>   

in theory there is no need to use identical since the elements are
DoubleLinkedQueueEntrys.
added comment, and used identical below.
this will become different with the new approach.

https://codereview.chromium.org/11410086/diff/1/tests/corelib/list_iterators_...
File tests/corelib/list_iterators_test.dart (right):

https://codereview.chromium.org/11410086/diff/1/tests/corelib/list_iterators_...
tests/corelib/list_iterators_test.dart:10: var elem = it.current;
On 2012/11/15 10:24:37, Lasse Reichstein Nielsen wrote:
> I'd expect (pun intended) an "Expect.equals(a[i], elem)" here.

Done.

[Ivan Posva, 2012-11-26 18:20:13]

https://codereview.chromium.org/11410086/diff/10001/runtime/lib/array.dart
File runtime/lib/array.dart (right):

https://codereview.chromium.org/11410086/diff/10001/runtime/lib/array.dart#ne...
runtime/lib/array.dart:108: Iterator<E> get iterator => new
_FixedSizeArrayIterator<E>(this);
We like our curly braces. There is no need to remove them. Thanks!

https://codereview.chromium.org/11410086/diff/10001/runtime/lib/array.dart#ne...
runtime/lib/array.dart:253: Iterator<E> get iterator => new
_FixedSizeArrayIterator<E>(this);
ditto

https://codereview.chromium.org/11410086/diff/10001/runtime/lib/growable_arra...
File runtime/lib/growable_array.dart (right):

https://codereview.chromium.org/11410086/diff/10001/runtime/lib/growable_arra...
runtime/lib/growable_array.dart:218: Iterator<T> get iterator => new
SequenceIterator<T>(this);
ditto

https://codereview.chromium.org/11410086/diff/10001/runtime/vm/intrinsifier.h
File runtime/vm/intrinsifier.h (left):

https://codereview.chromium.org/11410086/diff/10001/runtime/vm/intrinsifier.h...
runtime/vm/intrinsifier.h:74: V(_FixedSizeArrayIterator, get:hasNext,
FixedSizeArrayIterator_getHasNext)   \
The equivalent routines will need to be handled. Please add a TODO, otherwise we
will be hunting which things went missing.

[floitsch, 2012-11-28 13:44:34]

Ivan's comments addressed in https://codereview.chromium.org/11416240

https://codereview.chromium.org/11410086/diff/10001/runtime/lib/array.dart
File runtime/lib/array.dart (right):

https://codereview.chromium.org/11410086/diff/10001/runtime/lib/array.dart#ne...
runtime/lib/array.dart:108: Iterator<E> get iterator => new
_FixedSizeArrayIterator<E>(this);
On 2012/11/26 18:20:13, Ivan Posva wrote:
> We like our curly braces. There is no need to remove them. Thanks!

Done.

https://codereview.chromium.org/11410086/diff/10001/runtime/lib/array.dart#ne...
runtime/lib/array.dart:253: Iterator<E> get iterator => new
_FixedSizeArrayIterator<E>(this);
On 2012/11/26 18:20:13, Ivan Posva wrote:
> ditto

Done.

https://codereview.chromium.org/11410086/diff/10001/runtime/lib/growable_arra...
File runtime/lib/growable_array.dart (right):

https://codereview.chromium.org/11410086/diff/10001/runtime/lib/growable_arra...
runtime/lib/growable_array.dart:218: Iterator<T> get iterator => new
SequenceIterator<T>(this);
On 2012/11/26 18:20:13, Ivan Posva wrote:
> ditto

Done.

https://codereview.chromium.org/11410086/diff/10001/runtime/vm/intrinsifier.h
File runtime/vm/intrinsifier.h (left):

https://codereview.chromium.org/11410086/diff/10001/runtime/vm/intrinsifier.h...
runtime/vm/intrinsifier.h:74: V(_FixedSizeArrayIterator, get:hasNext,
FixedSizeArrayIterator_getHasNext)   \
On 2012/11/26 18:20:13, Ivan Posva wrote:
> The equivalent routines will need to be handled. Please add a TODO, otherwise
we
> will be hunting which things went missing.

done.