Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

For a quiz web app, I decided the best way to store quiz questions, answers, and other user data related to the question was to use an array of objects for the quiz. I'm not sure if this is the best way to go about it since static information such as the questions and answers will be mixed with user data.

Each object in the array contains the quiz question, answer, if the user has enabled the question, and if the question has already been asked during the quiz session.

During a quiz session I wish to ask all questions that are enabled and haven't been asked yet. My script works but I am concerned it is inefficient (with a bank of 1000+ questions)? Is it alright to store user data with my quiz questions? Will this make adding additional questions down the road more difficult? Can I improve what I have or should I look for a different approach?

My script: JS Bin: http://jsbin.com/owehix/1/edit

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
<title>Untitled Document</title>
<script>
window.onload = function() {

    /////////////////////////////////////////////////
    // CREATE SAMPLE QUIZ ARRAY TO USE IN EXAMPLE //
    /////////////////////////////////////////////////

    // Quiz constructor
    function quizConstructor(question, answer, enabled, asked) {
        this.question = question;
        this.answer = answer;
        this.enabled = enabled;
        this.asked = asked;
    }

    // Create quiz array
    var quiz = new Array();

    // All quiz questions and answers
    quiz[0] = new quizConstructor("Montgomery", "Alabama", false, 0);
    quiz[1] = new quizConstructor("Juneau", "Alaska", true, 0);
    quiz[2] = new quizConstructor("Phoenix", "Arizona", true, 0);
    quiz[3] = new quizConstructor("Little Rock", "Arkansas", false, 0);
    quiz[4] = new quizConstructor("Sacramento", "California", true, 0);
    quiz[5] = new quizConstructor("Denver", "Colorado", false, 0);
    quiz[6] = new quizConstructor("Hartford", "Connecticut", false, 0);
    quiz[7] = new quizConstructor("Dover", "Delaware", false, 0);
    quiz[8] = new quizConstructor("Tallahassee", "Florida", false, 0);
    quiz[9] = new quizConstructor("Atlanta", "Georgia", true, 0);

    /////////////////////////////////////////////////
    // END: CREATE SAMPLE QUIZ ARRAY TO USE IN EXAMPLE //
    /////////////////////////////////////////////////


    // Find the number of questions that the user has enabled
    var numEnabled = 0;

    for (var i = 0; i < quiz.length; i++) {
        if (quiz[i].enabled == true) {
            numEnabled++;
        }
    }


    // Ask all enabled questions in random order
    for (var i = 0; i < numEnabled; i++) {

        // Find random question that hasn't been asked yet
        do {

            var randomNum = Math.floor(Math.random() * quiz.length);

        } while (quiz[randomNum].enabled == false || quiz[randomNum].asked == 1);

        // Ask question
        var question = quiz[randomNum].question + " is the capital of which state?";
        document.getElementById("divSolution").innerHTML += "<p>" + question + "</p>";

        // Mark question as asked
        quiz[randomNum].asked++;
    }
}
</script>
</head>

<body>
<div id="divSolution"></div>
</body>
</html>
share|improve this question

3 Answers 3

How you're making the questions really looks ugly. How about this way?

var quiz = [{
    question: "Montgomery",
    answer: "Alabama",
    enabled: false,
    asked: 0
},{
    question: "Juneau",
    answer: "Alaska",
    enabled: true,
    asked: 0
}, {} /* ... */ ];

// Now instantiate every quiz
quiz = quiz.map(function(el) {
    return new quizConstructor(el.question, el.answer, el.enabled, el.asked);
});

Now if you're not doing anything more with your quizConstructor object, you can just as well use plain objects. (What's done before instantiating every quiz up there.)

share|improve this answer
    do {

        var randomNum = Math.floor(Math.random() * quiz.length);

    } while (quiz[randomNum].enabled == false || quiz[randomNum].asked == 1);

Instead of repeating until finding a question asked exactly once, keep a set of questions to be asked and remove questions that won't be asked anymore from that set. With more than 1000 questions above loop will run ~700 times for the last question. And in 1% of the occasions that loop will run ~4600 times for the last question. Your users may or may not notice the performance difference but it's just wasteful.

Keeping a set of questions to be asked also makes the criteria used for deciding which questions asked anymore more configurable, modifiable. Imagine what would you change if you would ask every question twice or keep asking questions until they were answered correctly thrice in a row, etc.

Moreover, since US only has 50 states and a handful of territories, and you plan to have a collection of 1000 questions, this part should be somewhere else:

    var question = quiz[randomNum].question + " is the capital of which state?";
share|improve this answer

For adaptability I would set up a Quiz class that can ask any question, not just one about state capitals, then have another object or function that makes questions specifically about state capitals. Also, you can attach a method to the Quiz class so each question can 'ask' itself; you will probably want to add another method that also checks the user's answer.

A more efficient way to get all the enabled questions in random order is to make an array of the enabled questions (enabled) then splice a random element from enabled until it is empty. (In fact, in the code below, the array quiz is not used at all, only enabled. Maybe you want to do something later with the non-enabled questions?)

(JSFiddle for the code below and another one using prototypes to set up StateQuiz as a class).

function Quiz(question, answer, enabled, asked) {
  this.question = question;
  this.answer = answer;
  this.enabled = enabled == false ? false : true;
  this.asked = asked ? asked : 0;
}
Quiz.prototype.ask = function () {
  document.getElementById("divSolution").innerHTML += "<p>" + this.question + "</p>";
  this.asked++;
}
function stateQuiz(capital, state, enabled) {
  return new Quiz(capital + ' is the capital of which state?', state, enabled);
}

// Create quiz array
var question, 
  quiz = [],
  enabled = [],
  stateCapitals = [
    ['Montgomery', 'Alabama', false],
    ['Juneau', 'Alaska', true],
    ['Phoenix', 'Arizona', true],
    ['Little Rock', 'Arkansas', false],
    ["Sacramento", "California", true],
    ["Denver", "Colorado", false],
    ["Hartford", "Connecticut", false],
    ["Dover", "Delaware", false],
    ["Tallahassee", "Florida", false],
    ["Atlanta", "Georgia", true]
  ];

for (var i = 0; i < stateCapitals.length; i++) {
  question = stateQuiz(stateCapitals[i][0], stateCapitals[i][1], stateCapitals[i][2]);
  quiz.push(question);
  if (question.enabled) enabled.push(question);
}
// Ask all enabled questions in random order
while (enabled.length) {
  // splice removes an element from the array and returns the removed element
  question = enabled.splice(Math.floor(Math.random() * enabled.length), 1)[0];
  question.ask();
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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