List.mappedBy/skip/take returns a List.

List.mappedBy/skip/take returns a List.

BUG: http://dartbug.com/8063
BUG: http://dartbug.com/8064

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

[Lasse Reichstein Nielsen, 2013-01-24 06:49:32]

lgtm

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

https://codereview.chromium.org/12041045/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:204: * by the result of [:f(e):].
Mention "unmodifiable" somewhere.

https://codereview.chromium.org/12041045/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:215: * Returns a [List] that hides the first [n]
elements. The returned
Newlines after '.'.

https://codereview.chromium.org/12041045/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:216: * list is a view backed by [this].
[this] -> [:this:]. It's not a declared name.

https://codereview.chromium.org/12041045/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:218: * If [this] has fewer than [n] elements, then the
resulting [Iterable] will
If -> While. It can change if the underlying list changes.
[Iterable] -> [List].

https://codereview.chromium.org/12041045/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:225: * backed by this.
this -> [:this:]  (or "this [List]").

https://codereview.chromium.org/12041045/diff/1/tests/corelib/iterable_mappin...
File tests/corelib/iterable_mapping_test.dart (right):

https://codereview.chromium.org/12041045/diff/1/tests/corelib/iterable_mappin...
tests/corelib/iterable_mapping_test.dart:10: set1..add(11)
Why not set1.addAll([1, 2, 3]); ?
What is the purpose of creating it like this (i.e., needs comment).

https://codereview.chromium.org/12041045/diff/1/tests/corelib/iterable_mappin...
tests/corelib/iterable_mapping_test.dart:15: Iterable mapped =
list1.mappedBy((x) => x + 1);
Why "Iterable" and not "List"?

https://codereview.chromium.org/12041045/diff/1/tests/corelib/iterable_mappin...
tests/corelib/iterable_mapping_test.dart:17: Expect.listEquals([2, 3, 4],
mapped.toList());
And why .toList then?

[floitsch, 2013-01-24 13:48:34]

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

https://codereview.chromium.org/12041045/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:204: * by the result of [:f(e):].
On 2013/01/24 06:49:32, Lasse Reichstein Nielsen wrote:
> Mention "unmodifiable" somewhere.

Done.

https://codereview.chromium.org/12041045/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:215: * Returns a [List] that hides the first [n]
elements. The returned
On 2013/01/24 06:49:32, Lasse Reichstein Nielsen wrote:
> Newlines after '.'.

Done.

https://codereview.chromium.org/12041045/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:216: * list is a view backed by [this].
On 2013/01/24 06:49:32, Lasse Reichstein Nielsen wrote:
> [this] -> [:this:]. It's not a declared name.

Done.

https://codereview.chromium.org/12041045/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:218: * If [this] has fewer than [n] elements, then the
resulting [Iterable] will
On 2013/01/24 06:49:32, Lasse Reichstein Nielsen wrote:
> If -> While. It can change if the underlying list changes.
> [Iterable] -> [List].

Done.

https://codereview.chromium.org/12041045/diff/1/sdk/lib/core/list.dart#newcod...
sdk/lib/core/list.dart:225: * backed by this.
On 2013/01/24 06:49:32, Lasse Reichstein Nielsen wrote:
> this -> [:this:]  (or "this [List]").

Done.

https://codereview.chromium.org/12041045/diff/1/tests/corelib/iterable_mappin...
File tests/corelib/iterable_mapping_test.dart (right):

https://codereview.chromium.org/12041045/diff/1/tests/corelib/iterable_mappin...
tests/corelib/iterable_mapping_test.dart:10: set1..add(11)
On 2013/01/24 06:49:32, Lasse Reichstein Nielsen wrote:
> Why not set1.addAll([1, 2, 3]); ?
> What is the purpose of creating it like this (i.e., needs comment).

Done.

https://codereview.chromium.org/12041045/diff/1/tests/corelib/iterable_mappin...
tests/corelib/iterable_mapping_test.dart:15: Iterable mapped =
list1.mappedBy((x) => x + 1);
On 2013/01/24 06:49:32, Lasse Reichstein Nielsen wrote:
> Why "Iterable" and not "List"?

Because I want to prepare for the change back.

https://codereview.chromium.org/12041045/diff/1/tests/corelib/iterable_mappin...
tests/corelib/iterable_mapping_test.dart:17: Expect.listEquals([2, 3, 4],
mapped.toList());
On 2013/01/24 06:49:32, Lasse Reichstein Nielsen wrote:
> And why .toList then?

removed.

[Sean Eagan, 2013-01-24 17:36:23]

On 2013/01/23 16:10:13, floitsch wrote:

You probably already thought of this, but:

* same for where, takeWhile, and skipWhile?
* should corresponding methods on Sets return Sets (other than mappedBy)?

Thanks!

[floitsch, 2013-01-28 16:32:55]

On 2013/01/24 17:36:23, seaneagan1 wrote:
> On 2013/01/23 16:10:13, floitsch wrote:
> 
> You probably already thought of this, but:
> 
> * same for where, takeWhile, and skipWhile?
> * should corresponding methods on Sets return Sets (other than mappedBy)?
> 
> Thanks!

where, takeWhile and skipWhile are not lists, since there is no immediate
mapping of indices.
Sets are more difficult. For 'where', the contains-method can (and should) be
made more efficient, but since the length is not O(1) I wouldn't call it a Set.
For 'mappedBy' the length stays the same, but the contains becomes O(n). So
definitely not a Set.
In general Sets are not as clear-cut as Lists, where mappedBy, skip and take
really just wrap the original list and provide efficient List interfaces.