CombinedMapView does not maintain keys uniqueness

[renatoathaydes]

The following test fails:

final map = CombinedMapView([{'1': 1, '2': 2, '3': 3}, {'2': 22, '4': 44}]);
expect(map.length, equals(4));
expect(map.keys.toList(), equals(['1', '2', '3', '4']));

The first expectation fails because length returns 5.
The second, because keys returns ['1', '2', '3', '2', '4'].

This seems to violate Map's interface, which says:

There is a finite number of keys in the map, and each key has exactly one value associated with it.

This was a problem for me because I needed something that looks just like a normal Map but based on two other Maps, and which lets a map coming first "override" values on the map coming later in the list of maps. I believe that's the main purpose of combining maps (possibly unmodifiable), but due to this issue, it can't be used like that.

[renatoathaydes]

My workaround was creating my own type on top of it:

class SnapshotMap<K, V> extends CombinedMapView<K, V> {
  Iterable<K> Function() _lazyKeys;

  SnapshotMap(Map<K, V> topMap, Map<K, V> bottomMap)
      : super([topMap, bottomMap]) {
    _lazyKeys = lazy(() => super.keys.toSet());
  }

  UnmodifiableMapView<K, V> get view => UnmodifiableMapView(this);

  @override
  Iterable<K> get keys => _lazyKeys();

  @override
  int get length => keys.length;
}

[jakemac53]

Ya, I am going to see if I can maintain the lazy behavior of the current implementation though if possible. Otherwise this would likely break some projects.

[renatoathaydes]

No the prettiest thing, but my lazy function above:

T Function() lazy<T>(T Function() f) {
  T instance;
  return () {
    if (instance == null) {
      instance = f();
      if (instance == null) {
        throw StateError('lazy function must not return null');
      }
    }
    return instance;
  };
}

[jakemac53]

The problem there is the toSet call ends up eagerly iterating all the keys of all the maps as soon as you ask for the first key - I have a simple implementation coming though.

[jakemac53]

Note that in adding the test for this I also noticed forEach and values were broken, fun!

[jakemac53]

(these all get fixed by fixing keys though)

[renatoathaydes]

That's true, but if you avoid that you'll have to check for already-iterated keys on all maps except the first... which may be more costly?! Unless perhaps if you accumulate keys as you see them into a set?

[jakemac53]

Yes I am accumulating them as i see them, the impl is actually really easy/small.

[renatoathaydes]

@jakemac53 is this going to be released soon?

[jakemac53]

Yes, should be very soon (waiting for an existing publisher to add me as an uploader or publish it themselves).

[jakemac53]

This is now published as version 1.14.12