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\$
2
  • \$\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, 2014 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, 2014 at 16:27

2 Answers 2

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
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\$

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