From 3698049543751e19cd944fc2e5c555b2e873c82c Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Tue, 19 Feb 2013 14:03:48 -0700 Subject: [PATCH 1/7] View needs to call constructor on super --- src/ol/view.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ol/view.js b/src/ol/view.js index 871928073e..f9303af991 100644 --- a/src/ol/view.js +++ b/src/ol/view.js @@ -23,6 +23,8 @@ ol.ViewHint = { */ ol.View = function() { + goog.base(this); + /** * @private * @type {Array.} From b9507de3ea3469c8581364b1ba6c3c58334b94d7 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Tue, 19 Feb 2013 14:14:36 -0700 Subject: [PATCH 2/7] Removing unnecessary overhead --- src/ol/object.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ol/object.js b/src/ol/object.js index dbcf0a3e4e..aff2209925 100644 --- a/src/ol/object.js +++ b/src/ol/object.js @@ -232,14 +232,16 @@ ol.Object.prototype.set = function(key, value) { * @param {Object.} options Options. */ ol.Object.prototype.setOptions = function(options) { - goog.object.forEach(options, function(value, key) { - var setterName = ol.Object.getSetterName(key); + var key, value, setterName; + for (key in options) { + value = options[key]; + setterName = ol.Object.getSetterName(key); if (this[setterName]) { this[setterName](value); } else { this.set(key, value); } - }, this); + } }; @@ -270,9 +272,7 @@ ol.Object.prototype.unbind = function(key) { * Removes all bindings. */ ol.Object.prototype.unbindAll = function() { - var listeners = ol.Object.getListeners(this); - var keys = goog.object.getKeys(listeners); - goog.array.forEach(keys, function(key) { + for (var key in ol.Object.getListeners(this)) { this.unbind(key); - }, this); + } }; From 6cfc36d8cc77e739a8d2299247db3147b6d93e6f Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Tue, 19 Feb 2013 14:17:58 -0700 Subject: [PATCH 3/7] Using hasOwnProperty is better for user keys on objects we control --- src/ol/object.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/ol/object.js b/src/ol/object.js index aff2209925..60e36e2162 100644 --- a/src/ol/object.js +++ b/src/ol/object.js @@ -91,7 +91,8 @@ ol.Object.getAccessors = function(obj) { * @return {string} Changed name. */ ol.Object.getChangedEventType = function(key) { - return ol.Object.changedEventTypeCache_[key] || + return ol.Object.changedEventTypeCache_.hasOwnProperty(key) ? + ol.Object.changedEventTypeCache_[key] : (ol.Object.changedEventTypeCache_[key] = key.toLowerCase() + '_changed'); }; @@ -101,7 +102,8 @@ ol.Object.getChangedEventType = function(key) { * @return {string} Getter name. */ ol.Object.getGetterName = function(key) { - return ol.Object.getterNameCache_[key] || + return ol.Object.getterNameCache_.hasOwnProperty(key) ? + ol.Object.getterNameCache_[key] : (ol.Object.getterNameCache_[key] = 'get' + ol.Object.capitalize(key)); }; @@ -121,7 +123,8 @@ ol.Object.getListeners = function(obj) { * @return {string} Setter name. */ ol.Object.getSetterName = function(key) { - return ol.Object.setterNameCache_[key] || + return ol.Object.setterNameCache_.hasOwnProperty(key) ? + ol.Object.setterNameCache_[key] : (ol.Object.setterNameCache_[key] = 'set' + ol.Object.capitalize(key)); }; From 052e995f2e74cf250a61033177e283a46cc58641 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Tue, 19 Feb 2013 14:19:40 -0700 Subject: [PATCH 4/7] Key in obj check not appropriate for user supplied keys --- src/ol/object.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ol/object.js b/src/ol/object.js index 60e36e2162..dbb5f95aea 100644 --- a/src/ol/object.js +++ b/src/ol/object.js @@ -165,7 +165,7 @@ ol.Object.prototype.changed = goog.nullFunction; */ ol.Object.prototype.get = function(key) { var accessors = ol.Object.getAccessors(this); - if (goog.object.containsKey(accessors, key)) { + if (accessors.hasOwnProperty(key)) { var accessor = accessors[key]; var target = accessor.target; var targetKey = accessor.key; @@ -186,7 +186,7 @@ ol.Object.prototype.get = function(key) { */ ol.Object.prototype.notify = function(key) { var accessors = ol.Object.getAccessors(this); - if (goog.object.containsKey(accessors, key)) { + if (accessors.hasOwnProperty(key)) { var accessor = accessors[key]; var target = accessor.target; var targetKey = accessor.key; @@ -214,7 +214,7 @@ ol.Object.prototype.notifyInternal_ = function(key) { */ ol.Object.prototype.set = function(key, value) { var accessors = ol.Object.getAccessors(this); - if (goog.object.containsKey(accessors, key)) { + if (accessors.hasOwnProperty(key)) { var accessor = accessors[key]; var target = accessor.target; var targetKey = accessor.key; From 0707deb465a4494e40c89ee783d0ca5c20fd88e5 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Tue, 19 Feb 2013 14:28:59 -0700 Subject: [PATCH 5/7] Properties set with set should not be accessed with bracket --- src/ol/collection.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ol/collection.js b/src/ol/collection.js index f1de3a535e..86c331302f 100644 --- a/src/ol/collection.js +++ b/src/ol/collection.js @@ -93,7 +93,7 @@ goog.inherits(ol.Collection, ol.Object); * Remove all elements from the collection. */ ol.Collection.prototype.clear = function() { - while (this[ol.CollectionProperty.LENGTH]) { + while (this.getLength() > 0) { this.pop(); } }; @@ -187,7 +187,7 @@ ol.Collection.prototype.removeAt = function(index) { * @param {*} elem Element. */ ol.Collection.prototype.setAt = function(index, elem) { - var n = this[ol.CollectionProperty.LENGTH]; + var n = this.getLength(); if (index < n) { var prev = this.array_[index]; this.array_[index] = elem; From d6ff58305de50954fd68506f3cde4c411c0f72ac Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Tue, 19 Feb 2013 14:35:11 -0700 Subject: [PATCH 6/7] Using a dedicated object for value storage Without this, we are limited in the key names that we can accept from users. And because of compiler renaming, we don't know ahead of time what the limitations are (e.g. the key 'a' may clobber the 'set' method). --- src/ol/object.js | 21 +++++++++++++++------ test/spec/ol/object.test.js | 35 +++++++++++++++++++++++++++++++++-- 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/src/ol/object.js b/src/ol/object.js index dbb5f95aea..af7a0a9edb 100644 --- a/src/ol/object.js +++ b/src/ol/object.js @@ -39,6 +39,13 @@ ol.ObjectProperty = { */ ol.Object = function(opt_values) { goog.base(this); + + /** + * @private + * @type {Object.} + */ + this.values_ = {}; + if (goog.isDef(opt_values)) { this.setValues(opt_values); } @@ -164,6 +171,7 @@ ol.Object.prototype.changed = goog.nullFunction; * @return {*} Value. */ ol.Object.prototype.get = function(key) { + var value; var accessors = ol.Object.getAccessors(this); if (accessors.hasOwnProperty(key)) { var accessor = accessors[key]; @@ -171,13 +179,14 @@ ol.Object.prototype.get = function(key) { var targetKey = accessor.key; var getterName = ol.Object.getGetterName(targetKey); if (target[getterName]) { - return target[getterName](); + value = target[getterName](); } else { - return target.get(targetKey); + value = target.get(targetKey); } - } else { - return this[key]; + } else if (this.values_.hasOwnProperty(key)) { + value = this.values_[key]; } + return value; }; @@ -225,7 +234,7 @@ ol.Object.prototype.set = function(key, value) { target.set(targetKey, value); } } else { - this[key] = value; + this.values_[key] = value; this.notifyInternal_(key); } }; @@ -266,7 +275,7 @@ ol.Object.prototype.unbind = function(key) { var value = this.get(key); var accessors = ol.Object.getAccessors(this); delete accessors[key]; - this[key] = value; + this.values_[key] = value; } }; diff --git a/test/spec/ol/object.test.js b/test/spec/ol/object.test.js index 3dd69c04e8..bb195088dd 100644 --- a/test/spec/ol/object.test.js +++ b/test/spec/ol/object.test.js @@ -34,6 +34,37 @@ describe('ol.Object', function() { }); }); + describe('#get()', function() { + + it('does not return values that are not explicitly set', function() { + var o = new ol.Object(); + expect(o.get('constructor')).toBeUndefined(); + expect(o.get('hasOwnProperty')).toBeUndefined(); + expect(o.get('isPrototypeOf')).toBeUndefined(); + expect(o.get('propertyIsEnumerable')).toBeUndefined(); + expect(o.get('toLocaleString')).toBeUndefined(); + expect(o.get('toString')).toBeUndefined(); + expect(o.get('valueOf')).toBeUndefined(); + }); + + }); + + describe('#set()', function() { + it('can be used with arbitrary names', function() { + var o = new ol.Object(); + + o.set('set', 'sat'); + expect(o.get('set')).toBe('sat'); + + o.set('get', 'got'); + expect(o.get('get')).toBe('got'); + + o.set('toString', 'string'); + expect(o.get('toString')).toBe('string'); + expect(typeof o.toString).toBe('function'); + }); + }); + describe('setValues', function() { it('sets multiple values at once', function() { @@ -309,7 +340,7 @@ describe('ol.Object', function() { describe('setter', function() { beforeEach(function() { o.setX = function(x) { - this.x = x; + this.set('x', x); }; spyOn(o, 'setX').andCallThrough(); }); @@ -327,8 +358,8 @@ describe('ol.Object', function() { var o2 = new ol.Object(); o2.bindTo('x', o); o2.set('x', 1); - expect(o.get('x')).toEqual(1); expect(o.setX).toHaveBeenCalled(); + expect(o.get('x')).toEqual(1); }); }); }); From 2919906ba664c87a0c845af3b684622b88ba6d72 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Tue, 19 Feb 2013 14:47:49 -0700 Subject: [PATCH 7/7] Add method for getting keys --- src/ol/object.js | 12 ++++++++ test/spec/ol/object.test.js | 55 +++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/ol/object.js b/src/ol/object.js index af7a0a9edb..861eb651ad 100644 --- a/src/ol/object.js +++ b/src/ol/object.js @@ -190,6 +190,18 @@ ol.Object.prototype.get = function(key) { }; +/** + * Get a list of object property names. + * @return {Array.} List of property names. + */ +ol.Object.prototype.getKeys = function() { + var keys = goog.object.getKeys(ol.Object.getAccessors(this)).concat( + goog.object.getKeys(this.values_)); + goog.array.removeDuplicates(keys); + return keys; +}; + + /** * @param {string} key Key. */ diff --git a/test/spec/ol/object.test.js b/test/spec/ol/object.test.js index bb195088dd..e8d2ccfe1b 100644 --- a/test/spec/ol/object.test.js +++ b/test/spec/ol/object.test.js @@ -65,6 +65,23 @@ describe('ol.Object', function() { }); }); + describe('#getKeys()', function() { + + it('returns property names set at construction', function() { + var o = new ol.Object({ + prop1: 'val1', + prop2: 'val2', + toString: 'string', + get: 'foo' + }); + + var keys = o.getKeys(); + expect(keys.length).toBe(4); + expect(keys.sort()).toEqual(['get', 'prop1', 'prop2', 'toString']); + }); + + }); + describe('setValues', function() { it('sets multiple values at once', function() { @@ -74,6 +91,9 @@ describe('ol.Object', function() { }); expect(o.get('k1')).toEqual(1); expect(o.get('k2')).toEqual(2); + + var keys = o.getKeys().sort(); + expect(keys).toEqual(['k1', 'k2']); }); }); @@ -130,6 +150,9 @@ describe('ol.Object', function() { it('dispatches events to object', function() { o.set('k', 1); expect(listener1).toHaveBeenCalled(); + + expect(o.getKeys()).toEqual(['k']); + expect(o2.getKeys()).toEqual(['k']); }); it('dispatches generic change events to object', function() { @@ -145,6 +168,9 @@ describe('ol.Object', function() { it('dispatches events to object bound to', function() { o2.set('k', 2); expect(listener1).toHaveBeenCalled(); + + expect(o.getKeys()).toEqual(['k']); + expect(o2.getKeys()).toEqual(['k']); }); it('dispatches generic change events to object bound to', function() { @@ -168,6 +194,9 @@ describe('ol.Object', function() { o2.bindTo('k', o); expect(o.get('k')).toEqual(1); expect(o2.get('k')).toEqual(1); + + expect(o.getKeys()).toEqual(['k']); + expect(o2.getKeys()).toEqual(['k']); }); }); @@ -178,6 +207,9 @@ describe('ol.Object', function() { o.set('k', 1); expect(o.get('k')).toEqual(1); expect(o2.get('k')).toEqual(1); + + expect(o.getKeys()).toEqual(['k']); + expect(o2.getKeys()).toEqual(['k']); }); }); @@ -270,6 +302,9 @@ describe('ol.Object', function() { expect(o2.get('k1')).toBeUndefined(); expect(listener1).toHaveBeenCalled(); expect(listener2).toHaveBeenCalled(); + + expect(o.getKeys()).toEqual(['k1']); + expect(o2.getKeys()).toEqual(['k2']); }); }); @@ -288,6 +323,10 @@ describe('ol.Object', function() { expect(o.get('k1')).toEqual(1); expect(o2.get('k2')).toEqual(1); expect(o3.get('k3')).toEqual(1); + + expect(o.getKeys()).toEqual(['k1']); + expect(o2.getKeys()).toEqual(['k2']); + expect(o3.getKeys()).toEqual(['k3']); }); describe('backward', function() { @@ -297,6 +336,10 @@ describe('ol.Object', function() { expect(o.get('k1')).toEqual(1); expect(o2.get('k2')).toEqual(1); expect(o3.get('k3')).toEqual(1); + + expect(o.getKeys()).toEqual(['k1']); + expect(o2.getKeys()).toEqual(['k2']); + expect(o3.getKeys()).toEqual(['k3']); }); }); }); @@ -350,6 +393,8 @@ describe('ol.Object', function() { o.set('x', 1); expect(o.get('x')).toEqual(1); expect(o.setX).not.toHaveBeenCalled(); + + expect(o.getKeys()).toEqual(['x']); }); }); @@ -360,6 +405,9 @@ describe('ol.Object', function() { o2.set('x', 1); expect(o.setX).toHaveBeenCalled(); expect(o.get('x')).toEqual(1); + + expect(o.getKeys()).toEqual(['x']); + expect(o2.getKeys()).toEqual(['x']); }); }); }); @@ -385,6 +433,9 @@ describe('ol.Object', function() { o2.bindTo('x', o); expect(o2.get('x')).toEqual(1); expect(o.getX).toHaveBeenCalled(); + + expect(o.getKeys()).toEqual([]); + expect(o2.getKeys()).toEqual(['x']); }); }); }); @@ -399,6 +450,8 @@ describe('ol.Object', function() { it('sets the property', function() { var o = new ol.Object({k: 1}); expect(o.get('k')).toEqual(1); + + expect(o.getKeys()).toEqual(['k']); }); }); @@ -416,6 +469,8 @@ describe('ol.Object', function() { o.set('K', 1); expect(listener1).toHaveBeenCalled(); expect(listener2).not.toHaveBeenCalled(); + + expect(o.getKeys()).toEqual(['K']); }); }); });