Merge pull request jashkenas#1644 from braddunbar/default-options

Refactor reset/sort.
matthewyang · Sep 14, 2012 · 8ea7d9a · 8ea7d9a
2 parents 5b19d8a + 85fca58
commit 8ea7d9a
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 12 deletions.
diff --git a/backbone.js b/backbone.js
@@ -731,22 +731,23 @@
     // normal circumstances, as the set will maintain sort order as each item
     // is added.
     sort: function(options) {
-      options || (options = {});
-      if (!this.comparator) throw new Error('Cannot sort a set without a comparator');
+      if (!this.comparator) {
+        throw new Error('Cannot sort a set without a comparator');
+      }
 
       // If provided an attribute name, use it to sort the collection.
       if (_.isString(this.comparator)) {
         var attr = this.comparator;
         this.comparator = function(model){ return model.get(attr); };
       }
 
-      var boundComparator = _.bind(this.comparator, this);
       if (this.comparator.length === 1) {
-        this.models = this.sortBy(boundComparator);
+        this.models = this.sortBy(this.comparator, this);
       } else {
-        this.models.sort(boundComparator);
+        this.models.sort(_.bind(this.comparator, this));
       }
-      if (!options.silent) this.trigger('reset', this, options);
+
+      if (!options || !options.silent) this.trigger('reset', this, options);
       return this;
     },
 
@@ -759,14 +760,12 @@
     // you can reset the entire set with a new list of models, without firing
     // any `add` or `remove` events. Fires `reset` when finished.
     reset: function(models, options) {
-      models  || (models = []);
-      options || (options = {});
       for (var i = 0, l = this.models.length; i < l; i++) {
         this._removeReference(this.models[i]);
       }
       this._reset();
-      this.add(models, _.extend({silent: true}, options));
-      if (!options.silent) this.trigger('reset', this, options);
+      if (models) this.add(models, _.extend({silent: true}, options));
+      if (!options || !options.silent) this.trigger('reset', this, options);
       return this;
     },
 

diff --git a/test/collection.js b/test/collection.js
@@ -231,7 +231,7 @@ $(document).ready(function() {
     equal(col.indexOf(tom), 2);
   });
 
-  test("comparator that depends on `this`", 1, function() {
+  test("comparator that depends on `this`", 2, function() {
     var col = new Backbone.Collection;
     col.negative = function(num) {
       return -num;
@@ -240,7 +240,12 @@ $(document).ready(function() {
       return this.negative(a.id);
     };
     col.add([{id: 1}, {id: 2}, {id: 3}]);
-    equal(col.pluck('id').join(' '), '3 2 1');
+    deepEqual(col.pluck('id'), [3, 2, 1]);
+    col.comparator = function(a, b) {
+      return this.negative(b.id) - this.negative(a.id);
+    };
+    col.sort();
+    deepEqual(col.pluck('id'), [1, 2, 3]);
   });
 
   test("remove", 5, function() {