Skip to content

Commit

Permalink
Fixes #9, Backbone comparators can now be either sort() or sortBy() i…
Browse files Browse the repository at this point in the history
…terators.
  • Loading branch information
jashkenas committed Jan 6, 2012
1 parent 44bb57a commit 6b3ff7b
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 45 deletions.
8 changes: 6 additions & 2 deletions backbone.js
Expand Up @@ -328,7 +328,7 @@
// Determine if the model has changed since the last `"change"` event.
// If you specify an attribute name, determine if that attribute has changed.
hasChanged : function(attr) {
if (attr) return this._previousAttributes[attr] != this.attributes[attr];
if (attr) return !_.isEqual(this._previousAttributes[attr], this.attributes[attr]);
return this._changed;
},

Expand Down Expand Up @@ -462,7 +462,11 @@
sort : function(options) {
options || (options = {});
if (!this.comparator) throw new Error('Cannot sort a set without a comparator');
this.models = this.sortBy(this.comparator);
if (this.comparator.length == 1) {

This comment has been minimized.

Copy link
@sstephenson

sstephenson Jan 6, 2012

Contributor

I believe this changes the behavior for comparators defined as CoffeeScript bound methods, where the arity will always be zero. They'd previously be subject to sortBy, but now will be subject to sort.

This comment has been minimized.

Copy link
@jashkenas

jashkenas Jan 6, 2012

Author Owner

Grr. !$^@! CoffeeScript.

Apart from being something that we should mention explicitly in the docs, do you have a suggestion for changing this?

This comment has been minimized.

Copy link
@sstephenson

sstephenson Jan 6, 2012

Contributor

Here are a couple ideas...

  1. Use a different function name for the sort behavior—compare maybe?
  2. Automatically bind this.comparator to avoid the need for => at all.

This comment has been minimized.

Copy link
@jashkenas

jashkenas Jan 6, 2012

Author Owner

Is there any reason why comparator would want to be a bound function in the first place? Seeing as how it's class-, not instance-specific, I can't think of one.

This comment has been minimized.

Copy link
@sstephenson

sstephenson Jan 6, 2012

Contributor

We use it in Basecamp Mobile to change the sort behavior on a collection based on where it's being used:

module.exports = class Projects extends Collection
  model: Project

  initialize: (models, {@recent}) ->
    @url = if @recent
      "/api/v3/projects/recent"
    else
      "/api/v3/projects?status=active"

  # ...

  comparator: (project) =>
    if @recent
      -Date.parse project.lastAccessedOn
    else
      project.name

Perhaps that's not the best way to do it, though (we could also use inheritance). In that case I'd be fine with just documenting it as a gotcha.

This comment has been minimized.

Copy link
@jashkenas

jashkenas Jan 11, 2012

Author Owner

Thinking about this a bit more (as I'd rather not expand the API for comparators), perhaps this should be viewed as a breaking change with a bit of backwards compatibility, instead of a feature enhancement.

When upgrading, would you want to change your bound comparators to use the sort API, or do you prefer sortBy for this sort of thing?

This comment has been minimized.

Copy link
@jashkenas

jashkenas Jan 12, 2012

Author Owner

Ok -- I think I'll do exactly as you suggest, and have this.comparator be bound when you first assign it, so there should be no need to use the fat arrow here.

this.models = this.sortBy(this.comparator);
} else {
this.models.sort(this.comparator);
}
if (!options.silent) this.trigger('reset', this, options);
return this;
},
Expand Down
2 changes: 1 addition & 1 deletion examples/todos/index.html
Expand Up @@ -6,7 +6,7 @@
<link href="todos.css" media="all" rel="stylesheet" type="text/css"/>
<script src="../../test/vendor/json2.js"></script>
<script src="../../test/vendor/jquery-1.6.4.js"></script>
<script src="../../test/vendor/underscore-1.2.2.js"></script>
<script src="../../test/vendor/underscore-1.2.4.js"></script>
<script src="../../backbone.js"></script>
<script src="../backbone-localstorage.js"></script>
<script src="todos.js"></script>
Expand Down
2 changes: 1 addition & 1 deletion index.html
Expand Up @@ -3012,7 +3012,7 @@ <h2 id="changelog">Change Log</h2>

</div>

<script src="test/vendor/underscore-1.2.2.js"></script>
<script src="test/vendor/underscore-1.2.4.js"></script>
<script src="test/vendor/jquery-1.6.4.js"></script>
<script src="test/vendor/json2.js"></script>
<script src="backbone.js"></script>
Expand Down
6 changes: 6 additions & 0 deletions test/collection.js
Expand Up @@ -17,6 +17,12 @@ $(document).ready(function() {
var otherCol = new Backbone.Collection();

test("Collection: new and sort", function() {
equals(col.first(), a, "a should be first");
equals(col.last(), d, "d should be last");
col.comparator = function(a, b) {
return a.id > b.id ? -1 : 1;
};
col.sort();
equals(col.first(), a, "a should be first");
equals(col.last(), d, "d should be last");
col.comparator = function(model) { return model.id; };
Expand Down
2 changes: 1 addition & 1 deletion test/model.js
Expand Up @@ -91,7 +91,7 @@ $(document).ready(function() {
var Model = Backbone.Model.extend({
urlRoot: function() { return '/nested/' + this.get('parent_id') + '/collection'}
// looks better in coffeescript: urlRoot: => "/nested/#{@get('parent_id')}/collection"
});
});

var model = new Model({parent_id: 1});
equals(model.url(), '/nested/1/collection');
Expand Down
2 changes: 1 addition & 1 deletion test/test-ender.html
Expand Up @@ -7,7 +7,7 @@
<script type="text/javascript" src="vendor/ender-jeesh.js"></script>
<script type="text/javascript" src="vendor/qunit.js"></script>
<script type="text/javascript" src="vendor/jslitmus.js"></script>
<script type="text/javascript" src="vendor/underscore-1.2.2.js"></script>
<script type="text/javascript" src="vendor/underscore-1.2.4.js"></script>
<script type="text/javascript" src="../backbone.js"></script>

<script type="text/javascript" src="events.js"></script>
Expand Down
2 changes: 1 addition & 1 deletion test/test-zepto.html
Expand Up @@ -7,7 +7,7 @@
<script type="text/javascript" src="vendor/zepto-0.6.js"></script>
<script type="text/javascript" src="vendor/qunit.js"></script>
<script type="text/javascript" src="vendor/jslitmus.js"></script>
<script type="text/javascript" src="vendor/underscore-1.2.2.js"></script>
<script type="text/javascript" src="vendor/underscore-1.2.4.js"></script>
<script type="text/javascript" src="../backbone.js"></script>

<script type="text/javascript" src="events.js"></script>
Expand Down
2 changes: 1 addition & 1 deletion test/test.html
Expand Up @@ -10,7 +10,7 @@
QUnit.config.reorder = false;
</script>
<script type="text/javascript" src="vendor/jslitmus.js"></script>
<script type="text/javascript" src="vendor/underscore-1.2.2.js"></script>
<script type="text/javascript" src="vendor/underscore-1.2.4.js"></script>
<script type="text/javascript" src="../backbone.js"></script>

<script type="text/javascript" src="noconflict.js"></script>
Expand Down
92 changes: 55 additions & 37 deletions test/vendor/underscore-1.2.2.js → test/vendor/underscore-1.2.4.js
@@ -1,5 +1,5 @@
// Underscore.js 1.2.2
// (c) 2011 Jeremy Ashkenas, DocumentCloud Inc.
// Underscore.js 1.2.4
// (c) 2009-2012 Jeremy Ashkenas, DocumentCloud Inc.
// Underscore is freely distributable under the MIT license.
// Portions of Underscore are inspired or borrowed from Prototype,
// Oliver Steele's Functional, and John Resig's Micro-Templating.
Expand Down Expand Up @@ -67,7 +67,7 @@
}

// Current version.
_.VERSION = '1.2.2';
_.VERSION = '1.2.4';

// Collection Functions
// --------------------
Expand Down Expand Up @@ -101,13 +101,14 @@
each(obj, function(value, index, list) {
results[results.length] = iterator.call(context, value, index, list);
});
if (obj.length === +obj.length) results.length = obj.length;
return results;
};

// **Reduce** builds up a single result from a list of values, aka `inject`,
// or `foldl`. Delegates to **ECMAScript 5**'s native `reduce` if available.
_.reduce = _.foldl = _.inject = function(obj, iterator, memo, context) {
var initial = memo !== void 0;
var initial = arguments.length > 2;
if (obj == null) obj = [];
if (nativeReduce && obj.reduce === nativeReduce) {
if (context) iterator = _.bind(iterator, context);
Expand All @@ -121,20 +122,22 @@
memo = iterator.call(context, memo, value, index, list);
}
});
if (!initial) throw new TypeError("Reduce of empty array with no initial value");
if (!initial) throw new TypeError('Reduce of empty array with no initial value');
return memo;
};

// The right-associative version of reduce, also known as `foldr`.
// Delegates to **ECMAScript 5**'s native `reduceRight` if available.
_.reduceRight = _.foldr = function(obj, iterator, memo, context) {
var initial = arguments.length > 2;
if (obj == null) obj = [];
if (nativeReduceRight && obj.reduceRight === nativeReduceRight) {
if (context) iterator = _.bind(iterator, context);
return memo !== void 0 ? obj.reduceRight(iterator, memo) : obj.reduceRight(iterator);
return initial ? obj.reduceRight(iterator, memo) : obj.reduceRight(iterator);
}
var reversed = (_.isArray(obj) ? obj.slice() : _.toArray(obj)).reverse();
return _.reduce(reversed, iterator, memo, context);
var reversed = _.toArray(obj).reverse();
if (context && !initial) iterator = _.bind(iterator, context);
return initial ? _.reduce(reversed, iterator, memo, context) : _.reduce(reversed, iterator);
};

// Return the first value which passes a truth test. Aliased as `detect`.
Expand Down Expand Up @@ -189,7 +192,7 @@
// Delegates to **ECMAScript 5**'s native `some` if available.
// Aliased as `any`.
var any = _.some = _.any = function(obj, iterator, context) {
iterator = iterator || _.identity;
iterator || (iterator = _.identity);
var result = false;
if (obj == null) return result;
if (nativeSome && obj.some === nativeSome) return obj.some(iterator, context);
Expand All @@ -215,7 +218,7 @@
_.invoke = function(obj, method) {
var args = slice.call(arguments, 2);
return _.map(obj, function(value) {
return (method.call ? method || value : value[method]).apply(value, args);
return (_.isFunction(method) ? method || value : value[method]).apply(value, args);
});
};

Expand Down Expand Up @@ -402,10 +405,11 @@
});
};

