snippetjavascriptMinor
Backbone collection filter
Viewed 0 times
backbonecollectionfilter
Problem
I've got this Collection in my Backbone application:
This works, and randomSet is called by a View when config have certain options or user click on 'rerender' button, but I'm wondering that maybe I can improve this code ?
The flow looks like this
This code doesn't look to cool, maybe you will have some ideas how to make It cooler?:)
var AnswersCollection = Backbone.Collection.extend({
model: Answer,
initialize: function() {
console.log('Hello from new answers collection');
},
getCorrect: function() {
return this.where({correct: true});
},
getWrong: function() {
return this.where({correct: false});
},
randomSet: function(correct, wrong) {
arr1 = _.sample(this.getCorrect(), correct);
arr2 = _.sample(this.getWrong(), wrong);
result = _.shuffle(arr1.concat(arr2));
coll = new AnswersCollection(result)
return coll;
}
});This works, and randomSet is called by a View when config have certain options or user click on 'rerender' button, but I'm wondering that maybe I can improve this code ?
The flow looks like this
- Get randomly selected X correct answers
- Get randomly selected Y wrong answers
- Shuffle all answers
- Create array of answers, that can be passed to new Collection
This code doesn't look to cool, maybe you will have some ideas how to make It cooler?:)
Solution
Reviewing code for coolness is a first ;)
Some observations:
-
-
-
-
The most serious problem though is that you are not declaring the prior mentioned variables with
Also, from a coolness perspective, braces on their own line is where it's at.
I would counter-propose this:
Some observations:
-
initialize : if really all you want to do there is Hello World, then you should take it out. initialize is meant for initializing your model with answers.-
getCorrect and getWrong are short one-liners that are called once, you should inline them-
arr1, arr2, coll and even result are pretty terrible names, how about correctAnswers, wrongAnswers, answers and not creating coll since you could return the results of new AnswersCollection immediately.-
The most serious problem though is that you are not declaring the prior mentioned variables with
var, that is bad.Also, from a coolness perspective, braces on their own line is where it's at.
I would counter-propose this:
var AnswersCollection = Backbone.Collection.extend(
{
model: Answer,
randomSet: function(correct, wrong)
{
var correctAnswers = _.sample(this.where({correct: true}), correct),
wrongAnswers = _.sample(this.where({correct: false}), wrong),
answers = _.shuffle(correctAnswers.concat(wrongAnswers));
return new AnswersCollection( answers );
}
});Code Snippets
var AnswersCollection = Backbone.Collection.extend(
{
model: Answer,
randomSet: function(correct, wrong)
{
var correctAnswers = _.sample(this.where({correct: true}), correct),
wrongAnswers = _.sample(this.where({correct: false}), wrong),
answers = _.shuffle(correctAnswers.concat(wrongAnswers));
return new AnswersCollection( answers );
}
});Context
StackExchange Code Review Q#40503, answer score: 4
Revisions (0)
No revisions yet.