From 1715c181adde7bdf64a4576512c014d52391d99c Mon Sep 17 00:00:00 2001 From: Jeremy Ashkenas Date: Fri, 6 Jan 2012 15:42:43 -0500 Subject: [PATCH] Fixes #81 -- optimize for addition of large arrays of models, not for individual inserts. --- backbone.js | 41 +++++++++++++++++++------------------ test/collection.js | 50 +++++++++++++++------------------------------- 2 files changed, 36 insertions(+), 55 deletions(-) diff --git a/backbone.js b/backbone.js index 561064b3..1234c060 100644 --- a/backbone.js +++ b/backbone.js @@ -416,13 +416,18 @@ // Add a model, or list of models to the set. Pass **silent** to avoid // firing the `added` event for every new model. add : function(models, options) { - if (_.isArray(models)) { - for (var i = 0, l = models.length; i < l; i++) { - if (options && (options.at == +options.at) && i) options.at += 1; - this._add(models[i], options); + var i, l; + options || (options = {}); + if (!_.isArray(models)) models = [models]; + for (i = 0, l = models.length; i < l; i++) { + if (options && (options.at == +options.at) && i) options.at += 1; + models[i] = this._add(models[i], options); + } + if (this.comparator) this.sort({silent: true}); + if (!options.silent) { + for (i = 0; i < l; i++) { + models[i].trigger('add', models[i], this, options); } - } else { - this._add(models, options); } return this; }, @@ -430,12 +435,10 @@ // Remove a model, or a list of models from the set. Pass silent to avoid // firing the `removed` event for every model removed. remove : function(models, options) { - if (_.isArray(models)) { - for (var i = 0, l = models.length; i < l; i++) { - this._remove(models[i], options); - } - } else { - this._remove(models, options); + options || (options = {}); + if (!_.isArray(models)) models = [models]; + for (var i = 0, l = models.length; i < l; i++) { + this._remove(models[i], options); } return this; }, @@ -559,28 +562,25 @@ // hash indexes for `id` and `cid` lookups. // Returns the model, or 'false' if validation on a new model fails. _add : function(model, options) { - options || (options = {}); model = this._prepareModel(model, options); if (!model) return false; var already = this.getByCid(model); if (already) throw new Error(["Can't add the same model to a set twice", already.id]); this._byId[model.id] = model; this._byCid[model.cid] = model; - var index = options.at != null ? options.at : - this.comparator ? this.sortedIndex(model, this.comparator) : - this.length; - this.models.splice(index, 0, model); + if (options.at != null) { + this.models.splice(options.at, 0, model); + } else { + this.models.push(model); + } model.bind('all', this._onModelEvent); this.length++; - options.index = index; - if (!options.silent) model.trigger('add', model, this, options); return model; }, // Internal implementation of removing a single model from the set, updating // hash indexes for `id` and `cid` lookups. _remove : function(model, options) { - options || (options = {}); model = this.getByCid(model) || this.get(model); if (!model) return null; delete this._byId[model.id]; @@ -588,7 +588,6 @@ var index = this.indexOf(model); this.models.splice(index, 1); this.length--; - options.index = index; if (!options.silent) model.trigger('remove', model, this, options); this._removeReference(model); return model; diff --git a/test/collection.js b/test/collection.js index 15952641..4f5984da 100644 --- a/test/collection.js +++ b/test/collection.js @@ -136,23 +136,6 @@ $(document).ready(function() { } }); - test("Collection: add model to collection and verify index updates", function() { - var f = new Backbone.Model({id: 20, label : 'f'}); - var g = new Backbone.Model({id: 21, label : 'g'}); - var h = new Backbone.Model({id: 22, label : 'h'}); - var col = new Backbone.Collection(); - - var counts = []; - - col.bind('add', function(model, collection, options) { - counts.push(options.index); - }); - col.add(f); - col.add(g); - col.add(h); - ok(_.isEqual(counts, [0,1,2])); - }); - test("Collection: add model to collection twice", function() { try { // no id, same cid @@ -207,6 +190,22 @@ $(document).ready(function() { equals(col.at(0).get('value'), 2); }); + test("Collection: add model to collection with sort()-style comparator", function() { + var col = new Backbone.Collection; + col.comparator = function(a, b) { + return a.get('name') < b.get('name') ? -1 : 1; + }; + var tom = new Backbone.Model({name: 'Tom'}); + var rob = new Backbone.Model({name: 'Rob'}); + var tim = new Backbone.Model({name: 'Tim'}); + col.add(tom); + col.add(rob); + col.add(tim); + equals(col.indexOf(rob), 0); + equals(col.indexOf(tim), 1); + equals(col.indexOf(tom), 2); + }); + test("Collection: remove", function() { var removed = otherRemoved = null; col.bind('remove', function(model){ removed = model.get('label'); }); @@ -218,23 +217,6 @@ $(document).ready(function() { equals(otherRemoved, null); }); - test("Collection: remove should return correct index events", function() { - var f = new Backbone.Model({id: 20, label : 'f'}); - var g = new Backbone.Model({id: 21, label : 'g'}); - var h = new Backbone.Model({id: 22, label : 'h'}); - var col = new Backbone.Collection([f,g,h]); - - var counts = []; - - col.bind('remove', function(model, collection, options) { - counts.push(options.index); - }); - col.remove(h); - col.remove(g); - col.remove(f); - ok(_.isEqual(counts, [2,1,0])); - }); - test("Collection: events are unbound on remove", function() { var counter = 0; var dj = new Backbone.Model();