Add MapDiffer, cleanup MapChangeRecord (#8)

* Fix bug in list_differ

* Add MapDiffer and refactor MapChangeRecord

CHANGELOG.md

@@ -1,3 +1,10 @@
+## 0.16.0
+
+* Refactored `MapChangeRecord`
+    * Added equality and hashCode checks
+    * Added `MapChangeRecord.apply` to apply a change record
+* Added `MapDiffer`, which implements `Differ` for a `Map`
+
 ## 0.15.0+2
 
 * Fix a bug in `ListDiffer` that caused a `RangeError`

lib/observable.dart

@@ -4,8 +4,8 @@
 
 library observable;
 
-export 'src/differs.dart' show Differ, EqualityDiffer, ListDiffer;
-export 'src/records.dart' show ChangeRecord, ListChangeRecord;
+export 'src/differs.dart' show Differ, EqualityDiffer, ListDiffer, MapDiffer;
+export 'src/records.dart' show ChangeRecord, ListChangeRecord, MapChangeRecord;
 export 'src/observable.dart';
 export 'src/observable_list.dart';
 export 'src/observable_map.dart';

lib/src/differs.dart

@@ -10,14 +10,17 @@ import 'package:collection/collection.dart';
 
 import 'records.dart';
 
+import 'internal.dart';
+
 part 'differs/list_differ.dart';
+part 'differs/map_differ.dart';
 
 /// Generic comparisons between two comparable objects.
 abstract class Differ<E> {
   /// Returns a list of change records between [e1] and [e2].
   ///
   /// A return value of an empty [ChangeRecord.NONE] means no changes found.
-  List<ChangeRecord> diff(E e1, E e2);
+  List<ChangeRecord> diff(E oldValue, E newValue);
 }
 
 /// Uses [Equality] to determine a simple [ChangeRecord.ANY] response.
@@ -29,7 +32,9 @@ class EqualityDiffer<E> implements Differ<E> {
   const EqualityDiffer.identity() : this._equality = const IdentityEquality();
 
   @override
-  List<ChangeRecord> diff(E e1, E e2) {
-    return _equality.equals(e1, e2) ? ChangeRecord.NONE : ChangeRecord.ANY;
+  List<ChangeRecord> diff(E oldValue, E newValue) {
+    return _equality.equals(oldValue, newValue)
+        ? ChangeRecord.NONE
+        : ChangeRecord.ANY;
   }
 }

lib/src/differs/map_differ.dart

@@ -0,0 +1,38 @@
+// 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.
+
+part of observable.src.differs;
+
+/// Determines differences between two maps, returning [MapChangeRecord]s.
+///
+/// While [MapChangeRecord] has more information and can be replayed they carry
+/// a more significant cost to calculate and create and should only be used when
+/// the details in the record will actually be used.
+///
+/// See also [EqualityDiffer] for a simpler comparison.
+class MapDiffer<K, V> implements Differ<Map<K, V>> {
+  const MapDiffer();
+
+  @override
+  List<MapChangeRecord<K, V>> diff(Map<K, V> oldValue, Map<K, V> newValue) {
+    if (identical(oldValue, newValue)) {
+      return ChangeRecord.NONE;
+    }
+    final changes = <MapChangeRecord<K, V>>[];
+    oldValue.forEach((oldK, oldV) {
+      final newV = newValue[oldK];
+      if (newV == null && !newValue.containsKey(oldK)) {
+        changes.add(new MapChangeRecord<K, V>.remove(oldK, oldV));
+      } else if (newV != oldV) {
+        changes.add(new MapChangeRecord<K, V>(oldK, oldV, newV));
+      }
+    });
+    newValue.forEach((newK, newV) {
+      if (!oldValue.containsKey(newK)) {
+        changes.add(new MapChangeRecord<K, V>.insert(newK, newV));
+      }
+    });
+    return freezeInDevMode(changes);
+  }
+}

lib/src/internal.dart

@@ -0,0 +1,12 @@
+// 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.
+
+List/*<E>*/ freezeInDevMode/*<E>*/(List/*<E>*/ list) {
+  if (list == null) return const [];
+  assert(() {
+    list = new List/*<E>*/ .unmodifiable(list);
+    return true;
+  });
+  return list;
+}

lib/src/observable.dart

@@ -9,7 +9,6 @@ import 'dart:collection' show UnmodifiableListView;
 
 import 'package:meta/meta.dart';
 
-import 'observable_map.dart';
 import 'property_change_record.dart' show PropertyChangeRecord;
 import 'records.dart' show ChangeRecord;
 

lib/src/observable_map.dart

@@ -8,7 +8,7 @@ import 'dart:collection';
 
 import 'observable.dart' show Observable;
 import 'property_change_record.dart' show PropertyChangeRecord;
-import 'records.dart' show ChangeRecord;
+import 'records.dart' show MapChangeRecord;
 import 'to_observable.dart';
 
 // TODO(jmesserly): this needs to be faster. We currently require multiple
@@ -17,47 +17,6 @@ import 'to_observable.dart';
 // LinkedHashMap, SplayTreeMap or HashMap. However it can use them for the
 // backing store.
 
-// TODO(jmesserly): should we summarize map changes like we do for list changes?
-class MapChangeRecord<K, V> extends ChangeRecord {
-  // TODO(jmesserly): we could store this more compactly if it matters, with
-  // subtypes for inserted and removed.
-
-  /// The map key that changed.
-  final K key;
-
-  /// The previous value associated with this key.
-  final V oldValue;
-
-  /// The new value associated with this key.
-  final V newValue;
-
-  /// True if this key was inserted.
-  final bool isInsert;
-
-  /// True if this key was removed.
-  final bool isRemove;
-
-  MapChangeRecord(this.key, this.oldValue, this.newValue)
-      : isInsert = false,
-        isRemove = false;
-
-  MapChangeRecord.insert(this.key, this.newValue)
-      : isInsert = true,
-        isRemove = false,
-        oldValue = null;
-
-  MapChangeRecord.remove(this.key, this.oldValue)
-      : isInsert = false,
-        isRemove = true,
-        newValue = null;
-
-  @override
-  String toString() {
-    var kind = isInsert ? 'insert' : isRemove ? 'remove' : 'set';
-    return '#<MapChangeRecord $kind $key from: $oldValue to: $newValue>';
-  }
-}
-
 /// 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.

lib/src/records.dart

@@ -7,7 +7,10 @@ library observable.src.records;
 import 'package:collection/collection.dart';
 import 'package:quiver/core.dart';
 
+import 'internal.dart';
+
 part 'records/list_change_record.dart';
+part 'records/map_change_record.dart';
 
 /// Result of a change to an observed object.
 class ChangeRecord {

lib/src/records/list_change_record.dart

@@ -4,15 +4,6 @@
 
 part of observable.src.records;
 
-List/*<E>*/ _freezeInDevMode/*<E>*/(List/*<E>*/ list) {
-  if (list == null) return const [];
-  assert(() {
-    list = new List/*<E>*/ .unmodifiable(list);
-    return true;
-  });
-  return list;
-}
-
 /// A [ChangeRecord] that denotes adding or removing nodes at [index].
 ///
 /// It should be assumed that elements are [removed] *before* being added.
@@ -50,7 +41,7 @@ class ListChangeRecord<E> implements ChangeRecord {
 
   /// Records a `remove` operation at `object[index]` of [removed] elements.
   ListChangeRecord.remove(this.object, this.index, List<E> removed)
-      : this.removed = _freezeInDevMode/*<E>*/(removed),
+      : this.removed = freezeInDevMode/*<E>*/(removed),
         this.addedCount = 0 {
     _assertValidState();
   }
@@ -60,7 +51,7 @@ class ListChangeRecord<E> implements ChangeRecord {
   /// If [addedCount] is not specified it defaults to `removed.length`.
   ListChangeRecord.replace(this.object, this.index, List<E> removed,
       [int addedCount])
-      : this.removed = _freezeInDevMode/*<E>*/(removed),
+      : this.removed = freezeInDevMode/*<E>*/(removed),
         this.addedCount = addedCount ?? removed.length {
     _assertValidState();
   }

lib/src/records/map_change_record.dart

@@ -0,0 +1,82 @@
+// 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.
+
+part of observable.src.records;
+
+/// A [ChangeRecord] that denotes adding, removing, or updating a map.
+class MapChangeRecord<K, V> implements ChangeRecord {
+  /// The map key that changed.
+  final K key;
+
+  /// The previous value associated with this key.
+  ///
+  /// Is always `null` if [isInsert].
+  final V oldValue;
+
+  /// The new value associated with this key.
+  ///
+  /// Is always `null` if [isRemove].
+  final V newValue;
+
+  /// True if this key was inserted.
+  final bool isInsert;
+
+  /// True if this key was removed.
+  final bool isRemove;
+
+  /// Create an update record of [key] from [oldValue] to [newValue].
+  const MapChangeRecord(this.key, this.oldValue, this.newValue)
+      : isInsert = false,
+        isRemove = false;
+
+  /// Create an insert record of [key] and [newValue].
+  const MapChangeRecord.insert(this.key, this.newValue)
+      : isInsert = true,
+        isRemove = false,
+        oldValue = null;
+
+  /// Create a remove record of [key] with a former [oldValue].
+  const MapChangeRecord.remove(this.key, this.oldValue)
+      : isInsert = false,
+        isRemove = true,
+        newValue = null;
+
+  /// Apply this change record to [map].
+  void apply(Map<K, V> map) {
+    if (isRemove) {
+      map.remove(key);
+    } else {
+      map[key] = newValue;
+    }
+  }
+
+  @override
+  bool operator ==(Object o) {
+    if (o is MapChangeRecord<K, V>) {
+      return key == o.key &&
+             oldValue == o.oldValue &&
+             newValue == o.newValue &&
+             isInsert == o.isInsert &&
+             isRemove == o.isRemove;
+    }
+    return false;
+  }
+
+  @override
+  int get hashCode {
+    return hashObjects([
+      key,
+      oldValue,
+      newValue,
+      isInsert,
+      isRemove,
+    ]);
+  }
+
+  @override
+  String toString() {
+    final kind = isInsert ? 'insert' : isRemove ? 'remove' : 'set';
+    return '#<MapChangeRecord $kind $key from $oldValue to $newValue';
+  }
+}

pubspec.yaml

@@ -1,5 +1,5 @@
 name: observable
-version: 0.15.0+2
+version: 0.16.0
 author: Dart Team <[email protected]>
 description: Support for marking objects as observable
 homepage: https://github.com/dart-lang/observable