Planning Poker Using Knockout

Question

I am trying to build something similar to planning poker and am very new to Knockout and was wondering if anyone could help me improve on my very crude start?

This is what I have so far:

HTML

Selected Card

CSS

 .profile { width: 50px; height: 80px; color: #FFF; background: black; border: 1px solid   #FFF; float: left; line-height:80px } 
 .highlight { background: yellow !important; border:1px solid #000; color: black; }

JavaScript/Knockout

function Step(number) {
    this.active = ko.observable(false);
    this.name = ko.observable( number);
}

var model = function () {
    var items = ko.observableArray([new Step(1), new Step(2), new Step(3), new Step(5),new Step(8),new Step(13),new Step(20),new Step(40),new Step(100)]);
    var selectedItems = ko.computed(function () {
        return _.filter(items(), function (item) {
            return item.active();
        });
    })

    var clicked = function (item) {
        items().forEach(function(item){ item.active(false) });           
        item.active(!this.active());
    };

    var save = function () {
        alert("sending items \n" + ko.toJSON(selectedItems()));
    }

    return {
        items: items,
        selectedItems: selectedItems,
        save: save,
        clicked: clicked
    }
}

ko.applyBindings(model);

http://jsfiddle.net/grahamwalsh/63yeD/

Answer 1

You are using function as definition for model and inside that function you are defined an interface to have access to those properties/methods. In complicated cases you would have inflexible usage between definition and public interface. Better solution would be to create definition for the class like this

var Model = function () {
   var self = this;
   self.selectedItems = ko.computed(function () {
        return _.filter(items(), function (item) {
            return item.active();
        });
    })
}

and then create an instance of it

var model = new Model();

because a new operator you will create new JS object and this object will be send as scope for current this content. This is commonly used patterns in order to follow OOJ see link here https://developer.mozilla.org/en-US/docs/Web/JavaScript/Introduction_to_Object-Oriented_JavaScript

Answer 2

A surprisingly good start, most knockout code looks terrible to me, but I can follow this, it's well laid out, JsHint has nothing to complain about.

Only this bothered me:

var items = ko.observableArray([new Step(1), new Step(2), new Step(3), new Step(5),new Step(8),new Step(13),new Step(20),new Step(40),new Step(100)]);

I would have approached that a little differently:

var steps = [1,2,3,5,8,13,20,40,100];
for ( var i = 0 ; i < steps.length ; i++ )
  steps[i] = new Step( steps[i] );
var items = ko.observableArray( steps )

What this does is more clearly show the different steps from 1-> 100, and take out the repetition in your code of calling new Step for every single step.

Answer 3

I'm only going to focus on this piece of code since I'm not a knockout expert:

var selectedItems = ko.computed(function () {
    return _.filter(items(), function (item) {
        return item.active();
    });
})

First of all I would call this variable selectedItem since you can only have one and not multiple selected items. Secondly UnderscoreJS provides you a function called find. It's in my opinion more clear to use this function instead of filter because filter can return multiple results.

var selectedItem = ko.computed(function () {
    return _.find(items(), function (item) {
        return item.active();
    });
})

Another small remark, add semicolons after your statements. Consider the following:

var save = function() {

}
// Another developer adds an anonymous function after your save function.
// This will break your code
(function() {

})();

It won't break your code if you add semicolons:

var save = function() {

};
(function() {
   // This will work
})();

This is not necessary if you define a function as follows:

function save() {

}
(function() {
    // This will also work
})();