Merge pull request #217 from tschaub/safer-object

Allow ol.Object to be used with arbitrary keys.  Add a getKeys method.

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;

src/ol/object.js

@@ -39,6 +39,13 @@ ol.ObjectProperty = {
  */
 ol.Object = function(opt_values) {
   goog.base(this);
+
+  /**
+   * @private
+   * @type {Object.<string, *>}
+   */
+  this.values_ = {};
+
   if (goog.isDef(opt_values)) {
     this.setValues(opt_values);
   }
@@ -91,7 +98,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 +109,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 +130,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));
 };
 
@@ -161,20 +171,34 @@ ol.Object.prototype.changed = goog.nullFunction;
  * @return {*} Value.
  */
 ol.Object.prototype.get = function(key) {
+  var 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;
     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 {
+  } else if (this.values_.hasOwnProperty(key)) {
-    return this[key];
+    value = this.values_[key];
   }
+  return value;
+};
+
+
+/**
+ * Get a list of object property names.
+ * @return {Array.<string>} 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;
 };
 
 
@@ -183,7 +207,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;
@@ -211,7 +235,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;
@@ -222,7 +246,7 @@ ol.Object.prototype.set = function(key, value) {
       target.set(targetKey, value);
     }
   } else {
-    this[key] = value;
+    this.values_[key] = value;
     this.notifyInternal_(key);
   }
 };
@@ -232,14 +256,16 @@ ol.Object.prototype.set = function(key, value) {
  * @param {Object.<string, *>} options Options.
  */
 ol.Object.prototype.setOptions = function(options) {
-  goog.object.forEach(options, function(value, key) {
+  var key, value, setterName;
-    var setterName = ol.Object.getSetterName(key);
+  for (key in options) {
+    value = options[key];
+    setterName = ol.Object.getSetterName(key);
     if (this[setterName]) {
       this[setterName](value);
     } else {
       this.set(key, value);
     }
-  }, this);
+  }
 };
 
 
@@ -261,7 +287,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;
   }
 };
 
@@ -270,9 +296,7 @@ ol.Object.prototype.unbind = function(key) {
  * Removes all bindings.
  */
 ol.Object.prototype.unbindAll = function() {
-  var listeners = ol.Object.getListeners(this);
+  for (var key in ol.Object.getListeners(this)) {
-  var keys = goog.object.getKeys(listeners);
-  goog.array.forEach(keys, function(key) {
     this.unbind(key);
-  }, this);
+  }
 };

src/ol/view.js

@@ -23,6 +23,8 @@ ol.ViewHint = {
  */
 ol.View = function() {
 
+  goog.base(this);
+
   /**
    * @private
    * @type {Array.<number>}

test/spec/ol/object.test.js

@@ -34,6 +34,54 @@ 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('#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() {
@@ -43,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']);
     });
   });
 
@@ -99,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() {
@@ -114,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() {
@@ -137,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']);
       });
     });
 
@@ -147,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']);
       });
     });
 
@@ -239,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']);
     });
   });
 
@@ -257,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() {
@@ -266,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']);
       });
     });
   });
@@ -309,7 +383,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();
     });
@@ -319,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']);
       });
     });
 
@@ -327,8 +403,11 @@ 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);
+
+        expect(o.getKeys()).toEqual(['x']);
+        expect(o2.getKeys()).toEqual(['x']);
       });
     });
   });
@@ -354,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']);
       });
     });
   });
@@ -368,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']);
     });
   });
 
@@ -385,6 +469,8 @@ describe('ol.Object', function() {
       o.set('K', 1);
       expect(listener1).toHaveBeenCalled();
       expect(listener2).not.toHaveBeenCalled();
+
+      expect(o.getKeys()).toEqual(['K']);
     });
   });
 });