style(change detection): some cleanup

Closes #834

Author: vicb

lib/change_detection/ast.dart

@@ -124,8 +124,8 @@ String _argList(List<AST> items) => items.join(', ');
 /**
  * The name is a bit oxymoron, but it is essentially the NullObject pattern.
  *
- * This allows children to set a handler on this Record and then let it write the initial
- * constant value to the forwarding Record.
+ * This allows children to set a handler on this Record and then let it write
+ * the initial constant value to the forwarding Record.
  */
 class _ConstantWatchRecord extends WatchRecord<_Handler> {
   final currentValue;

lib/change_detection/change_detection.dart

@@ -1,6 +1,6 @@
 library change_detection;
 
-typedef EvalExceptionHandler(error, stack);
+typedef void EvalExceptionHandler(error, stack);
 
 /**
  * An interface for [ChangeDetectorGroup] groups related watches together. It
@@ -13,49 +13,48 @@ abstract class ChangeDetectorGroup<H> {
    * Watch a specific [field] on an [object].
    *
    * If the [field] is:
-   *   - _name_ - Name of the property to watch. (If the [object] is a Map then
+   * * _name_ - Name of the property to watch. (If the [object] is a Map then
    *   treat the name as a key.)
-   *   - _[]_ - Watch all items in an array.
-   *   - _{}_ - Watch all items in a Map.
-   *   - _._ - Watch the actual object identity.
-   *
+   * * _null_ - Watch all the items for arrays and maps otherwise the object
+   *   identity.
    *
    * Parameters:
-   * - [object] to watch.
-   * - [field] to watch on the [object].
-   * - [handler] an opaque object passed on to [Record].
+   * * [object] to watch.
+   * * [field] to watch on the [object].
+   * * [handler] an opaque object passed on to [Record].
    */
   WatchRecord<H> watch(Object object, String field, H handler);
 
-  /** Use to remove all watches in the group in an efficient manner. */
+  /// Remove all the watches in an efficient manner.
   void remove();
 
-  /** Create a child [ChangeDetectorGroup] */
+  /// Create a child [ChangeDetectorGroup]
   ChangeDetectorGroup<H> newGroup();
 }
 
 /**
  * An interface for [ChangeDetector]. An application can have multiple instances
- * of the [ChangeDetector] to be used for checking different application domains.
+ * of the [ChangeDetector] to be used for checking different application
+ * domains.
  *
  * [ChangeDetector] works by comparing the identity of the objects not by
  * calling the `.equals()` method. This is because ChangeDetector needs to have
  * predictable performance, and the developer can implement `.equals()` on top
  * of identity checks.
  *
- * - [H] A [Record] has associated handler object. The handler object is
- * opaque to the [ChangeDetector] but it is meaningful to the code which
- * registered the watcher. It can be a data structure, an object, or a function.
- * It is up to the developer to attach meaning to it.
+ * [H] A [Record] has associated handler object. The handler object is opaque
+ * to the [ChangeDetector] but it is meaningful to the code which registered the
+ * watcher. It can be a data structure, an object, or a function. It is up to
+ * the developer to attach meaning to it.
  */
 abstract class ChangeDetector<H> extends ChangeDetectorGroup<H> {
   /**
    * This method does the work of collecting the changes and returns them as a
    * linked list of [Record]s. The [Record]s are returned in the
    * same order as they were registered.
    */
-  Iterator<Record<H>> collectChanges({ EvalExceptionHandler exceptionHandler,
-                                   AvgStopwatch stopwatch });
+  Iterator<Record<H>> collectChanges({EvalExceptionHandler exceptionHandler,
+                                      AvgStopwatch stopwatch });
 }
 
 abstract class Record<H> {
@@ -64,10 +63,9 @@ abstract class Record<H> {
 
   /**
    * The field which is being watched:
-   *   - _name_ - Name of the field to watch.
-   *   - _[]_ - Watch all items in an array.
-   *   - _{}_ - Watch all items in a Map.
-   *   - _._ - Watch the actual object identity.
+   * * _name_ - Name of the field to watch.
+   * * _null_ - Watch all the items for arrays and maps otherwise the object
+   *   identity.
    */
   String get field;
 
@@ -78,9 +76,16 @@ abstract class Record<H> {
    */
   H get handler;
 
-  /** Current value of the [field] on the [object] */
+  /**
+   * * The current value of the [field] on the [object],
+   * * a [CollectionChangeRecord] if an iterable is observed,
+   * * a [MapChangeRecord] if a map is observed.
+   */
   get currentValue;
-  /** Previous value of the [field] on the [object] */
+  /**
+   * * Previous value of the [field] on the [object],
+   * * [:null:] when an iterable or a map are observed.
+   */
   get previousValue;
 }
 
@@ -89,27 +94,24 @@ abstract class Record<H> {
  * manually triggering the checking.
  */
 abstract class WatchRecord<H> extends Record<H> {
-  /** Set a new object for checking */
+  /// Set a new object for checking
   set object(value);
 
-  /**
-   * Check to see if the field on the object has changed. Returns [true] if
-   * change has been detected.
-   */
+  /// Returns [:true:] when changes have been detected
   bool check();
 
   void remove();
 }
 
 /**
  * If the [ChangeDetector] is watching a [Map] then the [currentValue] of
- * [Record] will contain an instance of this object. A [MapChangeRecord]
+ * [Record] will contain an instance of [MapChangeRecord]. A [MapChangeRecord]
  * contains the changes to the map since the last execution. The changes are
  * reported as a list of [MapKeyValue]s which contain the key as well as its
  * current and previous value.
  */
 abstract class MapChangeRecord<K, V> {
-  /// The underlying iterable object
+  /// The underlying map object
   Map get map;
 
   /// A list of [CollectionKeyValue]s which are in the iteration order. */
@@ -162,13 +164,12 @@ abstract class ChangedKeyValue<K, V> extends MapKeyValue<K, V> {
   ChangedKeyValue<K, V> get nextChangedKeyValue;
 }
 
-
 /**
  * If the [ChangeDetector] is watching an [Iterable] then the [currentValue] of
- * [Record] will contain this object. The [CollectionChangeRecord] contains the
- * changes to the collection since the last execution. The changes are reported
- * as a list of [CollectionChangeItem]s which contain the item as well as its
- * current and previous position in the list.
+ * [Record] will contain an instance of [CollectionChangeRecord]. The
+ * [CollectionChangeRecord] contains the changes to the collection since the
+ * last execution. The changes are reported as a list of [CollectionChangeItem]s
+ * which contain the item as well as its current and previous index.
  */
 abstract class CollectionChangeRecord<V> {
   /** The underlying iterable object */
@@ -195,10 +196,10 @@ abstract class CollectionChangeRecord<V> {
  * which tracks the [item]s [currentKey] and [previousKey] location.
  */
 abstract class CollectionChangeItem<V> {
-  /** Previous item location in the list or [null] if addition. */
+  /** Previous item location in the list or [:null:] if addition. */
   int get previousIndex;
 
-  /** Current item location in the list or [null] if removal. */
+  /** Current item location in the list or [:null:] if removal. */
   int get currentIndex;
 
   /** The item. */
@@ -241,8 +242,8 @@ abstract class RemovedItem<V> extends CollectionChangeItem<V> {
   RemovedItem<V> get nextRemovedItem;
 }
 
-typedef FieldGetter(object);
-typedef FieldSetter(object, value);
+typedef dynamic FieldGetter(object);
+typedef void FieldSetter(object, value);
 
 abstract class FieldGetterFactory {
   get isMethodInvoke;

lib/change_detection/dirty_checking_change_detector.dart

@@ -9,17 +9,17 @@ import 'package:angular/change_detection/change_detection.dart';
  *
  * GOALS:
  *   - Plugable implementation, replaceable with other technologies, such as
- *   Object.observe().
+ *     Object.observe().
  *   - SPEED this needs to be as fast as possible.
  *   - No GC pressure. Since change detection runs often it should perform no
- *   memory allocations.
+ *     memory allocations.
  *   - The changes need to be delivered in a single data-structure at once.
- *   There are two reasons for this:
- *     1. It should be easy to measure the cost of change detection vs
- *     processing.
- *     2. The feature may move to VM for performance reason. The VM should be
- *     free to implement it in any way. The only requirement is that the list of
- *     changes need to be delivered.
+ *     There are two reasons for this:
+ *       1. It should be easy to measure the cost of change detection vs
+ *          processing.
+ *       2. The feature may move to VM for performance reason. The VM should be
+ *          free to implement it in any way. The only requirement is that the
+ *          list of changes need to be delivered.
  *
  * [DirtyCheckingRecord]
  *
@@ -395,8 +395,8 @@ class DirtyCheckingRecord<H> implements Record<H>, WatchRecord<H> {
   FieldGetter _getter;
 
   DirtyCheckingRecord(this._group, this._fieldGetterFactory, this.handler,
-                      this.field, object) {
-    this.object = object;
+                      this.field, _object) {
+    object = _object;
   }
 
   DirtyCheckingRecord.marker()
@@ -407,14 +407,14 @@ class DirtyCheckingRecord<H> implements Record<H>, WatchRecord<H> {
         _getter = null,
         _mode = _MODE_MARKER_;
 
-  get object => _object;
+  dynamic get object => _object;
 
   /**
    * Setting an [object] will cause the setter to introspect it and place
    * [DirtyCheckingRecord] into different access modes. If Object it sets up
    * reflection. If [Map] then it sets up map accessor.
    */
-  set object(obj) {
+  void set object(obj) {
     _object = obj;
     if (obj == null) {
       _mode = _MODE_IDENTITY_;

lib/change_detection/dirty_checking_change_detector_dynamic.dart

@@ -17,24 +17,20 @@ class DynamicFieldGetterFactory implements FieldGetterFactory {
     try {
       return method(object, name) != null;
     } catch (e, s) {
-      print('isMethod($object, $name) => false\n$s');
       return false;
     }
   }
 
   Function method(Object object, String name) {
     Symbol symbol = new Symbol(name);
     InstanceMirror instanceMirror = reflect(object);
-    return (List args, Map namedArgs) {
-      return instanceMirror.invoke(symbol, args, namedArgs).reflectee;
-    };
+    return (List args, Map namedArgs) =>
+        instanceMirror.invoke(symbol, args, namedArgs).reflectee;
   }
 
   FieldGetter getter(Object object, String name) {
     Symbol symbol = new Symbol(name);
     InstanceMirror instanceMirror = reflect(object);
-    return (Object object) {
-      return instanceMirror.getField(symbol).reflectee;
-    };
+    return (Object object) =>  instanceMirror.getField(symbol).reflectee;
   }
 }

lib/change_detection/dirty_checking_change_detector_static.dart

@@ -22,8 +22,8 @@ class StaticFieldGetterFactory implements FieldGetterFactory {
   }
 
   Function method(Object object, String name) {
-    var getter = getters[name];
-    if (getter == null) throw "Missing getter: (o) => o.$name";
-    return getter;
+    var method = getters[name];
+    if (method == null) throw "Missing method: $name";
+    return method;
   }
 }

lib/change_detection/watch_group.dart

@@ -6,8 +6,8 @@ part 'linked_list.dart';
 part 'ast.dart';
 part 'prototype_map.dart';
 
-typedef ReactionFn(value, previousValue);
-typedef ChangeLog(String expression, current, previous);
+typedef void ReactionFn(value, previousValue);
+typedef void ChangeLog(String expression, current, previous);
 
 /**
  * Extend this class if you wish to pretend to be a function, but you don't know
@@ -340,7 +340,7 @@ class WatchGroup implements _EvalWatchList, _WatchGroupList {
     lines.add('WatchGroup[$id](watches: ${watches.join(', ')})');
     var childGroup = _watchGroupHead;
     while (childGroup != null) {
-      lines.add('  ' + childGroup.toString().split('\n').join('\n  '));
+      lines.add('  ' + childGroup.toString().replace('\n', '\n  '));
       childGroup = childGroup._nextWatchGroup;
     }
     return lines.join('\n');
@@ -364,10 +364,10 @@ class RootWatchGroup extends WatchGroup {
   int _removeCount = 0;
 
 
-  RootWatchGroup(FieldGetterFactory this._fieldGetterFactory,
+  RootWatchGroup(this._fieldGetterFactory,
                  ChangeDetector changeDetector,
-                 Object context):
-      super._root(changeDetector, context);
+                 Object context)
+      : super._root(changeDetector, context);
 
   RootWatchGroup get _rootGroup => this;
 
@@ -597,7 +597,7 @@ abstract class _Handler implements _LinkedList, _LinkedListItem, _WatchList {
 }
 
 class _ConstantHandler extends _Handler {
-  _ConstantHandler(WatchGroup watchGroup, String expression, dynamic constantValue)
+  _ConstantHandler(WatchGroup watchGroup, String expression, constantValue)
       : super(watchGroup, expression)
   {
     watchRecord = new _EvalWatchRecord.constant(this, constantValue);
@@ -681,10 +681,6 @@ class _InvokeHandler extends _Handler implements _ArgHandlerList {
     watchRecord.object = object;
   }
 
-  void onChange(Record<_Handler> record) {
-    super.onChange(record);
-  }
-
   void _releaseWatch() {
     (watchRecord as _EvalWatchRecord).remove();
   }