HiveBrain v1.2.0
Get Started
← Back to all entries
snippetjavascriptMinor

Backbone collection filter

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
backbonecollectionfilter

Problem

I've got this Collection in my Backbone application:

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:

-
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.