Add MapBase and UnmodifiableMapView classes to dart:collection.

Add MapBase and UnmodifiableMapView classes to dart:collection.

Includes supporting classes.

R=floitsch@google.com

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

[floitsch, 2014-03-25 21:03:18]

FYI.

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

https://codereview.chromium.org/211223002/diff/1/sdk/lib/collection/maps.dart...
sdk/lib/collection/maps.dart:20: * A more efficient implementation is usually
possible by also overriding
also ... as well
Remove either "also" or "as well".

https://codereview.chromium.org/211223002/diff/1/sdk/lib/collection/maps.dart...
sdk/lib/collection/maps.dart:94: class MapBaseValueIterable<V> extends
IterableBase<V>
Do we need to make this public?

https://codereview.chromium.org/211223002/diff/1/sdk/lib/collection/maps.dart...
sdk/lib/collection/maps.dart:115: class MapBaseValueIterator<V> implements
Iterator<V> {
ditto.

https://codereview.chromium.org/211223002/diff/1/sdk/lib/collection/maps.dart...
sdk/lib/collection/maps.dart:137: class UnmodifiableMapMixin<K, V> implements
Map<K, V> {
I know this comes from pkg:collection where I reviewed the code, but ...
This feels wrong: a MapMixin should implement Map operations, and not remove
them.
I would expect an UnmodifiableMapMixin to implement Map except for some abstract
functions and some throwing functions.

[kevmoo, 2014-03-25 21:25:26]

DBC

Also runtime/lib/mirrors_impl.dart:13:class _UnmodifiableMapView<K, V>

Since this will conflict with the class in pkg/collection It'd be great to align
this change with a release there with the corresponding SDK constraint to avoid
collisions.

https://codereview.chromium.org/211223002/diff/1/sdk/lib/_internal/lib/js_mir...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/211223002/diff/1/sdk/lib/_internal/lib/js_mir...
sdk/lib/_internal/lib/js_mirrors.dart:1: // Copyright (c) 2013, the Dart project
authors.  Please see the AUTHORS file
Is there meant to be a delta here?

[floitsch, 2014-03-25 23:35:16]

On 2014/03/25 21:25:26, kevmoo wrote:
> DBC
> 
> Also runtime/lib/mirrors_impl.dart:13:class _UnmodifiableMapView<K, V>
> 
> Since this will conflict with the class in pkg/collection It'd be great to
align
> this change with a release there with the corresponding SDK constraint to
avoid
> collisions.
Normally the dart:libraries should just be shadowed by the pkg classes. I could
be wrong, though.
> 
>
https://codereview.chromium.org/211223002/diff/1/sdk/lib/_internal/lib/js_mir...
> File sdk/lib/_internal/lib/js_mirrors.dart (right):
> 
>
https://codereview.chromium.org/211223002/diff/1/sdk/lib/_internal/lib/js_mir...
> sdk/lib/_internal/lib/js_mirrors.dart:1: // Copyright (c) 2013, the Dart
project
> authors.  Please see the AUTHORS file
> Is there meant to be a delta here?

[Lasse Reichstein Nielsen, 2014-03-26 10:25:19]

PTAL.

Florian is right. There should be no conflict between a dart:-library and a
package:-library. The latter overrides the former without a conflict.

We should change the package:collection library to use the UnmodifiableMapView
from dart:collection eventually, but there is no rush.

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

https://codereview.chromium.org/211223002/diff/1/sdk/lib/collection/maps.dart...
sdk/lib/collection/maps.dart:20: * A more efficient implementation is usually
possible by also overriding
On 2014/03/25 21:03:18, floitsch wrote:
> also ... as well
> Remove either "also" or "as well".

Done.

https://codereview.chromium.org/211223002/diff/1/sdk/lib/collection/maps.dart...
sdk/lib/collection/maps.dart:94: class MapBaseValueIterable<V> extends
IterableBase<V>
No. I've made this, the iterator and the mixin private.

https://codereview.chromium.org/211223002/diff/1/sdk/lib/collection/maps.dart...
sdk/lib/collection/maps.dart:115: class MapBaseValueIterator<V> implements
Iterator<V> {
On 2014/03/25 21:03:18, floitsch wrote:
> ditto.

Done.

https://codereview.chromium.org/211223002/diff/1/sdk/lib/collection/maps.dart...
sdk/lib/collection/maps.dart:137: class UnmodifiableMapMixin<K, V> implements
Map<K, V> {
This is now private, so it's only used internally.

[floitsch, 2014-03-26 10:27:02]

Please upload new version.

[Lasse Reichstein Nielsen, 2014-03-26 11:46:43]

Done.

[Lasse Reichstein Nielsen, 2014-03-26 11:48:45]

https://codereview.chromium.org/211223002/diff/1/sdk/lib/_internal/lib/js_mir...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/211223002/diff/1/sdk/lib/_internal/lib/js_mir...
sdk/lib/_internal/lib/js_mirrors.dart:1: // Copyright (c) 2013, the Dart project
authors.  Please see the AUTHORS file
Yes. Extra empty line detected and eliminated.

If there should be a separator between the non-throwing and the throwing
members, it should probably be a comment, not just an extra empty line.

[floitsch, 2014-03-26 12:00:29]

LGTM.

[Lasse Reichstein Nielsen, 2014-03-26 12:10:41]

Committed patchset #2 manually as r34422 (presubmit successful).