Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new Dart 2 Map methods to BiMap #423

Merged
merged 5 commits into from
Mar 23, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
#### Master

* New: BiMap now includes a real implementation of `addEntries`, `get
entries`, `map`, `removeWhere`, `update`, and `updateAll`.

#### 0.28.0 - 2018-01-19

* BREAKING CHANGE: The signature of `MultiMap`'s `update` stub has changed
Expand Down
67 changes: 33 additions & 34 deletions lib/src/collection/bimap.dart
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,18 @@ class HashBiMap<K, V> implements BiMap<K, V> {
int get length => _map.length;
Iterable<V> get values => _inverse.keys;

BiMap<V, K> get inverse {
if (_cached == null) {
_cached = new HashBiMap._from(_inverse, _map);
}
return _cached;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With null-aware assignment:

BiMap<V, K> get inverse => _cached ??= new HashBiMap._from(_inverse, _map);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it. Done.


@override
// TODO: Dart 2.0 requires this method to be implemented.
// ignore: override_on_non_overriding_method
void addEntries(Iterable<Object> entries) {
// Change Iterable<Object> to Iterable<MapEntry<K, V>> when
// the MapEntry class has been added.
throw new UnimplementedError("addEntries");
void addEntries(Iterable<MapEntry<K, V>> entries) {
for (var entry in entries) {
this[entry.key] = entry.value;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implement this using _add since it should apply the same invariants.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}
}

@override
Expand All @@ -87,28 +92,18 @@ class HashBiMap<K, V> implements BiMap<K, V> {
}

@override
// TODO: Dart 2.0 requires this method to be implemented.
// ignore: override_on_non_overriding_getter
Iterable<Null> get entries {
// Change Iterable<Null> to Iterable<MapEntry<K, V>> when
// the MapEntry class has been added.
throw new UnimplementedError("entries");
}

BiMap<V, K> get inverse {
if (_cached == null) {
_cached = new HashBiMap._from(_inverse, _map);
}
return _cached;
Iterable<MapEntry<K, V>> get entries {
return keys.map((K key) => new MapEntry<K, V>(key, this[key]));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not returning _map.entries ?

Iterable<MapEntry<K, V>> get entries => _map.entries;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}

@override
// TODO: Dart 2.0 requires this method to be implemented.
// ignore: override_on_non_overriding_method
Map<K2, V2> map<K2, V2>(Object transform(K key, V value)) {
// Change Object to MapEntry<K2, V2> when
// the MapEntry class has been added.
throw new UnimplementedError("map");
Map<K2, V2> map<K2, V2>(MapEntry<K2, V2> transform(K key, V value)) {
var result = <K2, V2>{};
for (var key in this.keys) {
var entry = transform(key, this[key]);
result[entry.key] = entry.value;
}
return result;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not returning _map.map(transform) ?

  Map<K2, V2> map<K2, V2>(MapEntry<K2, V2> transform(K key, V value)) {
    return _map.map(transform);
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}

V putIfAbsent(K key, V ifAbsent()) {
Expand All @@ -124,10 +119,9 @@ class HashBiMap<K, V> implements BiMap<K, V> {
}

@override
// TODO: Dart 2.0 requires this method to be implemented.
// ignore: override_on_non_overriding_method
void removeWhere(bool test(K key, V value)) {
throw new UnimplementedError("removeWhere");
_map.removeWhere(test);
_inverse.removeWhere((v, k) => test(k, v));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Let's do this in the same order as remove... not that it really matters :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}

@override
Expand All @@ -138,17 +132,22 @@ class HashBiMap<K, V> implements BiMap<K, V> {
}

@override
// TODO: Dart 2.0 requires this method to be implemented.
// ignore: override_on_non_overriding_method
V update(K key, V update(V value), {V ifAbsent()}) {
throw new UnimplementedError("update");
var value = _map[key];
if (value != null) {
return _add(key, update(value), true);
} else {
if (ifAbsent == null)
throw new ArgumentError.value(key, 'key', 'Key not in map');
return _add(key, ifAbsent(), false);
}
}

@override
// TODO: Dart 2.0 requires this method to be implemented.
// ignore: override_on_non_overriding_method
void updateAll(V update(K key, V value)) {
throw new UnimplementedError("updateAll");
for (var key in this.keys) {
_add(key, update(key, _map[key]), true);
}
}

void clear() {
Expand Down
93 changes: 93 additions & 0 deletions test/collection/bimap_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -161,13 +161,63 @@ main() {
map.remove(k1);
expect(map.containsKey(k1), false);
expect(map.inverse.containsKey(v1), false);

map[k1] = v1;
map[k2] = v2;
map.removeWhere((k, v) => v.isOdd);
expect(map.containsKey(k1), false);
expect(map.containsKey(k2), true);
expect(map.inverse.containsKey(v1), false);
expect(map.inverse.containsKey(v2), true);
});

test('should not contain mappings removed from its inverse', () {
map[k1] = v1;
map.inverse.remove(v1);
expect(map.containsKey(k1), false);
expect(map.inverse.containsKey(v1), false);

map[k1] = v1;
map[k2] = v2;
map.inverse.removeWhere((v, k) => v.isOdd);
expect(map.containsKey(k1), false);
expect(map.containsKey(k2), true);
expect(map.inverse.containsKey(v1), false);
expect(map.inverse.containsKey(v2), true);
});

test('should update both sides', () {
map[k1] = v1;
map.update(k1, (v) => v + 1);
expect(map[k1], equals(v1 + 1));
expect(map.inverse[v1 + 1], equals(k1));

map[k1] = v1;
map.inverse.update(v1, (k) => '_$k');
expect(map['_$k1'], equals(v1));
expect(map.inverse[v1], equals('_$k1'));
});

test('should update absent key values', () {
map[k1] = v1;
map.update(k2, (v) => v + 1, ifAbsent: () => 0);
expect(map[k2], equals(0));
expect(map.inverse[0], equals(k2));

map[k1] = v1;
map.inverse.update(v2, (k) => '_$k', ifAbsent: () => '_X');
expect(map['_X'], equals(v2));
expect(map.inverse[v2], equals('_X'));
});

test('should update all values', () {
map[k1] = v1;
map[k2] = v2;
map.updateAll((k, v) => v + k.length);
expect(map[k1], equals(3));
expect(map[k2], equals(4));
expect(map.inverse[3], equals(k1));
expect(map.inverse[4], equals(k2));
});

test('should be empty after clear', () {
Expand Down Expand Up @@ -230,6 +280,49 @@ main() {
expect(map.inverse.values, unorderedEquals([k1, k2]));
});

test('should add entries', () {
map.addEntries([new MapEntry<String, int>(k1, v1)]);
expect(map[k1], equals(v1));
expect(map.inverse[v1], equals(k1));

map.inverse.addEntries([new MapEntry<int, String>(v2, k2)]);
expect(map[k2], equals(v2));
expect(map.inverse[v2], equals(k2));
});

test('should get entries', () {
map[k1] = v1;
map.inverse[v2] = k2;

var mapEntries = map.entries;
expect(mapEntries, hasLength(2));
// MapEntry objects are not equal to each other; cannot use `contains`. :(
expect(mapEntries.singleWhere((e) => e.key == k1).value, equals(v1));
expect(mapEntries.singleWhere((e) => e.key == k2).value, equals(v2));

var inverseEntries = map.inverse.entries;
expect(inverseEntries, hasLength(2));
expect(inverseEntries.singleWhere((e) => e.key == v1).value, equals(k1));
expect(inverseEntries.singleWhere((e) => e.key == v2).value, equals(k2));
});

test('should map mappings', () {
map[k1] = v1;
map[k2] = v2;

var mapped = map.map((k, v) => new MapEntry(k.toUpperCase(), '$k / $v'));
expect(mapped, contains('K1'));
expect(mapped, contains('K2'));
expect(mapped['K1'], equals('k1 / 1'));
expect(mapped['K2'], equals('k2 / 2'));

var mapped2 = map.inverse.map((v, k) => new MapEntry('$v$v', k.length));
expect(mapped2, contains('11'));
expect(mapped2, contains('22'));
expect(mapped2['11'], equals(2));
expect(mapped2['22'], equals(2));
});

test('should add mappings via putIfAbsent if absent', () {
map.putIfAbsent(k1, () => v1);
expect(map[k1], v1);
Expand Down