Skip to content

Commit

Permalink
Remove Underscore dependency. Model.remove method now takes a model i…
Browse files Browse the repository at this point in the history
…nstance rather than an id.
  • Loading branch information
benpickles committed Jul 16, 2010
1 parent fa1814e commit 4a07a44
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 39 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.markdown
@@ -1,5 +1,7 @@
# Changelog

* Remove Underscore dependency.
* `Model.remove` method now takes a model instance rather than an id.
* Rename `sort` to `sortBy` and add the ability to specify an attribute name as well as a custom function. Add `sort` method that acts on a collection just like `Array#sort`.
* Add `pluck` method that takes an attribute name and returns an array of values.
* Fix for callbacks being wrongly called on multiple instances - they were being stored on the prototype and thus being shared across instances. Thanks to Oliver Nightingale for identifying the bug and writing a test case.
Expand Down
15 changes: 11 additions & 4 deletions src/model_class_methods.js
Expand Up @@ -71,10 +71,17 @@ Model.ClassMethods = {
return plucked
},

remove: function(id) {
var ids = _.invoke(this.collection, 'id');
var index = _.indexOf(ids, id);
if (index > -1) {
remove: function(model) {
var index

for (var i = 0; i < this.collection.length; i++) {
if (this.collection[i] === model) {
index = i
break
}
}

if (index != undefined) {
this.collection.splice(index, 1);
this.trigger("remove");
return true;
Expand Down
4 changes: 2 additions & 2 deletions src/model_instance_methods.js
Expand Up @@ -5,7 +5,7 @@ Model.InstanceMethods = {
return jQuery.extend({}, this.attributes, this.changes);
} else if (arguments.length === 2) {
// Don't write to attributes yet, store in changes for now.
if (_.isEqual(this.attributes[name], value)) {
if (this.attributes[name] === value) {
// Clean up any stale changes.
delete this.changes[name];
} else {
Expand Down Expand Up @@ -34,7 +34,7 @@ Model.InstanceMethods = {
if (method === "create") {
self.constructor.add(self);
} else if (method === "destroy") {
self.constructor.remove(self.id());
self.constructor.remove(self)
}
};

Expand Down
17 changes: 0 additions & 17 deletions test/public/javascripts/underscore-min.js

This file was deleted.

8 changes: 4 additions & 4 deletions test/tests/model_callbacks.js
Expand Up @@ -12,7 +12,7 @@ test("class-level", function() {
results.push(this);
results.push("add");
for (var i = 0; i < arguments.length; i++) {
results.push(arguments[i]);
results.push(arguments[i].id())
};
});

Expand All @@ -36,13 +36,13 @@ test("class-level", function() {
Post.add(post1, post2);
Post.add(post1);
Post.add(post3);
Post.remove(1);
Post.remove(post1);
Post.remove(666);
Post.trigger("custom");

same(results, [
Post, "add", post1, post2,
Post, "add", post3,
Post, "add", 1, 2,
Post, "add", 3,
Post, "remove",
Post, "custom",
Post, "custom-2"
Expand Down
26 changes: 15 additions & 11 deletions test/tests/model_class_methods.js
Expand Up @@ -15,17 +15,17 @@ test("all, count, find, first, add, remove", function() {

Post.add(post1, post2).add(post3);

same(Post.all(), [post1, post2, post3]);
same(Post.pluck("id"), [1, 2, 3]);
equals(Post.count(), 3);
equals(Post.find(1), post1);
equals(Post.find(2), post2);
equals(Post.find(3), post3);
equals(Post.find(4), null);
equals(Post.first(), post1);

ok(Post.remove(2));
ok(Post.remove(post2));

same(Post.all(), [post1, post3]);
same(Post.pluck("id"), [1, 3]);
equals(Post.count(), 2);
equals(Post.find(1), post1);
ok(Post.find(2) === null);
Expand All @@ -37,7 +37,7 @@ test("all, count, find, first, add, remove", function() {
var post1_duplicate = new Post({ id: 1 });
Post.add(post1_duplicate);

same(Post.all(), [post1, post3], "shouldn't be able to add if a model with the same id exists in the collection");
same(Post.pluck("id"), [1, 3], "shouldn't be able to add if a model with the same id exists in the collection");
});

test("detect, select, first, last, count (with chaining)", function() {
Expand Down Expand Up @@ -186,14 +186,18 @@ test("sort (and chaining)", function() {
test("Custom `all` method", function() {
var Post = Model('post', {
all: function() {
return _.sortBy(this.collection, function(model) {
return model.attr("position");
});
var not_deleted = []
var i, model
for (i = 0; i < this.collection.length; i++) {
model = this.collection[i]
if (!model.attr("deleted")) not_deleted.push(model)
}
return not_deleted
}
});
var post1 = new Post({ id: 1, position: 2 });
var post2 = new Post({ id: 2, position: 3 });
var post3 = new Post({ id: 3, position: 1 });
var post1 = new Post({ id: 1, deleted: false })
var post2 = new Post({ id: 2, deleted: true })
var post3 = new Post({ id: 3, deleted: false })

Post.add(post1, post2, post3);

Expand All @@ -203,7 +207,7 @@ test("Custom `all` method", function() {
results.push(this.id());
});

equals(results.join(", "), "3, 1, 2", "`each` should iterate over `all`");
same(results, [1,3], "`each` should iterate over `all`");
});

test("Custom method with chaining, then more chaining", function() {
Expand Down
1 change: 0 additions & 1 deletion test/views/index.erb
Expand Up @@ -8,7 +8,6 @@

<!-- Dependencies -->
<script src="javascripts/jquery-1.4.1.min.js"></script>
<script src="javascripts/underscore-min.js"></script>

<!-- js-model -->
<script src="src/model.js"></script>
Expand Down

0 comments on commit 4a07a44

Please sign in to comment.