Merge pull request #294 from twpayne/collection

Collection improvements

src/ol/collection.exports

@@ -1,11 +1,11 @@
 @exportSymbol ol.Collection
 @exportProperty ol.Collection.prototype.clear
 @exportProperty ol.Collection.prototype.forEach
-@exportProperty ol.Collection.prototype.getArray
 @exportProperty ol.Collection.prototype.getAt
 @exportProperty ol.Collection.prototype.getLength
 @exportProperty ol.Collection.prototype.insertAt
 @exportProperty ol.Collection.prototype.pop
 @exportProperty ol.Collection.prototype.push
+@exportProperty ol.Collection.prototype.remove
 @exportProperty ol.Collection.prototype.removeAt
 @exportProperty ol.Collection.prototype.setAt

src/ol/collection.js

@@ -18,10 +18,7 @@ goog.require('ol.Object');
  */
 ol.CollectionEventType = {
   ADD: 'add',
-  INSERT_AT: 'insert_at',
+  REMOVE: 'remove'
-  REMOVE: 'remove',
-  REMOVE_AT: 'remove_at',
-  SET_AT: 'set_at'
 };
 
 
@@ -31,12 +28,9 @@ ol.CollectionEventType = {
  * @extends {goog.events.Event}
  * @param {ol.CollectionEventType} type Type.
  * @param {*=} opt_elem Element.
- * @param {number=} opt_index Index.
- * @param {*=} opt_prev Value.
  * @param {Object=} opt_target Target.
  */
-ol.CollectionEvent =
+ol.CollectionEvent = function(type, opt_elem, opt_target) {
-    function(type, opt_elem, opt_index, opt_prev, opt_target) {
 
   goog.base(this, type, opt_target);
 
@@ -45,16 +39,6 @@ ol.CollectionEvent =
    */
   this.elem = opt_elem;
 
-  /**
-   * @type {number|undefined}
-   */
-  this.index = opt_index;
-
-  /**
-   * @type {*}
-   */
-  this.prev = opt_prev;
-
 };
 goog.inherits(ol.CollectionEvent, goog.events.Event);
 
@@ -151,10 +135,8 @@ ol.Collection.prototype.getLength = function() {
 ol.Collection.prototype.insertAt = function(index, elem) {
   goog.array.insertAt(this.array_, elem, index);
   this.updateLength_();
-  this.dispatchEvent(new ol.CollectionEvent(
+  this.dispatchEvent(
-      ol.CollectionEventType.ADD, elem, undefined, undefined, this));
+      new ol.CollectionEvent(ol.CollectionEventType.ADD, elem, this));
-  this.dispatchEvent(new ol.CollectionEvent(
-      ol.CollectionEventType.INSERT_AT, elem, index, undefined, this));
 };
 
 
@@ -177,6 +159,22 @@ ol.Collection.prototype.push = function(elem) {
 };
 
 
+/**
+ * Removes the first occurence of elem from the collection.
+ * @param {*} elem Element.
+ * @return {*} The removed element or undefined if elem was not found.
+ */
+ol.Collection.prototype.remove = function(elem) {
+  var i;
+  for (i = 0; i < this.array_.length; ++i) {
+    if (this.array_[i] === elem) {
+      return this.removeAt(i);
+    }
+  }
+  return undefined;
+};
+
+
 /**
  * @param {number} index Index.
  * @return {*} Value.
@@ -185,10 +183,8 @@ ol.Collection.prototype.removeAt = function(index) {
   var prev = this.array_[index];
   goog.array.removeAt(this.array_, index);
   this.updateLength_();
-  this.dispatchEvent(new ol.CollectionEvent(
+  this.dispatchEvent(
-      ol.CollectionEventType.REMOVE, prev, undefined, undefined, this));
+      new ol.CollectionEvent(ol.CollectionEventType.REMOVE, prev, this));
-  this.dispatchEvent(new ol.CollectionEvent(ol.CollectionEventType.REMOVE_AT,
-      undefined, index, prev, this));
   return prev;
 };
 
@@ -202,12 +198,10 @@ ol.Collection.prototype.setAt = function(index, elem) {
   if (index < n) {
     var prev = this.array_[index];
     this.array_[index] = elem;
-    this.dispatchEvent(new ol.CollectionEvent(ol.CollectionEventType.SET_AT,
+    this.dispatchEvent(
-        elem, index, prev, this));
+        new ol.CollectionEvent(ol.CollectionEventType.REMOVE, prev, this));
-    this.dispatchEvent(new ol.CollectionEvent(ol.CollectionEventType.REMOVE,
+    this.dispatchEvent(
-        prev, undefined, undefined, this));
+        new ol.CollectionEvent(ol.CollectionEventType.ADD, elem, this));
-    this.dispatchEvent(new ol.CollectionEvent(ol.CollectionEventType.ADD,
-        elem, undefined, undefined, this));
   } else {
     var j;
     for (j = n; j < index; ++j) {

test/spec/ol/collection.test.js

@@ -104,67 +104,94 @@ describe('ol.collection', function() {
     });
   });
 
+  describe('remove', function() {
+    it('removes the first matching element', function() {
+      var collection = new ol.Collection([0, 1, 2]);
+      expect(collection.remove(1)).toEqual(1);
+      expect(collection.getArray()).toEqual([0, 2]);
+      expect(collection.getLength()).toEqual(2);
+    });
+    it('fires a remove event', function() {
+      var collection = new ol.Collection([0, 1, 2]);
+      var cb = jasmine.createSpy();
+      goog.events.listen(collection, ol.CollectionEventType.REMOVE, cb);
+      expect(collection.remove(1)).toEqual(1);
+      expect(cb).toHaveBeenCalled();
+      expect(cb.mostRecentCall.args[0].elem).toEqual(1);
+    });
+    it('does not remove more than one matching element', function() {
+      var collection = new ol.Collection([0, 1, 1, 2]);
+      expect(collection.remove(1)).toEqual(1);
+      expect(collection.getArray()).toEqual([0, 1, 2]);
+      expect(collection.getLength()).toEqual(3);
+    });
+    it('returns undefined if the element is not found', function() {
+      var collection = new ol.Collection([0, 1, 2]);
+      expect(collection.remove(3)).toBeUndefined();
+      expect(collection.getArray()).toEqual([0, 1, 2]);
+      expect(collection.getLength()).toEqual(3);
+    });
+  });
+
   describe('setAt and event', function() {
     it('does dispatch events', function() {
       var collection = new ol.Collection(['a', 'b']);
-      var index, prev;
+      var added, removed;
+      goog.events.listen(collection, ol.CollectionEventType.ADD, function(e) {
+        added = e.elem;
+      });
       goog.events.listen(
-          collection,
+          collection, ol.CollectionEventType.REMOVE, function(e) {
-          ol.CollectionEventType.SET_AT,
+            removed = e.elem;
-          function(e) {
-            index = e.index;
-            prev = e.prev;
           });
       collection.setAt(1, 1);
-      expect(index).toEqual(1);
+      expect(added).toEqual(1);
-      expect(prev).toEqual('b');
+      expect(removed).toEqual('b');
     });
   });
 
   describe('removeAt and event', function() {
     it('does dispatch events', function() {
       var collection = new ol.Collection(['a']);
-      var index, prev;
+      var removed;
       goog.events.listen(
-          collection, ol.CollectionEventType.REMOVE_AT, function(e) {
+          collection, ol.CollectionEventType.REMOVE, function(e) {
-            index = e.index;
+            removed = e.elem;
-            prev = e.prev;
           });
       collection.pop();
-      expect(index).toEqual(0);
+      expect(removed).toEqual('a');
-      expect(prev).toEqual('a');
     });
   });
 
   describe('insertAt and event', function() {
     it('does dispatch events', function() {
       var collection = new ol.Collection([0, 2]);
-      var index;
+      var added;
       goog.events.listen(
-          collection, ol.CollectionEventType.INSERT_AT, function(e) {
+          collection, ol.CollectionEventType.ADD, function(e) {
-            index = e.index;
+            added = e.elem;
           });
       collection.insertAt(1, 1);
-      expect(index).toEqual(1);
+      expect(added).toEqual(1);
     });
   });
 
   describe('setAt beyond end', function() {
     it('triggers events properly', function() {
-      var inserts = [];
+      var added = [];
       goog.events.listen(
-          collection, ol.CollectionEventType.INSERT_AT, function(e) {
+          collection, ol.CollectionEventType.ADD, function(e) {
-            inserts.push(e.index);
+            added.push(e.elem);
           });
       collection.setAt(2, 0);
       expect(collection.getLength()).toEqual(3);
       expect(collection.getAt(0)).toBeUndefined();
       expect(collection.getAt(1)).toBeUndefined();
       expect(collection.getAt(2)).toEqual(0);
-      expect(inserts.length).toEqual(3);
+      expect(added.length).toEqual(3);
-      expect(inserts[0]).toEqual(0);
+      expect(added[0]).toEqual(undefined);
-      expect(inserts[1]).toEqual(1);
+      expect(added[1]).toEqual(undefined);
-      expect(inserts[2]).toEqual(2);
+      expect(added[2]).toEqual(0);
     });
   });