tastejs / todomvc

Helping you select an MV* framework - Todo apps for React.js, Ember.js, Angular, and many more
http://todomvc.com
Other
28.63k stars 13.77k forks source link

Extraneous param to filterAll in BackboneJS implementation? #698

Closed marcpare closed 11 years ago

marcpare commented 11 years ago

Hi there, I'm wondering what is the purpose of adding the context param to the each call in filterAll function in the BackboneJS implementation?

Here's the current code:

filterOne: function(todo){
      todo.trigger('visible');
},
filterAll: function(){
    app.todos.each(this.filterOne, this);
},

Appears to work correctly removing the context param to each

filterAll: function(){
    app.todos.each(this.filterOne);
}

Am I missing one of the execution paths for this?

passy commented 11 years ago

It does because filterOne is not actually relying on the binding of this to the current object. If, however, at some point in the future the code would change and would add a dependency on this, it would break. I think keeping the context parameter around there is good style.

marcpare commented 11 years ago

Makes sense, thanks.