Knockout first-timer

Question

Looking at knockout for the first time, would like to know if I'm approaching it correctly.

Here's a live example: http://jsfiddle.net/fergal_doyle/xhAe8/1/

I have a "car" select, and an "other" input to allow values not present in the select box. When "other" is populated, it over-rides "car" by disabling it and a computed observable is set to the actual chosen car. Only when "other" is emptied, will "car" become enabled again.

HTML:

<label for="car">Car</label>
<select id="car" data-bind="options: cars, value: selectedCar, enable: checkCar"></select>

<label for="other">Other</label>
<input id="other" type="text" data-bind="value: other, valueUpdate:'afterkeydown'"/>

<p>Chosen value: <span data-bind="text: chosenCar"></span></p>

JS:

function viewModel(){
    var self = this;

    self.cars = ko.observableArray(["Chrysler", "Chevrolet", "Ford", "Lincoln"]);
    self.other = ko.observable("");
    self.selectedCar = ko.observable("");

    self.chosenCar = ko.computed(function() {
        return self.other() || self.selectedCar();
    }); 

    self.checkCar = ko.computed(function(){
        if(self.other())
            return false;
        else
            return true;
    });
}

ko.applyBindings(new viewModel());

Answer 1

It looks fine to me. My only real criticism is your checkCar implementation. It ought to be written out as:

self.checkCar = ko.computed(function(){
    return !self.other();
});

On a minor note, you should case your view model name in PascalCase. By making constructors like that, they stand out better. Everything else should be in camelCase.

---

Other than that, you've made great use of the computed observables. A big mistake I see a lot on Stack Overflow IMHO is when I see people putting all that logic in the view.

I might change the name of the properties however to be more descriptive of what they represent. In particular, change checkCar to isSelectedCarOverridden.

Personally, I'd change it around a bit. I'd add an "Other" option in the cars list and make the Other car input enabled/visible based on that value.

Also, consider making cars not observable. If this is going to be a fixed list and you won't be dynamically adding to it, making it observable may be unnecessary. In fact, I'd pull that array out of the view model as it appears to be a constant value. That way if you have multiple view models instantiated in the future, they all can share the same instance.

var constants = {
    cars: ["Chrysler", "Chevrolet", "Ford", "Lincoln"]
};

function ViewModel() {
    var self = this;
    self.cars = constants.cars;
    // ...
}