From 905a0059dbe535bb495f33db127d074f70e06490 Mon Sep 17 00:00:00 2001 From: Tom Payne Date: Wed, 6 Mar 2013 15:28:36 +0100 Subject: [PATCH 1/3] Remove unused insert_at, set_at, and remove_at events --- src/ol/collection.js | 42 +++++++---------------------- test/spec/ol/collection.test.js | 48 ++++++++++++++++----------------- 2 files changed, 33 insertions(+), 57 deletions(-) diff --git a/src/ol/collection.js b/src/ol/collection.js index f8cf78ddab..5c2b043020 100644 --- a/src/ol/collection.js +++ b/src/ol/collection.js @@ -18,10 +18,7 @@ goog.require('ol.Object'); */ ol.CollectionEventType = { ADD: 'add', - INSERT_AT: 'insert_at', - REMOVE: 'remove', - REMOVE_AT: 'remove_at', - SET_AT: 'set_at' + REMOVE: 'remove' }; @@ -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 = - function(type, opt_elem, opt_index, opt_prev, opt_target) { +ol.CollectionEvent = function(type, opt_elem, 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( - ol.CollectionEventType.ADD, elem, undefined, undefined, this)); - this.dispatchEvent(new ol.CollectionEvent( - ol.CollectionEventType.INSERT_AT, elem, index, undefined, this)); + this.dispatchEvent( + new ol.CollectionEvent(ol.CollectionEventType.ADD, elem, this)); }; @@ -185,10 +167,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( - ol.CollectionEventType.REMOVE, prev, undefined, undefined, this)); - this.dispatchEvent(new ol.CollectionEvent(ol.CollectionEventType.REMOVE_AT, - undefined, index, prev, this)); + this.dispatchEvent( + new ol.CollectionEvent(ol.CollectionEventType.REMOVE, prev, this)); return prev; }; @@ -202,12 +182,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, - elem, index, prev, this)); - this.dispatchEvent(new ol.CollectionEvent(ol.CollectionEventType.REMOVE, - prev, undefined, undefined, this)); - this.dispatchEvent(new ol.CollectionEvent(ol.CollectionEventType.ADD, - elem, undefined, undefined, this)); + this.dispatchEvent( + new ol.CollectionEvent(ol.CollectionEventType.REMOVE, prev, this)); + this.dispatchEvent( + new ol.CollectionEvent(ol.CollectionEventType.ADD, elem, this)); } else { var j; for (j = n; j < index; ++j) { diff --git a/test/spec/ol/collection.test.js b/test/spec/ol/collection.test.js index 81ef412fbb..6253714d43 100644 --- a/test/spec/ol/collection.test.js +++ b/test/spec/ol/collection.test.js @@ -107,64 +107,62 @@ describe('ol.collection', function() { 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, - ol.CollectionEventType.SET_AT, - function(e) { - index = e.index; - prev = e.prev; + collection, ol.CollectionEventType.REMOVE, function(e) { + removed = e.elem; }); collection.setAt(1, 1); - expect(index).toEqual(1); - expect(prev).toEqual('b'); + expect(added).toEqual(1); + 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) { - index = e.index; - prev = e.prev; + collection, ol.CollectionEventType.REMOVE, function(e) { + removed = e.elem; }); collection.pop(); - expect(index).toEqual(0); - expect(prev).toEqual('a'); + expect(removed).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) { - index = e.index; + collection, ol.CollectionEventType.ADD, function(e) { + 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) { - inserts.push(e.index); + collection, ol.CollectionEventType.ADD, function(e) { + 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(inserts[0]).toEqual(0); - expect(inserts[1]).toEqual(1); - expect(inserts[2]).toEqual(2); + expect(added.length).toEqual(3); + expect(added[0]).toEqual(undefined); + expect(added[1]).toEqual(undefined); + expect(added[2]).toEqual(0); }); }); From 0a38f2f7a7541b708b23dff1932aa8b65dbe5c7f Mon Sep 17 00:00:00 2001 From: Tom Payne Date: Wed, 6 Mar 2013 15:39:23 +0100 Subject: [PATCH 2/3] Don't export ol.Collection.getArray --- src/ol/collection.exports | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ol/collection.exports b/src/ol/collection.exports index b052cc13f2..a22ce5d472 100644 --- a/src/ol/collection.exports +++ b/src/ol/collection.exports @@ -1,7 +1,6 @@ @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 From 3e7c913c44c64c7c0b3c815049ec294ca09ec287 Mon Sep 17 00:00:00 2001 From: Tom Payne Date: Wed, 6 Mar 2013 15:48:00 +0100 Subject: [PATCH 3/3] Add ol.Collection.remove --- src/ol/collection.exports | 1 + src/ol/collection.js | 16 ++++++++++++++++ test/spec/ol/collection.test.js | 29 +++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/src/ol/collection.exports b/src/ol/collection.exports index a22ce5d472..11b7e20c8f 100644 --- a/src/ol/collection.exports +++ b/src/ol/collection.exports @@ -6,5 +6,6 @@ @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 diff --git a/src/ol/collection.js b/src/ol/collection.js index 5c2b043020..08d3340188 100644 --- a/src/ol/collection.js +++ b/src/ol/collection.js @@ -159,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. diff --git a/test/spec/ol/collection.test.js b/test/spec/ol/collection.test.js index 6253714d43..b32b5893d9 100644 --- a/test/spec/ol/collection.test.js +++ b/test/spec/ol/collection.test.js @@ -104,6 +104,35 @@ 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']);