Multiple-choice quiz

Question

I have been working on improving my javascript skills, mainly "namespacing" and the best practices used by js developers. I would like to know how I can improve this code and any tips that may help me along the way. Below is the specific code I am working with and there is a working fiddler also.

Just to note, I only want to use straight javascript and not jQuery for this.

var quiz = window.quiz || {};

(function(ns) {
var currentIndex = 0;
var parent, back, next;
var selectedChoices = [];

ns.version = '1.0.0';

ns.author = 'Anonymous Hands';

ns.parentId = function(id) { parent = document.getElementById(id); };

ns.nextButtonId = function(id) { next = document.getElementById(id); next.onclick = moveNext; };

ns.backButtonId = function(id) { back = document.getElementById(id); back.onclick = moveBack; };

ns.questions = [];

ns.render = 
    function() {
        renderQuestion(0);
    };

//Pass results and missed questions?
ns.onComplete = function(results){};

function moveNext(){
    saveState();

    if (!atEnd()){
        currentIndex++;
        renderQuestion(currentIndex);
        restoreState();

        return;
    }

    renderComplete();
}

function moveBack(){
    saveState();

    if(!atBeginning()){
        currentIndex--;
        renderQuestion(currentIndex);
        restoreState();
    }
}

function renderQuestion(index){
    clearParent();

    var questionNum = index + 1;
    var node = ns.questions[index];

    var questionNumDiv = makeElement({type: "div", class: "question-number" ,html: questionNum});

    var questionText = makeElement({type: "div", class: "question-text" ,html: node.question});

    var choicesDiv = makeElement({type: "div", class: "choices"});

    var l = node.choices.length;

    for(var i = 0; i < l; i++){
        var choiceRadio = makeElement({
            type: "input", 
            subtype: "radio", 
            name: "choices", 
            id: "choice-" + i
            });

        var choiceLabel = makeElement({type: "label", class: "choice", html: node.choices[i], for: "choice-" + i});

        choicesDiv.appendChild(choiceRadio);
        choicesDiv.appendChild(choiceLabel);
    }

    parent.appendChild(questionNumDiv);
    parent.appendChild(questionText);
    parent.appendChild(choicesDiv);
}

function renderComplete(){
    clearParent();

    var n = getTotalCorrect();
    var d = ns.questions.length;
    var p = Math.round(((n / d) * 100))

    var totalDiv = makeElement({type: "div", id: "score-total", html: "Your score: " + n + "/" + d});

    var percentDiv = makeElement({type: "div", id: "score-percentage", html: p + "%"});

    parent.appendChild(totalDiv);
    parent.appendChild(percentDiv);

    back.style.display = "none";
    next.style.display = "none";

    //onComplete();
}

function saveState(){
    var anwser = selectedChoices[currentIndex];
    var userChoice = getUserChoice();

    if(anwser){
        anwser.choice = userChoice;

        return;
    }

    selectedChoices.push({questionId: currentIndex, choice: userChoice});
}

function restoreState(){
    var anwser = selectedChoices[currentIndex];

    if(anwser && anwser.choice != null){
        var choices = document.getElementsByName("choices");

        choices[anwser.choice].checked = true;
    }
}

function clearParent(){
    parent.innerHTML = "";
}

function atBeginning(){
    return (currentIndex <= 0) ? true : false;
}

function atEnd(){
    return (currentIndex >= ns.questions.length - 1) ? true : false;
}

function getTotalCorrect(){
    var correct = 0;

    var l = ns.questions.length;

    for(var i = 0; i < l; i++){
        if (ns.questions[i].anwser === selectedChoices[i].choice) correct++;
    }

    return correct;
}

function makeElement(p){
    var e = document.createElement(p.type);

    if (p.id) e.id = p.id;
    if (p.class) e.className = p.class;
    if (p.name) e.name = p.name;
    if (p.html) e.innerHTML = p.html;
    if (p.for) e.setAttribute("for", p.for);
    if (e.type) e.type = p.subtype;

    return e;
}

function getUserChoice(){
    var choices = document.getElementsByName("choices");
    var l = choices.length;
    var index = null;

    for(var i = 0; i < l; i++){
        var node = choices[i];

        if (node.checked) index = i;
    }

    return index;
}

})(quiz);

The jsfiddle, as promised.

Answer 1

I tend to use a lot of Java best practices in my JavaScript code. The biggest issue I see is the lack of extensible classes/methods. The methods are procedural and only seem to be broken into new methods because they are repeatable. The class as a whole violates the single responsibility principle because it has to handle the entire widget from modifying input (also bad practice), parsing the input, managing the question state, and appending the view. By following SRP your code will become much easier to read, maintain, and extend. I'm not going to comment on your grammatical issues but you should definitely set up Sonar to scan your code if you haven't already.

So how it could be improved?

I would start be making a new class called Question that could take a single question model and display it on the page. Then create a Questionnaire class that can take a questionnaire model (metadata & list of questions) and handle the state transitions until all questions have been answered. Questionnaire will have to instantiate a new Question object inside a div.

I create the classes this way which allows the class to maintain scope.

function Question(divId, question) {

  // private variables
  var myVar;

  // this is a public method
  this.getAnswer = function() {
  }

  // this is a private method
  function renderChoice() {
  }

  function init() {
    // build the view
  }

  // this method call initializes the class like a constructor
  // so the caller doesn't have to initialize
  init();
}

There are then two ways the state transitions could be handled. POST the answer after every question and store the values at the server or store each question result in the Questionnaire and POST at the end. Don't store the answers in the code because anyone that knows how to access and read the code could cheat. Either way you choose make sure that each class knows as little about each other as possible. Question should know nothing about Questionnaire and Questionnaire should only know that is has a list of questions to loop through and use accessor methods to get the value from.