Add fromIterable(s) constructors in SplayTreeMap and LinkedHashMap.

Add fromIterable(s) constructors in SplayTreeMap and LinkedHashMap.

R=floitsch@google.com

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

[zarah, 2013-06-27 15:15:28]

Ptal

[floitsch, 2013-06-27 15:21:38]

LGTM.

https://codereview.chromium.org/18072004/diff/1/sdk/lib/collection/linked_has...
File sdk/lib/collection/linked_hash_map.dart (right):

https://codereview.chromium.org/18072004/diff/1/sdk/lib/collection/linked_has...
sdk/lib/collection/linked_hash_map.dart:33: LinkedHashMap<K, V> map = new
LinkedHashMap<K, V>();
We start to have too much code-duplication.
Let's move these functions out and share them.

_id(x) => x;
// (missing types, documentation, ...)
_fillMapWithMappedIterable(map, iterable, key, element) { ... }
_fillMapWithIterables(map, keys, values) { ... }

I would put it in maps.dart.

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

https://codereview.chromium.org/18072004/diff/1/sdk/lib/collection/splay_tree...
sdk/lib/collection/splay_tree.dart:257: {int compare(K key1, K key2), K
key(element), V value(element)}) {
move compare last. It doesn't make any semantic difference, but for
documentation and consistency I prefer it there.

[zarah, 2013-06-27 15:58:52]

https://codereview.chromium.org/18072004/diff/1/sdk/lib/collection/linked_has...
File sdk/lib/collection/linked_hash_map.dart (right):

https://codereview.chromium.org/18072004/diff/1/sdk/lib/collection/linked_has...
sdk/lib/collection/linked_hash_map.dart:33: LinkedHashMap<K, V> map = new
LinkedHashMap<K, V>();
On 2013/06/27 15:21:38, floitsch wrote:
> We start to have too much code-duplication.
> Let's move these functions out and share them.
> 
> _id(x) => x;
> // (missing types, documentation, ...)
> _fillMapWithMappedIterable(map, iterable, key, element) { ... }
> _fillMapWithIterables(map, keys, values) { ... }
> 
> I would put it in maps.dart.

Agree! Done.

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

https://codereview.chromium.org/18072004/diff/1/sdk/lib/collection/splay_tree...
sdk/lib/collection/splay_tree.dart:257: {int compare(K key1, K key2), K
key(element), V value(element)}) {
On 2013/06/27 15:21:38, floitsch wrote:
> move compare last. It doesn't make any semantic difference, but for
> documentation and consistency I prefer it there.

Done.

[floitsch, 2013-06-27 16:04:31]

LGTM.

https://codereview.chromium.org/18072004/diff/5001/sdk/lib/collection/hash_ma...
File sdk/lib/collection/hash_map.dart (right):

https://codereview.chromium.org/18072004/diff/5001/sdk/lib/collection/hash_ma...
sdk/lib/collection/hash_map.dart:25: factory HashMap.fromIterable(Iterable
iterable,
Actually I think we need to copy the dart-doc from Map. These classes appear in
the dart-doc separately and don't inherit the doc from there.
Ditto for all other constructors.

https://codereview.chromium.org/18072004/diff/5001/sdk/lib/collection/maps.dart
File sdk/lib/collection/maps.dart (right):

https://codereview.chromium.org/18072004/diff/5001/sdk/lib/collection/maps.da...
sdk/lib/collection/maps.dart:84: * Helper to fill a map with key/value pairs
computed from [iterable].
Fills a map ...

Since it's a private method it cannot be much else than a helper.

https://codereview.chromium.org/18072004/diff/5001/sdk/lib/collection/maps.da...
sdk/lib/collection/maps.dart:88: static void _fillMapWithMappedIterable(Map<K,V>
map, Iterable<K> iterable,
Don't use named arguments for internal helper methods. Pass them directly. It's
cheaper.

[zarah, 2013-06-27 17:00:07]

https://codereview.chromium.org/18072004/diff/5001/sdk/lib/collection/hash_ma...
File sdk/lib/collection/hash_map.dart (right):

https://codereview.chromium.org/18072004/diff/5001/sdk/lib/collection/hash_ma...
sdk/lib/collection/hash_map.dart:25: factory HashMap.fromIterable(Iterable
iterable,
On 2013/06/27 16:04:32, floitsch wrote:
> Actually I think we need to copy the dart-doc from Map. These classes appear
in
> the dart-doc separately and don't inherit the doc from there.
> Ditto for all other constructors.

Done.

https://codereview.chromium.org/18072004/diff/5001/sdk/lib/collection/maps.dart
File sdk/lib/collection/maps.dart (right):

https://codereview.chromium.org/18072004/diff/5001/sdk/lib/collection/maps.da...
sdk/lib/collection/maps.dart:84: * Helper to fill a map with key/value pairs
computed from [iterable].
On 2013/06/27 16:04:32, floitsch wrote:
> Fills a map ...
> 
> Since it's a private method it cannot be much else than a helper.

Done.

https://codereview.chromium.org/18072004/diff/5001/sdk/lib/collection/maps.da...
sdk/lib/collection/maps.dart:88: static void _fillMapWithMappedIterable(Map<K,V>
map, Iterable<K> iterable,
On 2013/06/27 16:04:32, floitsch wrote:
> Don't use named arguments for internal helper methods. Pass them directly.
It's
> cheaper.

Done.

[zarah, 2013-06-27 17:03:01]

Committed patchset #4 manually as r24531 (presubmit successful).