# Is this a correct knockout ViewModel setup?

This is my first time using knockout. I'm reading/writing a JSON file with a fair bit of nested data. I had no problem creating the ViewModel to just write to the file, but now that I'm reading the data into my model also, I've had to write a few if/else statements to handle the different cases. Not sure if this is the right way to do it.

I've got a ViewModel with a function to add a new record, and to save the data. The 3 functions above are used for mapping the data.

function Survey(name, panes, tester)
{
this.name = ko.observable(name);
this.panes = ko.observableArray([new Pane(panes)]);
this.tester = ko.observable(tester);
}

function Pane(panes)
{
//var panesCount = panes.length;
//console.log(panes[0].type);
if (panes) {
this.type = ko.observable(panes[0].type);
this.content = ko.observable(panes[0].content);
this.options = ko.observableArray([new PaneOptions(panes[0].options)]);
}
else {
this.type = ko.observable();
this.content = ko.observable();
this.options = ko.observableArray([new PaneOptions()]);
}
}

function PaneOptions(options)
{
//console.log(options);
if (options) {
this.data = ko.observable(options[0].data);
this.target = ko.observable(options[0].target);
}
else {
this.data = ko.observable();
this.target = ko.observable();
}
}

function SurveyViewModel()
{
var self = this;
self.surveys = ko.observableArray([]);

$.getJSON("create/fetch", function(data) { //console.log(data.surveys[0].name); if (data) { // check if json file is empty for (var i=0; i < data.surveys.length; i++) { console.log(i); self.surveys.push(new Survey(data.surveys[i].name, data.surveys[i].panes, data.surveys[i].tester)) } } }); self.addSurvey = function () { self.surveys.push(new Survey()); }; self.saveUserData = function() { var data_to_send = ko.toJSON(self);$.post("create/save", data_to_send, function(data) {
console.log("Your data has been posted to the server!");
});
}

}

var vm = new SurveyViewModel();
ko.applyBindings(vm);


Here you can see the JSON representation:

{
"surveys": [
{
"name": "My First Survey1",
"panes": [
{
"type": "question1",
"content": "multichoice1",
"options": [
{
"data": "Time",
"target": 2
}
]
}
],
"tester": "default"
},
{
"name": "My Second Survey2",
"panes": [
{
"type": "response2",
"content": "multichoice2",
"options": [
{
"data": "Time",
"target": 2
}
]
}
],
"tester": "default2"
},
{
"name": "My Third Survey3",
"panes": [
{
"type": "thanks3",
"content": "multichoice3",
"options": [
{
"data": "Time",
"target": 2
}
]
}
],
"tester": "default3"
}
]
}


Any input is greatly appreciated!

Overall, it looks really good and serves as a pretty great model of what Knockout models should look like, but, internally there are some strange things here like arrays that you are using [0] only on (this could be the result of poor data format via your backend web services) and some ambiguity between Panes, Pane, and PaneOptions.

### (2) Fix ambiguity with singular and plural constructors (Planes, Plane)

The relationship between Panes and Pane are a little strange! In the Survey constructor, you are create a variable called panes (this.panes) and then instantiating and passing a new Pane to which you are passing an array of panes? A little bit recursive. In the Pane constructor I would not expect to take in panes in the arguments. I think your intention is for the Survey to create an array of panes, like this:

function Survey(name, panes, tester)
{
var _this = this;

// 1
this.name = ko.observable(name || '');
// 2
this.panes = ko.observableArray();
this.tester = ko.observable(tester);

if(panes){
panes.forEach(function(pane){
_this.panes.push(new Pane(pane));
});
}
}


### (3) Fix improper use of arrays with fixed [0] element

Why is Panes plural in the Pane constructor, if you are only using array item 0? I think the Pane constructor should just take in an parameter like options which would be used to return an individual pane:

function Pane(config)
{
this.type = ko.observable(config.type);
this.content = ko.observable(config.content);
this.options = ko.observableArray([new PaneOptions(config.options)]);

/*
* You could return "this" but in JS that is the default behavior.
*/
}


Additionally, this will help you avoid JS errors when you might attempt to access a null array. For example, if panes in panes[0].options were null, you would have an error at pane[0] before you even got to the options property.

I am not sure why you are operating on options[0] here - at the point when you are instantiating a Pane, you should be passing in a config (see above) which has an options property, which will most likely be an object like { data: "", target: "" } since that is what you are expecting in the PaneOptions constructor:

function PaneOptions(options)
{
// Setup defaults with ternary operator
this.data = ko.observable(options ? options.data : '');
this.target = ko.observable(options ? options.target : '');
}


Applicable original code with numbered callouts corresponding to items above

function Survey(name, panes, tester)
{
// 1
this.name = ko.observable(name || '');
// 2
this.panes = ko.observableArray([new Pane(panes)]);
this.tester = ko.observable(tester);
}

// 2
function Pane(panes)
{
if (panes) {
// 3
this.type = ko.observable(panes[0].type);
this.content = ko.observable(panes[0].content);
this.options = ko.observableArray([new PaneOptions(panes[0].options)]);
}
else {
this.type = ko.observable();
this.content = ko.observable();
this.options = ko.observableArray([new PaneOptions()]);
}
}

function PaneOptions(options)
{
if (options) {
// 3
this.data = ko.observable(options[0].data);
this.target = ko.observable(options[0].target);
}
else {
this.data = ko.observable();
this.target = ko.observable();
}
}