Skip to content
73 changes: 66 additions & 7 deletions backbone.js
Original file line number Diff line number Diff line change
Expand Up @@ -828,16 +828,75 @@
return this.models[index];
},

// Perform a binary search for a model in a sorted collection.
search: function (toFind, options) {
if (!this.comparator) throw new Error('Cannot search an unsorted Collection');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this broken for the _.isFunction(toFind) case

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well no, if the collection isn't sorted then it isn't sorted. I suppose you could allow this to pass in the case that the user has sorted the collection himself. I believe the thinking behind the _.isFunction(toFind) case was that if you were, for example, looking for a model who had a timestamp within a certain range (say a week or day) in a collection sorted by the timestamps property.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like the toFind function feature, to easy to introduce inconsistencies in dev code if theres any inconsitency with toFind and comparitor. 👎 on that

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose I also thought that this was a good idea for the case that the comparator is a function. The user needs to have a model which matches equal to that he is searching for, whereas supplying the compare function, allows him to search specifically for models which match certain criteria without having to create a model to do it if one is not at hand.

options || (options = {});

// Create `compare` function for searching.
var compare;
if (_.isFunction(toFind)) {
// The user has provided the `compare` function.
compare = toFind;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No binding?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true, probably worth adding

} else if (_.isFunction(this.comparator)) {
// Use the `comparator` function, `toFind` is a model.
if (this.comparator.length === 2) {
compare = _.bind(this.comparator, this, toFind);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clever!

} else {
compare = _.bind(function (valResult, model) {
var modelResult = this.comparator(model);
return valResult === modelResult ? 0 : (valResult > modelResult ? 1 : -1);
}, this, this.comparator(toFind));
}
} else {
// `comparator` is a string indicating the model property used to sort
// `toFind` is the value sought.
compare = _.bind(function (model) {
var modelValue = model.get(this.comparator);
return toFind === modelValue ? 0 : (toFind > modelValue ? 1 : -1);
}, this);
}

// Perform binary search.
var found = false, max = this.length - 1, min = 0, index, relValue;
while (max >= min && !found) {
index = Math.floor((max + min) / 2);
relValue = compare(this.at(index));
if (relValue > 0) {
min = index + 1;
} else if (relValue < 0) {
max = index - 1;
} else if (options.getMax && index < max && compare(this.at(index + 1)) === 0) {
min = index + 1;
} else if (options.getMin && index > min && compare(this.at(index - 1)) === 0) {
max = index - 1;
} else {
found = true;
}
}
if (!found) return options.returnIndex ? -1 : void 0;
return options.returnIndex ? index : this.at(index);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👎 on this point. Just return the index

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may have a good point - what is your reasoning?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearer API, and easier to document. Let the user call model.at(index)

},

// Return models with matching attributes. Useful for simple cases of
// `filter`.
where: function(attrs, first) {
if (_.isEmpty(attrs)) return first ? void 0 : [];
return this[first ? 'find' : 'filter'](function(model) {
for (var key in attrs) {
if (attrs[key] !== model.get(key)) return false;
}
return true;
});
if (_.isEmpty(attrs)) {
return first ? void 0 : [];
} else if (!_.isEqual(_.keys(attrs), [this.comparator])) {
return this[first ? 'find' : 'filter'](function(model) {
for (var key in attrs) {
if (attrs[key] !== model.get(key)) return false;
}
return true;
});
} else if (first) {
return this.search(attrs[this.comparator], {getMin: true});
} else {
var minIndex = this.search(attrs[this.comparator], {returnIndex: true, getMin: true});
var maxIndex = this.search(attrs[this.comparator], {returnIndex: true, getMax: true});
return minIndex == null ? [] : this.models.slice(minIndex, maxIndex + 1);
}
},

// Return the first model with matching attributes. Useful for simple cases
Expand Down
61 changes: 58 additions & 3 deletions test/collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -547,15 +547,58 @@
equal(JSON.stringify(col), '[{"id":3,"label":"a"},{"id":2,"label":"b"},{"id":1,"label":"c"},{"id":0,"label":"d"}]');
});

test("where and findWhere", 8, function() {
test("search", 12, function() {
var model1 = new Backbone.Model({a: 0});
var model2 = new Backbone.Model({a: 3});
var models = [
model1, {a: 1}, {a: 1}, {a: 1}, {a: 2}, model2
];
var coll = new Backbone.Collection(models, {
comparator: 'a'
});

equal(coll.search(1).get('a'), 1);
equal(coll.search(1, {getMin: true}), coll.at(1));
equal(coll.search(1, {getMax: true}), coll.at(3));
equal(coll.search(2, {returnIndex: true}), 4);
equal(coll.search(3, {returnIndex: true}), 5);
equal(coll.search(4, {returnIndex: true}), -1);
ok(!coll.search(4));

equal(coll.search(function (model) {
var v = model.get('a');
if (v < 2) return 1;
if (v > 2) return -1;
return 0;
}).get('a'), 2);

var col2 = new Backbone.Collection(models, {
comparator: function (m) {
return 5 - m.get('a');
}
});
equal(col2.search(model1, {returnIndex: true}), 5);
equal(col2.search(model2, {returnIndex: true}), 0);

var col3 = new Backbone.Collection(models, {
comparator: function (m1, m2) {
return m2.get('a') - m1.get('a');
}
});
equal(col3.search(model1), model1);
equal(col3.search(model2), model2);
});

test("where and findWhere", 16, function() {
var model = new Backbone.Model({a: 1});
var coll = new Backbone.Collection([
var models = [
model,
{a: 1},
{a: 1, b: 2},
{a: 2, b: 2},
{a: 3}
]);
];
var coll = new Backbone.Collection(models);
equal(coll.where({a: 1}).length, 3);
equal(coll.where({a: 2}).length, 1);
equal(coll.where({a: 3}).length, 1);
Expand All @@ -564,6 +607,18 @@
equal(coll.where({a: 1, b: 2}).length, 1);
equal(coll.findWhere({a: 1}), model);
equal(coll.findWhere({a: 4}), void 0);

var col2 = new Backbone.Collection(models, {
comparator: 'a'
});
equal(col2.where({a: 1}).length, 3);
equal(col2.where({a: 2}).length, 1);
equal(col2.where({a: 3}).length, 1);
equal(col2.where({b: 1}).length, 0);
equal(col2.where({b: 2}).length, 2);
equal(col2.where({a: 1, b: 2}).length, 1);
equal(col2.findWhere({a: 1}), model);
equal(col2.findWhere({a: 4}), void 0);
});

test("Underscore methods", 16, function() {
Expand Down