// Take the difference between one array and another.
// Take the difference between one array and a number of other arrays.
// Only the elements present in just the first array will remain.
_.difference = function(array, other) {
return _.filter(array, function(value){ return !_.include(other, value); });
_.difference = function(array) {
var rest = _.flatten(slice.call(arguments, 1));
return _.filter(array, function(value){ return !_.include(rest, value); });
};

// Zip together multiple lists into a single array -- elements that share
Expand All @@ -432,7 +436,7 @@
return array[i] === item ? i : -1;
}
if (nativeIndexOf && array.indexOf === nativeIndexOf) return array.indexOf(item);
for (i = 0, l = array.length; i < l; i++) if (array[i] === item) return i;
for (i = 0, l = array.length; i < l; i++) if (i in array && array[i] === item) return i;
return -1;
};

Expand All @@ -441,7 +445,7 @@
if (array == null) return -1;
if (nativeLastIndexOf && array.lastIndexOf === nativeLastIndexOf) return array.lastIndexOf(item);
var i = array.length;
while (i--) if (array[i] === item) return i;
while (i--) if (i in array && array[i] === item) return i;
return -1;
};

Expand Down Expand Up @@ -579,17 +583,17 @@
// conditionally execute the original function.
_.wrap = function(func, wrapper) {
return function() {
var args = [func].concat(slice.call(arguments));
var args = [func].concat(slice.call(arguments, 0));
return wrapper.apply(this, args);
};
};

