Problem
My code fetches quiz data from a JSON file, displays the questions sequentially and records the user’s answers. It keeps track of the user’s score and at the end displays their result based on a second JSON file.
const QUIZ_API = 'quiz.json';
const RESULT_API = 'result.json';
const QUIZ_ID = 12; // Used to identify the quiz in the JSON file.
let jsonQuizData;
let questionNo = 0; // Current question number
let answerSubmitted = false; // Is set to true once the user submits an answer to the current question and still hasn't proceeded to the next one.
let points = 0; // User's current score
let maxPoints = 0; // Max possible score for quiz
/*
* When the document is ready, adds a key listener to enable playing the quiz using the keyboard only, and loads quiz data.
*/
$(document).ready(() => {
// Allow submitting answer with Enter key
document.addEventListener('keyup', (event) => {
if (event.keyCode === 13) // 13 is the Enter key
$('#next-btn').click();
});
$.getJSON( QUIZ_API, { quiz_id: QUIZ_ID})
.done((resp) => {
jsonQuizData = resp;
$('h1').text(resp.title);
$('h2').html('“' + resp.description + '”');
updateQuestionDisplay(resp.questions[0]);
})
.fail(() => {
alert('The quiz is not available at the moment! Please try again later.');
});
});
/*
* Displays the current question and its possible answers
* @param {Object} qObject - Object representing the current question
*/
function updateQuestionDisplay(qObject) {
/* Question image */
const imageAlt = `Question ${questionNo + 1} image`;
$('#question').find('img').attr({
src: qObject.img,
alt: imageAlt
});
/* Question header (Question number, question title, question points) */
let maxStr = ''; // Questions with multiple answers will have the word 'max' after the number of points, in order to show that the user may receive fewer points for a partially correct answer.
if (qObject.question_type === 'mutiplechoice-multiple')
maxStr = ' max';
let ptsStr = qObject.points === 1? ' pt': ' pts';
$('#qTitle').html(qObject.q_id + '. ' + qObject.title + ' <strong>[' + qObject.points + ptsStr + maxStr + ']</strong>');
/* Question type */
let inputType;
switch(qObject.question_type) {
case 'mutiplechoice-multiple':
inputType = 'checkbox';
break;
default:
inputType = 'radio';
}
/* Answers */
const inputsContainer = $('#inputs');
inputsContainer.css('text-align', 'left');
if (qObject.question_type === 'truefalse') {
inputsContainer.append(`
<div class="option">
<input type="${inputType}" id="true" name="${qObject.q_id}" value="true">
<label for="true">True</label>
</div>
`);
inputsContainer.append(`
<div class="option">
<input type="${inputType}" id="false" name="${qObject.q_id}" value="false">
<label for="false">False</label>
</div>
`);
}
else
qObject.possible_answers.forEach((item) => {
inputsContainer.append(`
<div class="option">
<input type="${inputType}" id="${item.a_id}" name="${qObject.q_id}" value="${item.a_id}">
<label for="${item.a_id}">${item.caption}</label>
</div>
`);
});
}
/*
* Performs appropriate task on clicking button based on answerSubmitted
*/
function nextButtonClicked() {
if (answerSubmitted)
nextQuestion();
else
submitAnswer();
}
/*
* Validates and submits the user's answer, changes document and styles accordingly.
*/
function submitAnswer() {
const inputs = $('input');
let ans = []; // ans is an array to accomodate the case of multiple answer questions.
inputs.each((i, obj) => {
if (obj.checked)
ans.push(obj.id); // obj.id is always a string.
});
if (!ans.length)
alert('No answer selected!');
else { // If an answer was selected
answerSubmitted = true;
checkAnswer(ans);
const nextBtn = $('#next-btn');
inputs.attr('disabled', 'disabled');
nextBtn.attr('disabled', 'disabled');
$('#inputs').css('text-align', 'center');
let newButtonText = jsonQuizData.questions[questionNo + 1]? 'Next question': 'See results!';
$(this).delay(500).queue(() => {
nextBtn.text(newButtonText);
nextBtn.removeAttr('disabled');
$(this).dequeue();
});
}
}
/*
* Performs 2 tasks:
* 1. Add css to indicate the correct answer and highlight the user's chosen answer.
* 2. Check answer and update points.
* @param {string[]} answer - User's submitted answer
*/
function checkAnswer(answer) {
const correctAnswer = jsonQuizData.questions[questionNo].correct_answer;
let pointsToAward = jsonQuizData.questions[questionNo].points;
maxPoints += pointsToAward; // Update maxPoints for the current question.
/* 1. Add css to indicate the correct answer and highlight the user's answer. */
if (jsonQuizData.questions[questionNo].question_type === 'truefalse') { // For true-false questions it is straightforward
$('label[for="' + correctAnswer.toString() + '"]').addClass('correct');
$('label[for="' + (!(correctAnswer)).toString() + '"]').addClass('incorrect');
$('label[for="' + answer.toString() + '"]').parent().addClass('selected');
}
else { // For questions which are not of true-false type
jsonQuizData.questions[questionNo].possible_answers.forEach((obj, i) => {
if (answer.includes(obj.a_id.toString()))
$('label[for="' + obj.a_id.toString() + '"]').parent().addClass('selected');
if (typeof correctAnswer === 'object' && correctAnswer != null && correctAnswer.includes(jsonQuizData.questions[questionNo].possible_answers[i].a_id) || correctAnswer === obj.a_id)
$('label[for="' + obj.a_id.toString() + '"]').addClass('correct');
else
$('label[for="' + obj.a_id.toString() + '"]').addClass('incorrect');
});
}
/* 2. Check answer and update points. */
if (typeof jsonQuizData.questions[questionNo].correct_answer === 'object' && jsonQuizData.questions[questionNo].correct_answer != null) { // The if statement checks if it is a question with multiple answers. This is because 'object' is the type of arrays, and of null. Since correct_answer might be null, we check if it's not null as well.
const noOfItems = jsonQuizData.questions[questionNo].possible_answers.length;
let resultBool = new Array(noOfItems).fill(false); // For each option, resultBool[i] is false if the user's answer is wrong, else true.
// Setting resultBool
jsonQuizData.questions[questionNo].possible_answers.forEach((obj, i) => {
if (correctAnswer.includes(obj.a_id) && answer.includes(obj.a_id.toString()) || !correctAnswer.includes(obj.a_id) && !answer.includes(obj.a_id.toString()))
resultBool[i] = true;
});
resultBool.forEach((obj) => {
if (!obj)
pointsToAward = pointsToAward - !!pointsToAward; // Decrements pointsToAward by 1 if it's greater than 0.
});
points += pointsToAward;
}
else { // Questions with a single answer
if (answer[0] === correctAnswer.toString())
points += pointsToAward;
}
}
/*
* If a subsequent question exists, increments questionNo and displays the next question
* with the help of updateQuestionDisplay(). Else, calls displayResults.
*/
function nextQuestion() {
if (jsonQuizData.questions[questionNo + 1]) { // If there is a next question
// Remove current options
$('.option').each((i, obj) => {
obj.remove();
});
// Replace image with placeholder image so users with slow internet connections do not see the previous level's image while the new level's image is still loading.
$('.img-container-small').find('img').attr('src', 'img/placeholder-image.png');
// Set button text
$('#next-btn').text('Submit');
// Set answerSubmitted to false
answerSubmitted = false;
// Increment question number and display next question
updateQuestionDisplay(jsonQuizData.questions[++questionNo]);
}
else
displayResults();
}
/*
* Calculates the user's score as a percentage, loads the result data, finds the user's
* result group, displays the user's result.
*/
function displayResults() {
const pointsPercent = Math.round(points / maxPoints * 100);
let resultGroupIndex = 0;
let jsonResultsData;
$.getJSON( RESULT_API, { quiz_id: QUIZ_ID})
.done((resp) => {
jsonResultsData = resp;
jsonResultsData.results.forEach((obj, i) => {
if (pointsPercent >= obj.minpoints)
resultGroupIndex = i;
});
$('main').html(`
<section>
<h3>Quiz completed! Points earned: <strong>${points}/${maxPoints} (${pointsPercent}%)</strong></h3>
<p class="huge-font m-y-sm">${jsonResultsData.results[resultGroupIndex].title}</p>
<div class="img-container-medium"><img src="${jsonResultsData.results[resultGroupIndex].img}" alt="Result image"></div>
<p class="p-b-md">${jsonResultsData.results[resultGroupIndex].message}</p>
</section>
`);
})
.fail(() => {
alert('Your result is not available at the moment! Please try again later.');
});
}
I use global variables and all my functions have global scope because, in my HTML file, I have this:
<button id="next-btn" onclick="nextButtonClicked()">Submit</button>
Since my HTML button references the function nextButtonClicked(), that function has to have global scope. This function eventually ends up calling all the other functions and also needs to use the global variables, so I have also given them global scope.
However, I’m not sure if this is the best way to go about solving this problem. Also, I’m wondering if it would be a good idea to use namespaces and how.
Moreover, the only ES6 features I’m using are arrow functions, let and const. Are there other ES6 features that I can incorporate?
Solution
Your Questions
However, I am not sure if [having the function
nextButtonClicked()in the global scope] is the best way to go about solving this problem.
As was already mentioned in Arnav’s answer, it would be wise to set the event handler for that element in Javascript. One technique is to use jQuery’s .click() method (which is really just a shortcut for .on('click'))
So at the beginning of the script, where constants and variables are defined, add let nextBtn;, and in the DOM-ready callback, assign that value:
$(document).ready(() => {
//move this out of submitAnswer()
nextBtn = $('#next-btn');
As the comment implies, that assignment can be removed from the function submitAnswer(). Then the HTML for the button can be simplified to not have that onclick attribute, as many believe this leads to good separation for the business logic to be out of the markup.
<button id="next-btn">Submit</button>
Actually, that function .ready() is deprecated as of version 3.0. See the last paragraph below for more details on that topic.
Moreover, the only ES6 features I’m using are arrow functions,let and const. Are there other ES6 features that I can incorporate?
The code already uses template literals. Most of the functions don’t appear to currently have more than one parameter but if any did, there could be a place to use default parameters. Also, you could perhaps utilize destructuring assignment somewhere – maybe for the assignment of resultsBool?
If you really wanted, you could perhaps utilize the class structure to move your code to being slightly more object-oriented, but be aware that it is “primarily syntactical sugar over JavaScript’s existing prototype-based inheritance. The class syntax does not introduce a new object-oriented inheritance model to JavaScript.”1.
Other feedback
switch statement
let inputType;
switch(qObject.question_type) {
case 'mutiplechoice-multiple':
inputType = 'checkbox';
break;
default:
inputType = 'radio';
}
If there were more cases in the switch statement, then it might make sense to have that but because there are really only two cases, one could argue that it should be simplified to a simple if-else statement:
let inputType;
if (qObject.question_type == 'mutiplechoice-multiple') {
inputType = 'checkbox';
}
else {
inputType = 'radio';
}
While that is only one less line, there is no need to worry about adding break statements (some might argue it should have been added in the default case). It could also be simplified by moving the assignment from the else block to the initial declaration:
let inputType = 'radio';
if (qObject.question_type == 'mutiplechoice-multiple') {
inputType = 'checkbox';
}
let vs const
There is a variable created for newButtonText
let newButtonText = jsonQuizData.questions[questionNo + 1]? 'Next question': 'See results!';
However it appears to never be re-assigned. It is wise to default using const until a valid reason to reassign that value arises. This will help avoid accidental re-assignment and other bugs
Using jQuery and then document.addEventListener()
If jQuery is loaded on the page, then why use document.addEventListener(), as in:
document.addEventListener('keyup', (event) => {
When instead it could be simplified using jQuery’s .on() method?
$(document).on('keyup', (event) => { ... })
Edit
Another more jQuery like way to avoid global variables, making use of IIFE’s and
$(document).ready(function() {})
Sure you could do that, though bear in mind that as of jQuery 3 “only $(handler) is recommended” and $(document).ready() will still work but is deprecated.2
The reason I use global variables and all my functions have global scope is because in my HTML file I have this:
<button id="next-btn" onclick="nextButtonClicked()">Submit</button>Since my HTML button references
nextButtonClicked(), the function has to have global scope
In modern javascript, polluting the global namespace is generally considered a bad idea as there is already so much in there. In order to move everything outside of the global namespace, I would wrap everything in a Anonymous IIFE such as the following:
(function() {
const QUIZ_API = 'quiz.json';
const RESULT_API = 'result.json';
const QUIZ_ID = 12; // Used to identify the quiz in the JSON file.
// ... Rest of code ...
}());
You could even use an arrow function here.
Now to address your specific problem: Adding an event listener. To solve this issue, I would add the following to your $(document).ready(() =>:
$(document).ready(() => {
document.getElementById("next-btn").addEventListener("click", nextButtonClicked);
});
Using the jQuery click listener is also an option. The above will do the same thing as the onclick and one advantage to this is that the JS code now doesn’t pollute your HTML. (More information)
Also, I’m wondering if it would be a good idea to use namespaces and how.
TBH, in such a small program, namespacing wouldn’t really add much benefit. If you were planning on using a module system of some sort to separate the different parts of your code or were planning on increasing the program’s size, then you would multiple files with namespaces to create modularity. There are actually many examples online of this, such as the revealing module pattern.
A Minor Style Issue
else
qObject.possible_answers.forEach((item) => {
I notice that sometimes you omit the braces with conditional branches when there is only one statement inside. While this is allowed, this is generally not considered good practice because you never know when you may unknowingly add an extra line. It also hurts readability (Especially in the case above), since it is now hard to tell when the block starts/ends.
EDIT:
Another more jQuery like way to avoid global variables, making use of IIFE’s and $(document).ready(function() {}):
(function($) {
$(document).ready(function() {
// Code
});
})(jQuery);
This approach removes any namespacing conflicts, since now the $ only refers to jQuery inside the IIFE.