make PathObserver.defineProperty take four arguments and avoid needle…

…ss allocation R=arv BUG= Review URL: https://codereview.appspot.com/29320043
googlearchive · Nov 19, 2013 · 6da3137 · 6da3137
1 parent 809a851
commit 6da3137
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 34 deletions.
diff --git a/src/observe.js b/src/observe.js
@@ -887,36 +887,35 @@
   // every PathObserver used by defineProperty share a single Object.observe
   // callback, and thus get() can simply call observer.deliver() and any changes
   // to any dependent value will be observed.
-  PathObserver.defineProperty = function(object, name, descriptor) {
+  PathObserver.defineProperty = function(target, name, object, path) {
     // TODO(rafaelw): Validate errors
-    var obj = descriptor.object;
-    var path = getPath(descriptor.path);
-    var notify = notifyFunction(object, name);
+    path = getPath(path);
+    var notify = notifyFunction(target, name);
 
-    var observer = new PathObserver(obj, descriptor.path,
+    var observer = new PathObserver(object, path,
         function(newValue, oldValue) {
           if (notify)
             notify(PROP_UPDATE_TYPE, oldValue);
         }
     );
 
-    Object.defineProperty(object, name, {
+    Object.defineProperty(target, name, {
       get: function() {
-        return path.getValueFrom(obj);
+        return path.getValueFrom(object);
       },
       set: function(newValue) {
-        path.setValueFrom(obj, newValue);
+        path.setValueFrom(object, newValue);
       },
       configurable: true
     });
 
     return {
       close: function() {
-        var oldValue = path.getValueFrom(obj);
+        var oldValue = path.getValueFrom(object);
         if (notify)
           observer.deliver();
         observer.close();
-        Object.defineProperty(object, name, {
+        Object.defineProperty(target, name, {
           value: oldValue,
           writable: true,
           configurable: true

diff --git a/tests/test.js b/tests/test.js
@@ -722,20 +722,14 @@ suite('PathObserver Tests', function() {
     var b = {};
     var c = {};
 
-    root.a.observer = PathObserver.defineProperty(root.a, 'value', {
-      object: root,
-      path: 'value'
-    });
+    root.a.observer = PathObserver.defineProperty(root.a, 'value',
+                                                  root, 'value');
 
-    root.a.b.observer = PathObserver.defineProperty(root.a.b, 'value', {
-      object: root.a,
-      path: 'value'
-    });
+    root.a.b.observer = PathObserver.defineProperty(root.a.b, 'value',
+                                                    root.a, 'value');
 
-    root.c.observer = PathObserver.defineProperty(root.c, 'value', {
-      object: root,
-      path: 'value'
-    });
+    root.c.observer = PathObserver.defineProperty(root.c, 'value',
+                                                  root, 'value');
 
     root.c.value = 2;
     assert.strictEqual(2, root.a.b.value);
@@ -759,10 +753,8 @@ suite('PathObserver Tests', function() {
       Object.observe(target, callback);
     }
 
-    var observer = PathObserver.defineProperty(target, 'computed', {
-      object: source,
-      path: 'foo.bar'
-    });
+    var observer = PathObserver.defineProperty(target, 'computed',
+                                               source, 'foo.bar');
     assert.isTrue(target.hasOwnProperty('computed'));
     assert.strictEqual(1, target.computed);
 
@@ -819,18 +811,12 @@ suite('PathObserver Tests', function() {
 
   test('DefineProperty - empty path', function() {
     var target = {}
-    var observer = PathObserver.defineProperty(target, 'foo', {
-      object: 1,
-      path: ''
-    });
+    var observer = PathObserver.defineProperty(target, 'foo', 1, '');
     assert.isTrue(target.hasOwnProperty('foo'));
     assert.strictEqual(1, target.foo);
 
     var obj = {};
-    var observer2 = PathObserver.defineProperty(target, 'bar', {
-      object: obj,
-      path: ''
-    });
+    var observer2 = PathObserver.defineProperty(target, 'bar', obj, '');
     assert.isTrue(target.hasOwnProperty('bar'));
     assert.strictEqual(obj, target.bar);
   });