Refactor observable map. (#16)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a1cc51d..3549221 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,12 @@
+## 0.19.0
+
+* Refactor and deprecate `ObservableMap`-specific API
+ * `ObservableMap` no longer emits `#keys` and `#values` change records
+ * `ObservableMap.spy` is deprecated, becomes `.delegate` instead
+* Potentially breaking: `ObservableMap` may no longer be extended
+
+It is also considered deprecated to be notified of `length` changes.
+
## 0.18.1
* Bug fix: Do not throw when `Observable<T>.notifyChange` is used
diff --git a/lib/observable.dart b/lib/observable.dart
index a2d1fca..641fa7b 100644
--- a/lib/observable.dart
+++ b/lib/observable.dart
@@ -5,10 +5,9 @@
library observable;
export 'src/change_notifier.dart' show ChangeNotifier, PropertyChangeNotifier;
-export 'src/collections.dart' show ObservableList;
+export 'src/collections.dart' show ObservableList, ObservableMap;
export 'src/differs.dart' show Differ, EqualityDiffer, ListDiffer, MapDiffer;
export 'src/records.dart'
show ChangeRecord, ListChangeRecord, MapChangeRecord, PropertyChangeRecord;
export 'src/observable.dart';
-export 'src/observable_map.dart';
export 'src/to_observable.dart';
diff --git a/lib/src/collections.dart b/lib/src/collections.dart
index 7aaff1d..363e925 100644
--- a/lib/src/collections.dart
+++ b/lib/src/collections.dart
@@ -1 +1,2 @@
export 'collections/observable_list.dart';
+export 'collections/observable_map.dart';
diff --git a/lib/src/collections/observable_list.dart b/lib/src/collections/observable_list.dart
index 5cc643a..1052df2 100644
--- a/lib/src/collections/observable_list.dart
+++ b/lib/src/collections/observable_list.dart
@@ -10,12 +10,14 @@
/// efficient to notify _when_ something has changed, instead of constantly
/// diffing lists to find a single change (like an inserted record). You may
/// accept an observable list to be notified of mutations:
+/// ```
/// set names(List<String> names) {
/// clearAndWrite(names);
/// if (names is ObservableList<String>) {
/// names.listChanges.listen(smallIncrementalUpdate);
/// }
/// }
+/// ```
///
/// *See [ListDiffer] to manually diff two lists instead*
abstract class ObservableList<E> implements List<E>, Observable {
diff --git a/lib/src/collections/observable_map.dart b/lib/src/collections/observable_map.dart
new file mode 100644
index 0000000..50fac80
--- /dev/null
+++ b/lib/src/collections/observable_map.dart
@@ -0,0 +1,181 @@
+import 'dart:async';
+import 'dart:collection';
+
+import 'package:collection/collection.dart';
+import 'package:observable/observable.dart';
+
+/// A [Map] that broadcasts [changes] to subscribers for efficient mutations.
+///
+/// When client code expects a read heavy/write light workload, it is often more
+/// efficient to notify _when_ something has changed, instead of constantly
+/// diffing lists to find a single change (like an updated key-value). You may
+/// accept an observable map to be notified of mutations:
+/// ```
+/// set grades(Map<String, int> grades) {
+/// buildBook(grades);
+/// if (names is ObservableMap<String>, int) {
+/// grades.changes.listen(updateBook);
+/// }
+/// }
+/// ```
+///
+/// *See [MapDiffer] to manually diff two lists instead*
+abstract class ObservableMap<K, V> implements Map<K, V>, Observable {
+ /// Creates a new observable map.
+ factory ObservableMap() {
+ return new _ObservableDelegatingMap(new HashMap<K, V>());
+ }
+
+ /// Like [ObservableMap.from], but creates an empty type.
+ factory ObservableMap.createFromType(Map<K, V> other) {
+ ObservableMap<K, V> result;
+ if (other is LinkedHashMap) {
+ result = new ObservableMap<K, V>.linked();
+ } else if (other is SplayTreeMap) {
+ result = new ObservableMap<K, V>.sorted();
+ } else {
+ result = new ObservableMap<K, V>();
+ }
+ return result;
+ }
+
+ /// Create a new observable map using [map] as a backing store.
+ factory ObservableMap.delegate(Map<K, V> map) {
+ return new _ObservableDelegatingMap<K, V>(map);
+ }
+
+ /// Creates a new observable map that contains all entries in [other].
+ ///
+ /// It will attempt to use the same backing map type if the other map is
+ /// either a [LinkedHashMap], [SplayTreeMap], or [HashMap]. Otherwise it will
+ /// fall back to using a [HashMap].
+ factory ObservableMap.from(Map<K, V> other) {
+ return new ObservableMap<K, V>.createFromType(other)..addAll(other);
+ }
+
+ /// Creates a new observable map using a [LinkedHashMap].
+ factory ObservableMap.linked() {
+ return new _ObservableDelegatingMap<K, V>(new LinkedHashMap<K, V>());
+ }
+
+ /// Creates a new observable map using a [SplayTreeMap].
+ factory ObservableMap.sorted() {
+ return new _ObservableDelegatingMap<K, V>(new SplayTreeMap<K, V>());
+ }
+
+ /// Creates a new observable map wrapping [other].
+ @Deprecated('Use ObservableMap.delegate for API consistency')
+ factory ObservableMap.spy(Map<K, V> other) = ObservableMap<K, V>.delegate;
+}
+
+class _ObservableDelegatingMap<K, V> extends DelegatingMap<K, V>
+ implements ObservableMap<K, V> {
+ final _allChanges = new ChangeNotifier();
+
+ _ObservableDelegatingMap(Map<K, V> map) : super(map);
+
+ // Observable
+
+ @override
+ Stream<List<ChangeRecord>> get changes => _allChanges.changes;
+
+ // ChangeNotifier (deprecated for ObservableMap)
+
+ @override
+ bool deliverChanges() => _allChanges.deliverChanges();
+
+ @override
+ bool get hasObservers => _allChanges.hasObservers;
+
+ @override
+ void notifyChange([ChangeRecord change]) {
+ _allChanges.notifyChange(change);
+ }
+
+ @override
+ /*=T*/ notifyPropertyChange/*<T>*/(
+ Symbol field,
+ /*=T*/
+ oldValue,
+ /*=T*/
+ newValue,
+ ) {
+ if (oldValue != newValue) {
+ _allChanges.notifyChange(
+ new PropertyChangeRecord/*<T>*/(this, field, oldValue, newValue),
+ );
+ }
+ return newValue;
+ }
+
+ @override
+ void observed() {}
+
+ @override
+ void unobserved() {}
+
+ @override
+ operator []=(K key, V newValue) {
+ if (!hasObservers) {
+ super[key] = newValue;
+ return;
+ }
+
+ final oldLength = super.length;
+ V oldValue = super[key];
+ super[key] = newValue;
+
+ if (oldLength != length) {
+ notifyPropertyChange(#length, oldLength, length);
+ notifyChange(new MapChangeRecord<K, V>.insert(key, newValue));
+ } else {
+ notifyChange(new MapChangeRecord<K, V>(key, oldValue, newValue));
+ }
+ }
+
+ @override
+ void addAll(Map<K, V> other) {
+ if (!hasObservers) {
+ super.addAll(other);
+ return;
+ }
+
+ other.forEach((k, v) => this[k] = v);
+ }
+
+ @override
+ V putIfAbsent(K key, V ifAbsent()) {
+ final oldLength = length;
+ final result = super.putIfAbsent(key, ifAbsent);
+ if (hasObservers && oldLength != length) {
+ notifyPropertyChange(#length, oldLength, length);
+ notifyChange(new MapChangeRecord<K, V>.insert(key, result));
+ }
+ return result;
+ }
+
+ @override
+ V remove(Object key) {
+ final oldLength = length;
+ final result = super.remove(key);
+ if (hasObservers && oldLength != length) {
+ notifyChange(new MapChangeRecord<K, V>.remove(key as K, result));
+ notifyPropertyChange(#length, oldLength, length);
+ }
+ return result;
+ }
+
+ @override
+ void clear() {
+ if (!hasObservers || isEmpty) {
+ super.clear();
+ return;
+ }
+ final oldLength = length;
+ forEach((k, v) {
+ notifyChange(new MapChangeRecord<K, V>.remove(k, v));
+ });
+ notifyPropertyChange(#length, oldLength, 0);
+ super.clear();
+ }
+}
diff --git a/lib/src/observable_map.dart b/lib/src/observable_map.dart
deleted file mode 100644
index caebbda..0000000
--- a/lib/src/observable_map.dart
+++ /dev/null
@@ -1,167 +0,0 @@
-// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-
-library observable.src.observable_map;
-
-import 'dart:collection';
-
-import 'observable.dart';
-import 'records.dart';
-import 'to_observable.dart';
-
-// TODO(jmesserly): this needs to be faster. We currently require multiple
-// lookups per key to get the old value.
-// TODO(jmesserly): this doesn't implement the precise interfaces like
-// LinkedHashMap, SplayTreeMap or HashMap. However it can use them for the
-// backing store.
-
-/// Represents an observable map of model values. If any items are added,
-/// removed, or replaced, then observers that are listening to [changes]
-/// will be notified.
-class ObservableMap<K, V> extends Observable implements Map<K, V> {
- final Map<K, V> _map;
-
- /// Creates an observable map.
- ObservableMap() : _map = new HashMap<K, V>();
-
- /// Creates a new observable map using a [LinkedHashMap].
- ObservableMap.linked() : _map = new LinkedHashMap<K, V>();
-
- /// Creates a new observable map using a [SplayTreeMap].
- ObservableMap.sorted() : _map = new SplayTreeMap<K, V>();
-
- /// Creates an observable map that contains all key value pairs of [other].
- /// It will attempt to use the same backing map type if the other map is a
- /// [LinkedHashMap], [SplayTreeMap], or [HashMap]. Otherwise it defaults to
- /// [HashMap].
- ///
- /// Note this will perform a shallow conversion. If you want a deep conversion
- /// you should use [toObservable].
- factory ObservableMap.from(Map<K, V> other) {
- return new ObservableMap<K, V>.createFromType(other)..addAll(other);
- }
-
- /// Like [ObservableMap.from], but creates an empty map.
- factory ObservableMap.createFromType(Map<K, V> other) {
- ObservableMap<K, V> result;
- if (other is SplayTreeMap) {
- result = new ObservableMap<K, V>.sorted();
- } else if (other is LinkedHashMap) {
- result = new ObservableMap<K, V>.linked();
- } else {
- result = new ObservableMap<K, V>();
- }
- return result;
- }
-
- /// Creates a new observable map wrapping [other].
- ObservableMap.spy(Map<K, V> other) : _map = other;
-
- @override
- Iterable<K> get keys => _map.keys;
-
- @override
- Iterable<V> get values => _map.values;
-
- @override
- int get length => _map.length;
-
- @override
- bool get isEmpty => length == 0;
-
- @override
- bool get isNotEmpty => !isEmpty;
-
- @override
- bool containsValue(Object value) => _map.containsValue(value);
-
- @override
- bool containsKey(Object key) => _map.containsKey(key);
-
- @override
- V operator [](Object key) => _map[key];
-
- @override
- void operator []=(K key, V value) {
- if (!hasObservers) {
- _map[key] = value;
- return;
- }
-
- int len = _map.length;
- V oldValue = _map[key];
-
- _map[key] = value;
-
- if (len != _map.length) {
- notifyPropertyChange(#length, len, _map.length);
- notifyChange(new MapChangeRecord.insert(key, value));
- _notifyKeysValuesChanged();
- } else if (oldValue != value) {
- notifyChange(new MapChangeRecord(key, oldValue, value));
- _notifyValuesChanged();
- }
- }
-
- @override
- void addAll(Map<K, V> other) {
- other.forEach((K key, V value) {
- this[key] = value;
- });
- }
-
- @override
- V putIfAbsent(K key, V ifAbsent()) {
- int len = _map.length;
- V result = _map.putIfAbsent(key, ifAbsent);
- if (hasObservers && len != _map.length) {
- notifyPropertyChange(#length, len, _map.length);
- notifyChange(new MapChangeRecord.insert(key, result));
- _notifyKeysValuesChanged();
- }
- return result;
- }
-
- @override
- V remove(Object key) {
- int len = _map.length;
- V result = _map.remove(key);
- if (hasObservers && len != _map.length) {
- notifyChange(new MapChangeRecord.remove(key, result));
- notifyPropertyChange(#length, len, _map.length);
- _notifyKeysValuesChanged();
- }
- return result;
- }
-
- @override
- void clear() {
- int len = _map.length;
- if (hasObservers && len > 0) {
- _map.forEach((key, value) {
- notifyChange(new MapChangeRecord.remove(key, value));
- });
- notifyPropertyChange(#length, len, 0);
- _notifyKeysValuesChanged();
- }
- _map.clear();
- }
-
- @override
- void forEach(void f(K key, V value)) => _map.forEach(f);
-
- @override
- String toString() => Maps.mapToString(this);
-
- // Note: we don't really have a reasonable old/new value to use here.
- // But this should fix "keys" and "values" in templates with minimal overhead.
- void _notifyKeysValuesChanged() {
- notifyChange(new PropertyChangeRecord(this, #keys, null, null));
- _notifyValuesChanged();
- }
-
- void _notifyValuesChanged() {
- notifyChange(new PropertyChangeRecord(this, #values, null, null));
- }
-}
diff --git a/lib/src/to_observable.dart b/lib/src/to_observable.dart
index d117c89..b4cdaf0 100644
--- a/lib/src/to_observable.dart
+++ b/lib/src/to_observable.dart
@@ -7,8 +7,7 @@
import 'dart:collection';
import 'observable.dart' show Observable;
-import 'collections.dart' show ObservableList;
-import 'observable_map.dart' show ObservableMap;
+import 'collections.dart' show ObservableList, ObservableMap;
/// Converts the [Iterable] or [Map] to an [ObservableList] or [ObservableMap],
/// respectively. This is a convenience function to make it easier to convert
diff --git a/pubspec.yaml b/pubspec.yaml
index 7519461..8065fa9 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -1,5 +1,5 @@
name: observable
-version: 0.18.1
+version: 0.19.0
author: Dart Team <misc@dartlang.org>
description: Support for marking objects as observable
homepage: https://github.com/dart-lang/observable
diff --git a/test/observable_map_test.dart b/test/observable_map_test.dart
index 04aeccc..5e369d5 100644
--- a/test/observable_map_test.dart
+++ b/test/observable_map_test.dart
@@ -191,79 +191,6 @@
expect(map.toString(), '{a: 1, b: 2}');
});
- group('observe keys/values', () {
- ObservableMap map;
- int keysChanged;
- int valuesChanged;
-
- setUp(() {
- map = toObservable({'a': 1, 'b': 2, 'c': 3});
- keysChanged = 0;
- valuesChanged = 0;
- sub = map.changes.listen((records) {
- keysChanged += getPropertyChangeRecords(records, #keys).length;
- valuesChanged += getPropertyChangeRecords(records, #values).length;
- });
- });
-
- tearDown(sharedTearDown);
-
- test('add item changes keys/values', () {
- map['d'] = 4;
- expect(map, {'a': 1, 'b': 2, 'c': 3, 'd': 4});
- return new Future(() {
- expect(keysChanged, 1);
- expect(valuesChanged, 1);
- });
- });
-
- test('putIfAbsent changes keys/values', () {
- map.putIfAbsent('d', () => 4);
- expect(map, {'a': 1, 'b': 2, 'c': 3, 'd': 4});
- return new Future(() {
- expect(keysChanged, 1);
- expect(valuesChanged, 1);
- });
- });
-
- test('remove changes keys/values', () {
- map.remove('c');
- map.remove('a');
- expect(map, {'b': 2});
- return new Future(() {
- expect(keysChanged, 2);
- expect(valuesChanged, 2);
- });
- });
-
- test('remove non-existent item does not change keys/values', () {
- map.remove('d');
- expect(map, {'a': 1, 'b': 2, 'c': 3});
- return new Future(() {
- expect(keysChanged, 0);
- expect(valuesChanged, 0);
- });
- });
-
- test('set existing item does not change keys', () {
- map['c'] = 9000;
- expect(map, {'a': 1, 'b': 2, 'c': 9000});
- return new Future(() {
- expect(keysChanged, 0);
- expect(valuesChanged, 1);
- });
- });
-
- test('clear changes keys/values', () {
- map.clear();
- expect(map, {});
- return new Future(() {
- expect(keysChanged, 1);
- expect(valuesChanged, 1);
- });
- });
- });
-
group('change records', () {
List<ChangeRecord> records;
ObservableMap map;
@@ -308,8 +235,6 @@
expectChanges(records, [
_lengthChange(map, 2, 3),
_insertKey('c', 3),
- _propChange(map, #keys),
- _propChange(map, #values),
]);
});
});
@@ -324,11 +249,8 @@
return new Future(() {
expectChanges(records, [
_changeKey('a', 1, 42),
- _propChange(map, #values),
_lengthChange(map, 2, 3),
_insertKey('c', 3),
- _propChange(map, #keys),
- _propChange(map, #values),
]);
});
});
@@ -341,8 +263,6 @@
expectChanges(records, [
_removeKey('b', 2),
_lengthChange(map, 2, 1),
- _propChange(map, #keys),
- _propChange(map, #values),
]);
});
});
@@ -356,8 +276,6 @@
_removeKey('a', 1),
_removeKey('b', 2),
_lengthChange(map, 2, 0),
- _propChange(map, #keys),
- _propChange(map, #values),
]);
});
});
