One more time. Remove Path.isValid, expose path.valid. Make invalidPa…

…th a singleton
googlearchive · Sep 3, 2013 · db1d00d · db1d00d
1 parent cf35308
commit db1d00d
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 55 deletions.
diff --git a/src/observe.js b/src/observe.js
@@ -147,7 +147,7 @@
     if (path)
       return path;
     if (!isPathValid(pathString))
-      return;
+      return invalidPath;
     var path = new Path(pathString, constructorIsPrivate);
     pathCache[pathString] = path;
     return path;
@@ -157,6 +157,7 @@
 
   Path.prototype = createObject({
     __proto__: [],
+    valid: true,
 
     toString: function() {
       return this.join('.');
@@ -213,6 +214,10 @@
     }
   });
 
+  var invalidPath = new Path('', constructorIsPrivate);
+  invalidPath.valid = false;
+  invalidPath.getValueFrom = invalidPath.setValueFrom = function() {};
+
   var MAX_DIRTY_CHECK_CYCLES = 1000;
 
   function dirtyCheck(observer) {
@@ -605,28 +610,10 @@
   function PathObserver(object, path, callback, target, token, valueFn,
                         setValueFn) {
     var path = path instanceof Path ? path : getPath(path);
-
-    if (!path) {
-      // Invalid path.
-      this.closed = true;
-      this.value_ = undefined;
+    if (!path || !path.length || !isObject(object)) {
+      this.value_ = path ? path.getValueFrom(object) : undefined;
       this.value = valueFn ? valueFn(this.value_) : this.value_;
-      return;
-    }
-
-    if (!path.length) {
-      // 0-length path.
       this.closed = true;
-      this.value_ = object;
-      this.value = valueFn ? valueFn(this.value_) : this.value_;
-      return;
-    }
-
-    if (!isObject(object)) {
-      // non-object & non-0-length path.
-      this.closed = true;
-      this.value_ = undefined;
-      this.value = valueFn ? valueFn(this.value_) : this.value_;
       return;
     }
 
@@ -718,19 +705,7 @@
         throw Error('Cannot add more paths once started.');
 
       var path = path instanceof Path ? path : getPath(path);
-      var value = undefined;
-
-      if (!path) {
-        // Invalid path.
-      } else if (!path.length) {
-        // 0-length path.
-        path = undefined;
-        value = object;
-      } else if (!isObject(object)) {
-        // non-object & non-0-length path.
-        path = undefined;
-        value = undefined;
-      }
+      var value = path ? path.getValueFrom(object) : undefined;
 
       this.observed.push(object, path);
       this.values.push(value);
@@ -1309,9 +1284,4 @@
   global.PathObserver = PathObserver;
   global.CompoundPathObserver = CompoundPathObserver;
   global.Path = Path;
-
-  global.Path.isValid = function(pathString) {
-    return Path.get(pathString) !== undefined;
-  };
-
 })(typeof global !== 'undefined' && global ? global : this);
diff --git a/tests/test.js b/tests/test.js
@@ -75,24 +75,27 @@ suite('Path', function() {
   });
 
   test('valid paths', function() {
-    assert.isTrue(Path.isValid('a'));
-    assert.isTrue(Path.isValid('a.b'));
-    assert.isTrue(Path.isValid('a. b'));
-    assert.isTrue(Path.isValid('a .b'));
-    assert.isTrue(Path.isValid('a . b'));
-    assert.isTrue(Path.isValid(''));
-    assert.isTrue(Path.isValid(' '));
-    assert.isTrue(Path.isValid(null));
-    assert.isTrue(Path.isValid(undefined));
-    assert.isTrue(Path.isValid());
-    assert.isTrue(Path.isValid(42));
+    assert.isTrue(Path.get('a').valid);
+    assert.isTrue(Path.get('a.b').valid);
+    assert.isTrue(Path.get('a. b').valid);
+    assert.isTrue(Path.get('a .b').valid);
+    assert.isTrue(Path.get('a . b').valid);
+    assert.isTrue(Path.get('').valid);
+    assert.isTrue(Path.get(' ').valid);
+    assert.isTrue(Path.get(null).valid);
+    assert.isTrue(Path.get(undefined).valid);
+    assert.isTrue(Path.get().valid);
+    assert.isTrue(Path.get(42).valid);
   });
 
   test('invalid paths', function() {
-    assert.isFalse(Path.isValid('a b'));
-    assert.isFalse(Path.isValid('.'));
-    assert.isFalse(Path.isValid(' . '));
-    assert.isFalse(Path.isValid('..'));
+    var p = Path.get('a b');
+    assert.isFalse(p.valid);
+    assert.isUndefined(p.getValueFrom({ a: { b: 2 }}));
+
+    assert.isFalse(Path.get('.').valid);
+    assert.isFalse(Path.get(' . ').valid);
+    assert.isFalse(Path.get('..').valid);
   });
 
   test('Paths are interned', function() {
@@ -819,7 +822,7 @@ suite('CompoundPathObserver Tests', function() {
 
   teardown(doTeardown);
 
-  test('CompoundPath Simple', function() {
+  test('Simple', function() {
     var model = { a: 1, b: 2, c: 3 };
 
     function valueFn(values) {
@@ -844,6 +847,27 @@ suite('CompoundPathObserver Tests', function() {
 
     observer.close();
   });
+
+  test('Degenerate Values', function() {
+    var model = {};
+
+    function valueFn(values) {
+      assert.strictEqual(4, values.length);
+      assert.strictEqual(undefined, values[0]);
+      assert.strictEqual('obj-value', values[1]);
+      assert.strictEqual(undefined, values[2]);
+      assert.strictEqual(undefined, values[3]);
+    }
+
+    observer = new CompoundPathObserver(callback, undefined, undefined,
+                                        valueFn);
+    observer.addPath({}, '.'); // invalid path
+    observer.addPath('obj-value', ''); // empty path
+    observer.addPath({}, 'foo'); // unreachable
+    observer.addPath(3, 'bar'); // non-object with non-empty path
+    observer.start();
+    observer.close();
+  });
 });
 
 suite('ArrayObserver Tests', function() {