1
\$\begingroup\$

I'm learning JavaScript and have tried making a simple quiz from scratch.

JS Bin

//Define global variables
    var count = 0,  
          zCount = 0, 
          yCount = 0, 
          sumPoints = 0, 
          aQuestion, 
          allQuest, 
          aChoices, 
          bChoices, 
          allQuestions, 
          finishedDoc,
          someValue;

    allQuestions = [
            {
            question: "What is the capital of Peru?",
            choices: ["Buenos Aires", "La Paz", "Lima", "Quito"], 
            correctAnswer: 2
        },
            {
            question: "What is the capital of Bolivia?",
            choices: ["La Paz", "Buenos Aires", "Quito", "Lima"], 
            correctAnswer: 0
        },
            {
            question: "What is the capital of Ecuador?",
            choices: ["Lima", "Quito", "Buenos Aires", "La Paz"], 
            correctAnswer: 1
        },
            {
            question: "What is the capital of Argentina?",
            choices: ["Quito", "La Paz", "Lima", "Buenos Aires"], 
            correctAnswer: 3
        }
    ];

    function renderQuestions() {
        aQuestion = document.getElementById("thisQuestion");
        aChoices = document.getElementsByTagName("label");
        bChoices = document.getElementsByTagName("input");
        if(count < allQuestions.length){
            allQuest = allQuestions[count].question;
            count++;
        }
        else{
            alert("You have a total of " + sumPoints + "/4" + " correct answers!");
            finished();
        }
        aQuestion.innerHTML = allQuest;
        //render the choices and structure
        for(var i = 0; i < allQuestions.length; i++){
            aChoices[i].innerHTML = allQuestions[zCount].choices[i];
        }
        zCount++;


    }
    renderQuestions() //render first question
    function checkAnswer() {

        someValue = allQuestions[yCount].correctAnswer;
        if(bChoices[someValue].checked == true){
            alert("That's correct!");
            sumPoints += 1;
        }
        else{
            alert("Nope, sorry!");
        }
        yCount++;
        renderQuestions();
    }

    function finished(){
        finishedDoc = document.getElementById("thisCont");
        finishedDoc.innerHTML = "<h1>Finished!<\/h1>";
    }
\$\endgroup\$
  • 2
    \$\begingroup\$ And.... what is the question/review specifically? What expect of your code, be more specific plz. \$\endgroup\$ – vmariano Aug 11 '15 at 12:58
4
\$\begingroup\$

Here are some critiques. I focused on what is most important for a beginner to think about.

Use Objects

You have conceptually clear units of data you should consider breaking into formally declared objects with constructors. Take a look at this link, especially "Using an Object Constructor". In the long run, this will keep you from writing spaghetti code that is hard to debug, longer than it has to be, and lacking in conceptually-based structure. For example, I would think instead of your allQuestions array you should make the questions with an object constructor method and put them in the array. If you ever want to do anything with the questions later, this will save you a headache.

function Question(q, choices, answer) {
    this.question = q;
    this.choices = choices;
    this.correctAnswer = answer;
}

allQuestions = [...];    // using the class you defined above

Structure

Your structure is confusing. You declare a function, call that function, then declare some other functions. Why not keep all your function declarations together?

Timing

It can be tempting to just dump invocation of your functions into the <script> body, but it's cleaner to link these with an onload event. You can read more here. This way you have tighter control and understanding of when/why functions execute. For example you wouldn't want to display the quiz when you hadn't yet set up the necessary HTML elements to do so.

Avoid global variables when possible

Read more in this Stack Overflow about why folks generally recommend not using global variables. They result in a crowded namespace and things happening that you don't expect, where the problem can be hard to track down. I know this is just a small project, but it's best to develop coding habits that are sustainable in larger projects. It's not actually more work, and it saves you a lot of debugging later.

Seems like many of your global variables do not need to be global. For example, aChoices is only used in one function. That's probably true for many of the others.

\$\endgroup\$
  • \$\begingroup\$ I really appreciate these observations. I will make sure to apply this feedback and keep improving the simple quizz app. Thank you. \$\endgroup\$ – RufioLJ Aug 11 '15 at 20:27
  • \$\begingroup\$ As an easy improvement to the global variable problem - OP can wrap his code in an anonymous self-invoking function to restrict those variables to that scope. \$\endgroup\$ – Scott Aug 12 '15 at 2:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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