// Returns a function that is the composition of a list of functions, each
// consuming the return value of the function that follows.
_.compose = function() {
var funcs = slice.call(arguments);
var funcs = arguments;
return function() {
var args = slice.call(arguments);
var args = arguments;
for (var i = funcs.length - 1; i >= 0; i--) {
args = [funcs[i].apply(this, args)];
}
Expand Down Expand Up @@ -677,8 +681,8 @@
if (a._chain) a = a._wrapped;
if (b._chain) b = b._wrapped;
// Invoke a custom `isEqual` method if one is provided.
if (_.isFunction(a.isEqual)) return a.isEqual(b);
if (_.isFunction(b.isEqual)) return b.isEqual(a);
if (a.isEqual && _.isFunction(a.isEqual)) return a.isEqual(b);
if (b.isEqual && _.isFunction(b.isEqual)) return b.isEqual(a);
// Compare `[[Class]]` names.
var className = toString.call(a);
if (className != toString.call(b)) return false;
Expand All @@ -687,13 +691,11 @@
case '[object String]':
// Primitives and their corresponding object wrappers are equivalent; thus, `"5"` is
// equivalent to `new String("5")`.
return String(a) == String(b);
return a == String(b);
case '[object Number]':
a = +a;
b = +b;
// `NaN`s are equivalent, but non-reflexive. An `egal` comparison is performed for
// other numeric values.
return a != a ? b != b : (a == 0 ? 1 / a == 1 / b : a == b);
return a != +a ? b != +b : (a == 0 ? 1 / a == 1 / b : a == +b);
case '[object Date]':
case '[object Boolean]':
// Coerce dates and booleans to numeric primitive values. Dates are compared by their
Expand Down Expand Up @@ -733,7 +735,7 @@
}
} else {
// Objects with different constructors are not equivalent.
if ("constructor" in a != "constructor" in b || a.constructor != b.constructor) return false;
if ('constructor' in a != 'constructor' in b || a.constructor != b.constructor) return false;
// Deep compare objects.
for (var key in a) {
if (hasOwnProperty.call(a, key)) {
Expand Down Expand Up @@ -786,11 +788,10 @@
};

// Is a given variable an arguments object?
if (toString.call(arguments) == '[object Arguments]') {
_.isArguments = function(obj) {
return toString.call(obj) == '[object Arguments]';
};
} else {
_.isArguments = function(obj) {
return toString.call(obj) == '[object Arguments]';
};
if (!_.isArguments(arguments)) {
_.isArguments = function(obj) {
return !!(obj && hasOwnProperty.call(obj, 'callee'));
};
Expand Down Expand Up @@ -891,6 +892,11 @@
escape : /<%-([\s\S]+?)%>/g
};

// When customizing `templateSettings`, if you don't want to define an
// interpolation, evaluation or escaping regex, we need one that is
// guaranteed not to match.
var noMatch = /.^/;

// JavaScript micro-templating, similar to John Resig's implementation.
// Underscore templating handles arbitrary delimiters, preserves whitespace,
// and correctly escapes quotes within interpolated code.
Expand All @@ -900,22 +906,31 @@
'with(obj||{}){__p.push(\'' +
str.replace(/\\/g, '\\\\')
.replace(/'/g, "\\'")
.replace(c.escape, function(match, code) {
.replace(c.escape || noMatch, function(match, code) {
return "',_.escape(" + code.replace(/\\'/g, "'") + "),'";
})
.replace(c.interpolate, function(match, code) {
.replace(c.interpolate || noMatch, function(match, code) {
return "'," + code.replace(/\\'/g, "'") + ",'";
})
.replace(c.evaluate || null, function(match, code) {
.replace(c.evaluate || noMatch, function(match, code) {
return "');" + code.replace(/\\'/g, "'")
.replace(/[\r\n\t]/g, ' ') + ";__p.push('";
.replace(/[\r\n\t]/g, ' ')
.replace(/\\\\/g, '\\') + ";__p.push('";
})
.replace(/\r/g, '\\r')
.replace(/\n/g, '\\n')
.replace(/\t/g, '\\t')
+ "');}return __p.join('');";
var func = new Function('obj', '_', tmpl);
return data ? func(data, _) : function(data) { return func(data, _) };
if (data) return func(data, _);
return function(data) {
return func.call(this, data, _);
};
};

// Add a "chain" function, which will delegate to the wrapper.
_.chain = function(obj) {
return _(obj).chain();
};

// The OOP Wrapper
Expand Down Expand Up @@ -950,8 +965,11 @@
each(['pop', 'push', 'reverse', 'shift', 'sort', 'splice', 'unshift'], function(name) {
var method = ArrayProto[name];
wrapper.prototype[name] = function() {
method.apply(this._wrapped, arguments);
return result(this._wrapped, this._chain);
var wrapped = this._wrapped;
method.apply(wrapped, arguments);
var length = wrapped.length;
if ((name == 'shift' || name == 'splice') && length === 0) delete wrapped[0];
return result(wrapped, this._chain);
};
});

Expand Down

0 comments on commit 6b3ff7b

Please sign in to comment.