10
\$\begingroup\$

Please review the code quality of this JavaScript Quiz App.

Code improvement tips and simplify are welcome.

Demo - http://jsbin.com/eRiSUhIB/1/

var quiz = [{
  "question": "What is the full form of IP?",
  "choices": ["Internet Provider", "Internet Port", "Internet Protocol"],
  "correct": "Internet Protocol"
}, {
  "question": "Who is the founder of Microsoft?",
  "choices": ["Bill Gates", "Steve Jobs", "Steve Wozniak"],
  "correct": "Bill Gates"
}, {
  "question": "1 byte = ?",
  "choices": ["8 bits", "64 bits", "1024 bits"],
  "correct": "8 bits"
}, {
  "question": "The C programming language was developed by?",
  "choices": ["Brendan Eich", "Dennis Ritchie", "Guido van Rossum"],
  "correct": "Dennis Ritchie"
}, {
  "question": "What does CC mean in emails?",
  "choices": ["Carbon Copy", "Creative Commons", "other"],
  "correct": "Carbon Copy"
}];


// define elements
var content = $("content"),
  questionContainer = $("question"),
  choicesContainer = $("choices"),
  scoreContainer = $("score"),
  submitBtn = $("submit");

// init vars
var currentQuestion = 0,
  score = 0,
  askingQuestion = true;

function $(id) { // shortcut for document.getElementById
  return document.getElementById(id);
}

function askQuestion() {
  var choices = quiz[currentQuestion].choices,
    choicesHtml = "";

  // loop through choices, and create radio buttons
  for (var i = 0; i < choices.length; i++) {
    choicesHtml += "<input type='radio' name='quiz" + currentQuestion +
      "' id='choice" + (i + 1) +
      "' value='" + choices[i] + "'>" +
      " <label for='choice" + (i + 1) + "'>" + choices[i] + "</label><br>";
  }

  // load the question
  questionContainer.textContent = "Q" + (currentQuestion + 1) + ". " +
    quiz[currentQuestion].question;

  // load the choices
  choicesContainer.innerHTML = choicesHtml;

  // setup for the first time
  if (currentQuestion === 0) {
    scoreContainer.textContent = "Score: 0 right answers out of " +
      quiz.length + " possible.";
    submitBtn.textContent = "Submit Answer";
  }
}

function checkAnswer() {
  // are we asking a question, or proceeding to next question?
  if (askingQuestion) {
    submitBtn.textContent = "Next Question";
    askingQuestion = false;

    // determine which radio button they clicked
    var userpick,
      correctIndex,
      radios = document.getElementsByName("quiz" + currentQuestion);
    for (var i = 0; i < radios.length; i++) {
      if (radios[i].checked) { // if this radio button is checked
        userpick = radios[i].value;
      }

      // get index of correct answer
      if (radios[i].value == quiz[currentQuestion].correct) {
        correctIndex = i;
      }
    }

    // setup if they got it right, or wrong
    var labelStyle = document.getElementsByTagName("label")[correctIndex].style;
    labelStyle.fontWeight = "bold";
    if (userpick == quiz[currentQuestion].correct) {
      score++;
      labelStyle.color = "green";
    } else {
      labelStyle.color = "red";
    }

    scoreContainer.textContent = "Score: " + score + " right answers out of " +
      quiz.length + " possible.";
  } else { // move to next question
    // setting up so user can ask a question
    askingQuestion = true;
    // change button text back to "Submit Answer"
    submitBtn.textContent = "Submit Answer";
    // if we're not on last question, increase question number
    if (currentQuestion < quiz.length - 1) {
      currentQuestion++;
      askQuestion();
    } else {
      showFinalResults();
    }
  }
}

function showFinalResults() {
  content.innerHTML = "<h2>You've complited the quiz!</h2>" +
    "<h2>Below are your results:</h2>" +
    "<h2>" + score + " out of " + quiz.length + " questions, " +
    Math.round(score / quiz.length * 100) + "%<h2>";
}

window.addEventListener("load", askQuestion, false);
submitBtn.addEventListener("click", checkAnswer, false);
\$\endgroup\$
  • \$\begingroup\$ If you rightclick you can see the correct answers.some time ago i wrote a script like this that loads the questions with ajax(but without the answers)... stackoverflow.com/a/18337257/2450730 \$\endgroup\$ – cocco Jan 25 '14 at 10:03
  • \$\begingroup\$ lol , yeah .. your right.Anyway i just added my code cause maybe you find something there what you could need.your code looks good. \$\endgroup\$ – cocco Jan 25 '14 at 16:27
6
\$\begingroup\$

Quite nice!

A general thought is that you could always store the correct answer first, and then show the possible answers in a random manner to make your code more DRY.

Furthermore:

  • Since you do not use the id anywhere, you can leave this out: "' id='choice" + (i + 1) +
  • You could put scoreContainer.textContent = "Score: " + score + " right answers out of " + quiz.length + " possible."; into it's own function which you could call from askQuestion() so that you do not need to repeat this with scoreContainer.textContent = "Score: 0 right answers out of " + quiz.length + " possible.";
  • Caching the elements is good
  • JSHint cannot complain about anything
  • checkAnswer has too much UI state management, it ought to just check the answer.
  • complited -> completed
  • Something to think about, finalResults could have read the HTML from a hidden DIV and simply filled in the missing parts. Now you are maintaining HTML in JavaScript which is rarely a good idea.

All in all, I like this code.

\$\endgroup\$
0
\$\begingroup\$

Just some teeny things:

Store the lengths of the arrays in a variable when referencing inside the for loops. This way the functions will only check for the lengths once instead of on each iteration.

For example...

function askQuestion() {
  var choices = quiz[currentQuestion].choices,
    choicesHtml = "",
    choicesLength = choices.length;

  // loop through choices, and create radio buttons
  for (var i = 0; i < choicesLength; i++) {

And...

// determine which radio button they clicked
var userpick,
  correctIndex,
  radios = document.getElementsByName("quiz" + currentQuestion),
  radiosLength = radios.length;

for (var i = 0; i < radiosLength; i++) {

And along the same lines, it may be good to store quiz.length in a variable as well. Especially since its referenced a couple of times throughout the script.

\$\endgroup\$

protected by syb0rg Mar 11 '14 at 2:23

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.