Feedback on my Code

Question

function print(message) {
  document.write(message);
}

var questArray = [
  ["Which gas helps in burning?","OXYGEN"],
  ["Who is the incumbent Prime Minister of India?","NARENDRA MODI"],
  ["Which state is the Capital of India?","DELHI"]
];

//All the main code is in this function:
function quiz (array) { //array is a parameter where argument will be passed.
  var finalHtml = "<p>" //To produce the Html code

  /*To make the 
  wrong answer Html Code*/
  var wrongStr = "<h4>You got these questions wrong: </h4><ol>" 

  var correctAnswer = 0; // Counter for number of correct guesses

  // To Make the correct Answer Html Code
  var correctStr = "<h4> You got these questions correct: </h4><ol>";

  //Engine(loop) where questions are asked and answers are compared by iterating through the loop and using a decision making if statement.
  for(i=0; i < array.length; i += 1) {
    var guess = prompt(array[i][0]);
    if (guess.toUpperCase() === array[i][1]) {
      correctAnswer += 1;
      correctStr += "<li>" + array[i][0] + "</li>"
    } else {
      wrongStr += "<li>" + array[i][0] +"</li>"  
      }
  }

  //completing the Html code 
  finalHtml += "You got " + correctAnswer + 
  " question(s) right. </p> </br>" +
  correctStr + "</ol> </br>" + wrongStr + "</ol>"; 

  //Printing the final Html code on the document
  print(finalHtml);
}

quiz(questArray); //Run the Program

Answer 1 · 2019-06-01T17:41:22Z

June 1, 2019 5:41pm

The first thing I would recommend is to make full use of ES2016 syntax. Nowadays, I use const pretty much by default, and only change it to let if I realize that this value will need to change later. But this helps prevent bugs by being explicit about what values in your code will vary and what will not. Similarly with functions, I use arrow functions by default, which avoids having to do strange things with the this keyword when passing around functions. You can also make use of template literals instead of string concatenation.

A few other more specific things:

On line 5, you’re using a multidimensional array to store your questions and answers. I think it would be more clear to use an array of objects. Each object can have a question key and an answer key. Then instead of referring to question[0] question[1] it could be question.question question.answer,
On line 12, you’re calling the parameter of this function array, but it would be nice for it to have a more specific name like questions. Whenever possible, it’s good to use names for things that make it clear what it is we’re doing.
Line 25 is somewhere you could be making use of forEach() loop instead of a for loop. For loops have some clunky boilerplate. The forEach function is a bit cleaner and lets you name the item you’re grabbing in the function parameter without having to use [i].

Here’s an example:

questions.forEach(question => { 
    const guess = prompt(question.question);
    if (guess.toUpperCase() === question.answer) {
      correctAnswer += 1;
      correctStr += `<li>${question.question}</li>`
    } else {
      wrongStr += `<li>${question.question}</li>`
    }
})

Welcome to the Treehouse Community

Looking to learn something new?

Siddharth Pande

Siddharth Pande

Feedback on my Code

1 Answer

Brendan Whiting

Brendan Whiting

Siddharth Pande

Siddharth Pande