Add List.reversed to give a reverse fixed-length view of a list.

Add List.reversed to give a reverse fixed-length view of a list.

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

[floitsch, 2013-01-21 14:38:06]

Skipped the actual ReversedList code, until it doesn't throw for length changes.

Please add tests.

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

https://codereview.chromium.org/11896013/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:209: abstract class BaseListMixin<E> extends Iterable<E>
implements List<E> {
These classes are no longer in core.

https://codereview.chromium.org/11896013/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:296: abstract class FixedLengthListMixin<E> extends
BaseListMixin<E> {
Can't be a mixin, since it extends (or mixins) BaseListMixin.

https://codereview.chromium.org/11896013/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:505: E operator[](int index) { throw new
ArgumentError(index); }
RangeError

https://codereview.chromium.org/11896013/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:574: * list changes its length below the slice used by
this reversed list.
That's now how iterables work. And ReversedList (like MappedList) shouldn't
behave this way either.
The Iterator, on the other hand, should catch length changes.

[Lasse Reichstein Nielsen, 2013-01-21 14:43:40]

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

https://codereview.chromium.org/11896013/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:574: * list changes its length below the slice used by
this reversed list.
On 2013/01/21 14:38:06, floitsch wrote:
> That's now how iterables work. And ReversedList (like MappedList) shouldn't
> behave this way either.
> The Iterator, on the other hand, should catch length changes.


So what should it do?

[floitsch, 2013-01-21 15:05:55]

On 2013/01/21 14:43:40, Lasse Reichstein Nielsen wrote:
> https://codereview.chromium.org/11896013/diff/1/sdk/lib/core/list.dart
> File sdk/lib/core/list.dart (right):
> 
>
https://codereview.chromium.org/11896013/diff/1/sdk/lib/core/list.dart#newcod...
> sdk/lib/core/list.dart:574: * list changes its length below the slice used by
> this reversed list.
> On 2013/01/21 14:38:06, floitsch wrote:
> > That's now how iterables work. And ReversedList (like MappedList) shouldn't
> > behave this way either.
> > The Iterator, on the other hand, should catch length changes.
> 
> 
> So what should it do?

As discussed offline:
The implementation of reversed on list should have the same semantics as if
'reversed' came from Iterable. Example:
var list = [1, 2, 3];
var toto = list.take(5).skip(2).reversed;
list.addAll([4, 5, 6]);

print(toto);  // => [5, 4, 3]

[Lasse Reichstein Nielsen, 2013-01-22 10:18:52]

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

https://codereview.chromium.org/11896013/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:209: abstract class BaseListMixin<E> extends Iterable<E>
implements List<E> {
Moved to collection-dev.

https://codereview.chromium.org/11896013/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:296: abstract class FixedLengthListMixin<E> extends
BaseListMixin<E> {
True, so it needs a better name.
Just FixedLengthList?
FixedLengthListBase? 
I think I'll go with the latter for now.

https://codereview.chromium.org/11896013/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:505: E operator[](int index) { throw new
ArgumentError(index); }
On 2013/01/21 14:38:06, floitsch wrote:
> RangeError

Done.

[floitsch, 2013-01-22 14:24:57]

LGTM.
make sure you don't break co19 tests that rely on concurrent modifications.

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
File sdk/lib/collection_dev/list.dart (right):

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:272: * Iterates over a [Sequence] in growing
index order.
Bad merge?

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:276: final int _initialLength;
I really like this change, but it should probably be in a separate CL.

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:410: void operator []=(int index, E value) {
Not in this CL. I'm not even sure I agree.

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:425: return new EmptyList<E>();
Can this happen? If _start is [null] then we would already have an empty list.

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:439: return new EmptyList<E>();
ditto.

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:450: void sort([int compare(E first, E
second)]) {
Different CL together with []=.

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:487: void operator []=(int index, E value) {
Different CL.

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:534: void sort([int compare(E first, E
second)]) {
different CL.

[Lasse Reichstein Nielsen, 2013-01-22 14:55:12]

co19 didn't need changes at all. It's unspecified behavior in any case.

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
File sdk/lib/collection_dev/list.dart (right):

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:272: * Iterates over a [Sequence] in growing
index order.
Fixed.

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:276: final int _initialLength;
Done.

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:410: void operator []=(int index, E value) {
Moved to separate CL.

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:425: return new EmptyList<E>();
I dare not assume it can't happen. Call it safety in depth.

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:439: return new EmptyList<E>();
Same.

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:450: void sort([int compare(E first, E
second)]) {
On 2013/01/22 14:24:57, floitsch wrote:
> Different CL together with []=.

Done.

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:487: void operator []=(int index, E value) {
On 2013/01/22 14:24:57, floitsch wrote:
> Different CL.

Done.

https://codereview.chromium.org/11896013/diff/7001/sdk/lib/collection_dev/lis...
sdk/lib/collection_dev/list.dart:534: void sort([int compare(E first, E
second)]) {
On 2013/01/22 14:24:57, floitsch wrote:
> different CL.

Done.