From 74fe213eee743ea85bcdd98a8ab68552872ff28b Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Fri, 16 Mar 2018 11:59:19 -0700 Subject: [PATCH] Add some real Dart 2 implementations to LruMap --- CHANGELOG.md | 5 +++ lib/src/collection/lru_map.dart | 72 ++++++++++++++++++------------- test/collection/lru_map_test.dart | 72 +++++++++++++++++++++++++++++-- 3 files changed, 116 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1dfb927b..2c4a4e1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +#### Master + + * New: LruMap now includes a real implementation of `addEntries`, `get + entries`, `removeWhere`, `update`, and `updateAll`. + #### 0.28.0 - 2018-01-19 * BREAKING CHANGE: The signature of `MultiMap`'s `update` stub has changed diff --git a/lib/src/collection/lru_map.dart b/lib/src/collection/lru_map.dart index d0e7813d..aacc9cac 100644 --- a/lib/src/collection/lru_map.dart +++ b/lib/src/collection/lru_map.dart @@ -68,9 +68,9 @@ class LinkedLruHashMap implements LruMap { /// Adds all key-value pairs of [other] to this map. /// - /// The operation is equivalent to doing this[key] = value for each key and - /// associated value in other. It iterates over other, which must therefore - /// not change during the iteration. + /// The operation is equivalent to doing `this[key] = value` for each key and + /// associated value in [other]. It iterates over [other], which must + /// therefore not change during the iteration. /// /// If a key of [other] is already in this map, its value is overwritten. If /// the number of unique keys is greater than [maximumSize] then the least @@ -80,13 +80,8 @@ class LinkedLruHashMap implements LruMap { void addAll(Map other) => other.forEach((k, v) => this[k] = v); @override - // TODO: Dart 2.0 requires this method to be implemented. - // ignore: override_on_non_overriding_method - void addEntries(Iterable entries) { - // Change Iterable to Iterable> when - // the MapEntry class has been added. - throw new UnimplementedError("addEntries"); - } + void addEntries(Iterable> entries) => + entries.forEach((entry) => this[entry.key] = entry.value); @override // TODO: Dart 2.0 requires this method to be implemented. @@ -108,13 +103,8 @@ class LinkedLruHashMap implements LruMap { bool containsValue(Object value) => values.contains(value); @override - // TODO: Dart 2.0 requires this method to be implemented. - // ignore: override_on_non_overriding_getter - Iterable get entries { - // Change Iterable to Iterable> when - // the MapEntry class has been added. - throw new UnimplementedError("entries"); - } + Iterable> get entries => + _entries.values.map((entry) => new MapEntry(entry.key, entry.value)); /// Applies [action] to each key-value pair of the map in order of MRU to /// LRU. @@ -195,10 +185,12 @@ class LinkedLruHashMap implements LruMap { return entry.value; } - /// Get the value for a [key] in the [Map]. The [key] will be promoted to the - /// 'Most Recently Used' position. *NOTE*: Calling [] inside an iteration - /// over keys/values is currently unsupported; use [keys] or [values] if you - /// need information about entries without modifying their position. + /// Get the value for a [key] in the [Map]. + /// The [key] will be promoted to the 'Most Recently Used' position. + /// + /// *NOTE*: Calling [] inside an iteration over keys/values is currently + /// unsupported; use [keys] or [values] if you need information about entries + /// without modifying their position. @override V operator [](Object key) { final entry = _entries[key]; @@ -247,10 +239,14 @@ class LinkedLruHashMap implements LruMap { } @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"); + var keysToRemove = []; + _entries.forEach((key, entry) { + if (test(key, entry.value)) keysToRemove.add(key); + }); + for (var key in keysToRemove) { + remove(key); + } } @override @@ -291,17 +287,33 @@ class LinkedLruHashMap implements LruMap { } @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"); + V newValue; + if (this.containsKey(key)) { + newValue = update(this[key]); + } else { + if (ifAbsent == null) + throw new ArgumentError.value(key, 'key', 'Key not in map'); + newValue = ifAbsent(); + } + + // Add this item to the MRU position. + _insertMru(_createEntry(key, newValue)); + + // Remove the LRU item if the size would be exceeded by adding this item. + if (length > maximumSize) { + assert(length == maximumSize + 1); + _removeLru(); + } + return newValue; } @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"); + _entries.forEach((key, entry) { + var newValue = _createEntry(key, update(key, entry.value)); + _entries[key] = newValue; + }); } /// Moves [entry] to the MRU position, shifting the linked list if necessary. diff --git a/test/collection/lru_map_test.dart b/test/collection/lru_map_test.dart index 25150dcb..5baa02a9 100644 --- a/test/collection/lru_map_test.dart +++ b/test/collection/lru_map_test.dart @@ -61,6 +61,34 @@ void main() { expect(lruMap['B'], 'Bravo'); }); + test('updating values on existing keys works, and promotes the key', () { + lruMap = new LruMap() + ..addAll({'A': 'Alpha', 'B': 'Beta', 'C': 'Charlie'}); + + lruMap.update('B', (v) => '$v$v'); + expect(lruMap.keys.toList(), ['B', 'C', 'A']); + expect(lruMap['B'], 'BetaBeta'); + }); + + test('updating values on absent keys works, and promotes the key', () { + lruMap = new LruMap() + ..addAll({'A': 'Alpha', 'B': 'Beta', 'C': 'Charlie'}); + + lruMap.update('D', (v) => '$v$v', ifAbsent: () => 'Delta'); + expect(lruMap.keys.toList(), ['D', 'C', 'B', 'A']); + expect(lruMap['D'], 'Delta'); + }); + + test('updating all values works, and does not change used order', () { + lruMap = new LruMap() + ..addAll({'A': 'Alpha', 'B': 'Beta', 'C': 'Charlie'}); + lruMap.updateAll((k, v) => '$v$v'); + expect(lruMap.keys.toList(), ['C', 'B', 'A']); + expect(lruMap['A'], 'AlphaAlpha'); + expect(lruMap['B'], 'BetaBeta'); + expect(lruMap['C'], 'CharlieCharlie'); + }); + test('the least recently used key is evicted when capacity hit', () { lruMap = new LruMap(maximumSize: 3) ..addAll({'A': 'Alpha', 'B': 'Beta', 'C': 'Charlie'}); @@ -159,6 +187,36 @@ void main() { expect(lruMap.values.toList(), ['Charlie', 'Beta', 'Alpha']); }); + test('`get entries` returns all entries', () { + lruMap = new LruMap() + ..addAll({'A': 'Alpha', 'B': 'Beta', 'C': 'Charlie'}); + + var entries = lruMap.entries; + expect(entries, hasLength(3)); + // MapEntry objects are not equal to each other; cannot use `contains`. :( + expect(entries.singleWhere((e) => e.key == 'A').value, equals('Alpha')); + expect(entries.singleWhere((e) => e.key == 'B').value, equals('Beta')); + expect(entries.singleWhere((e) => e.key == 'C').value, equals('Charlie')); + }); + + test('addEntries adds items to the beginning', () { + lruMap = new LruMap() + ..addAll({'A': 'Alpha', 'B': 'Beta', 'C': 'Charlie'}); + + var entries = [new MapEntry('D', 'Delta'), new MapEntry('E', 'Echo')]; + lruMap.addEntries(entries); + expect(lruMap.keys.toList(), ['E', 'D', 'C', 'B', 'A']); + }); + + test('addEntries adds existing items to the beginning', () { + lruMap = new LruMap() + ..addAll({'A': 'Alpha', 'B': 'Beta', 'C': 'Charlie'}); + + var entries = [new MapEntry('B', 'Bravo'), new MapEntry('E', 'Echo')]; + lruMap.addEntries(entries); + expect(lruMap.keys.toList(), ['E', 'B', 'C', 'A']); + }); + test('Re-adding the head entry is a no-op', () { // See: https://github.com/google/quiver-dart/issues/357 lruMap = new LruMap(); @@ -207,11 +265,19 @@ void main() { lruMap.remove('B'); expect(lruMap.keys.toList(), ['C', 'A']); }); + + test('can removeWhere items', () { + lruMap.removeWhere((k, v) => v.contains('h')); + expect(lruMap.keys.toList(), ['B']); + }); + + test('can removeWhere without changing order', () { + lruMap.removeWhere((k, v) => v.contains('A')); + expect(lruMap.keys.toList(), ['C', 'B']); + }); }); - test( - 'Test that the linked list is correctly mutated when promoting an element in the middle', - () { + test('the linked list is mutated when promoting an item in the middle', () { LruMap lruMap = new LruMap(maximumSize: 3) ..addAll({'C': 1, 'A': 1, 'B': 1}); lruMap['A'] = 1;