Cleaning up the working code
Now that the code is working, this is a good time to work through it again and make improvements to it.
The first thing that stands out is how lengthofobject is being used in several places. That’s a really bad variable name, and can be more usefully renamed to totalQuestions instead.
// const lengthofobject = data.quizcontent.length;
const totalQuestions = data.quizcontent.length;
// function scoreGrade(correct, lengthofobject) {
function scoreGrade(correct, totalQuestions) {
// const score = (correct / lengthofobject) * 100;
const score = (correct / totalQuestions) * 100;
...
}
...
// if (myAnswers[(lengthofobject - 1)]) {
if (myAnswers[(totalQuestions - 1)]) {
...
// const grade = scoreGrade(correct, lengthofobject);
const grade = scoreGrade(correct, totalQuestions);
...
// output = output + "<p>Имате " + correct + " од " + lengthofobject + " тачних одговора.</p></div> ";
output = output + "<p>Имате " + correct + " од " + totalQuestions + " тачних одговора.</p></div> ";
Working out the grades
Currently a series of if/else statements are used to give the grades.
let grade;
if (score >= 85) {
grade = "5";
} else if (score >= 70) {
grade = "4";
} else if (score >= 55) {
grade = "3";
} else if (score >= 40) {
grade = "2";
} else {
grade = "1";
}
return grade;
Each score has a difference of 15, which means that you can easily get from 85 to 5 by subtracting 10 and dividing by 15. That works for all of the grades. 40-10 is 30, and divided by 15 gives 2.
The only trouble comes with 100%, which would give a grade of 5, and with less than 15% which gives a grade of zero.
How this is calculated is not my main focus here. That the let keyword is needed is the problem to be resolved. It’s always better to use const over let.
So instead of using a formula of (score-10)/15 I will instead remove the need for the let variable, by returning the grade value instead.
function scoreGrade(correct, totalQuestions) {
const score = (correct / totalQuestions) * 100;
// let grade = parseInt((score - 10) / 15);
if (score >= 85) {
// grade = "5";
return "5";
} else if (score >= 70) {
// grade = "4";
return "4";
} else if (score >= 55) {
// grade = "3";
return "3";
} else if (score >= 40) {
// grade = "2";
return "2";
// } else {
// grade = "1";
}
return "1";
// return grade;
}
With the old lines removed that gives us the following code:
function scoreGrade(correct, totalQuestions) {
const score = (correct / totalQuestions) * 100;
if (score >= 85) {
return "5";
} else if (score >= 70) {
return "4";
} else if (score >= 55) {
return "3";
} else if (score >= 40) {
return "2";
}
return "1";
}
Fixing the endQuiz condition
Currently the endQuiz function checks if there is a myAnswers array item at the same place a the last array index of the totalQuestions array.
if (myAnswers[(totalQuestions - 1)]) {
That’s quite a complicated way of checking if they’re both the same length. Here’s a much easier way to do that:
if (myAnswers.length === totalQuestions) {
Removing more let keywords
Variables that need to change tend to be few and far between. We also tend to end up with better code structure when variables don’t need to change too.
My next focus is on the correct variable. The condition (data.quizcontent[i].correct === answer) can be moved out to a separate function, so that it can be used from multiple places.
function isCorrect(answer, i) {
return data.quizcontent[i].correct === answer;
}
...
// if (data.quizcontent[i].correct === answer) {
if (isCorrect(answer, i)) {
And we can now update the correct variable:
// let correct = 0;
const correct = myAnswers.filter(isCorrect).length;
...
// correct += 1;
That also helps to remove the correct increment from the forEach function, which will get a separate improvement made to it.
Replacing forEach with map
Instead of using forEach to update information outside of it, it’s more appropriate to return information instead. We can use map to return an array of values.
// let results = [];
// myAnswers.forEach(function (answer, i) {
const results = myAnswers.map(function (answer, i) {
if (isCorrect(answer, i)) {
questionResult = "<span class='glyphicon glyphicon-ok-circle' aria-hidden='true'></span>";
} else {
questionResult = "<span class='glyphicon glyphicon-remove-circle' aria-hidden='true'></span>";
}
// results.push("<p>Питање " + (i + 1) + " " + questionResult + "</p> ");
return "<p>Питање " + (i + 1) + " " + questionResult + "</p> ";
});
output += results.join("");
The result variable can be moved in to the map function:
// let questionResult = "NA";
const results = myAnswers.map(function (answer, i) {
// if (isCorrect(answer, i)) {
// questionResult = "<span class='glyphicon glyphicon-ok-circle' aria-hidden='true'></span>";
// } else {
// questionResult = "<span class='glyphicon glyphicon-remove-circle' aria-hidden='true'></span>";
// }
const result = (
isCorrect(answer, i)
? "<span class='glyphicon glyphicon-ok-circle' aria-hidden='true'></span>"
: "<span class='glyphicon glyphicon-remove-circle' aria-hidden='true'></span>"
);
// return "<p>Питање " + (i + 1) + " " + questionResult + "</p> ";
return "<p>Питање " + (i + 1) + " " + result + "</p> ";
});
output += results.join("");
And, the output variable can be declared after the myAnswers map.
// let output = "<div class='output'>Резултат<br>";
const results = myAnswers.map(function (answer, i) {
...
});
let output = "<div class='output'>Резултат<br>";
output += results.join("");
And the output variable can now be changed into a const variable instead:
// let output = "<div class='output'>Резултат<br>";
// output += results.join("");
...
// output = output + "<p>Имате " + correct + " од " + totalQuestions + " тачних одговора.</p></div> ";
const output = "<div class='output'>Резултат<br>" + results.join("") +
"<p>Имате " + correct + " од " + totalQuestions + " тачних одговора.</p></div> ";
Which for clarity, I’ll break apart into its separate parts:
const output = "<div class='output'>" +
"Резултат<br>" + results.join("") +
"<p>Имате " + correct + " од " + totalQuestions + " тачних одговора.</p>" +
"</div> ";
Is the output variable even needed, now that it’s right beside where it’s used? Let’s remove that output variable and combine the code with the innerHTML statement.
// const output = "<div class='output'>" +
// document.getElementById("quizContent").innerHTML = output;
document.getElementById("quizContent").innerHTML = "<div class='output'>" +
"Резултат<br>" + results.join("") +
"<p>Имате " + correct + " од " + totalQuestions + " тачних одговора.</p>" +
"</div> ";
So the information is prepared beforehand, and then all brought together at the innerHTML statement. That has a better flow to it.
Triangle indenting
The endQuiz function now shows quite a lot of indenting, to the point where the left margin has a distinctive triangle shape to it.
function endQuiz() {
if (myAnswers.length === totalQuestions) {
const correct = myAnswers.filter(isCorrect).length;
const results = myAnswers.map(function(answer, i) {
const result = (
isCorrect(answer, i) ?
"<span class='glyphicon glyphicon-ok-circle' aria-hidden='true'></span>" :
"<span class='glyphicon glyphicon-remove-circle' aria-hidden='true'></span>"
);
return "<p>Питање " + (i + 1) + " " + result + "</p> ";
});
const grade = scoreGrade(correct, totalQuestions);
document.getElementById("ocena").innerHTML = grade;
clearInterval(endTime);
const output = "<div class='output'>" +
"Резултат<br>" + results.join("") +
"<p>Имате " + correct + " од " + totalQuestions + " тачних одговора.</p>" +
"</div> ";
document.getElementById("quizContent").innerHTML = output;
}
}
As it can be tricky in the depths of that nesting to understand all of the different conditions that apply, it’s better to remove that indenting if possible.
First, the if statement at the top is a guard clause, preventing the rest of the function from being executed if the condition isn’t right. We can return immediately from there and remove one level of nesting.
function endQuiz() {
if (myAnswers.length !== totalQuestions) {
return;
}
const correct = myAnswers.filter(isCorrect).length;
...
}
Next, because the map function doesn’t refer to anything outside of the function that hasn’t been directly given to it, it can be named and moved out of the endQuiz function.
function checkAnswer(answer, i) {
const result = (
isCorrect(answer, i) ?
"<span class='glyphicon glyphicon-ok-circle' aria-hidden='true'></span>" :
"<span class='glyphicon glyphicon-remove-circle' aria-hidden='true'></span>"
);
return "<p>Питање " + (i + 1) + " " + result + "</p> ";
}
function endQuiz() {
...
const results = myAnswers.map(checkAnswer);
...
}
Rearranging the endQuiz function
After making those above changes, it’s now clear that the endQuiz function has several different things mixed in with each other.
Here, I’ve reorganised the endQuiz() function code.so that similar things are all grouped together.
function endQuiz() {
if (myAnswers.length !== totalQuestions) {
return;
}
clearInterval(endTime);
const correct = myAnswers.filter(isCorrect).length;
const grade = scoreGrade(correct, totalQuestions);
document.getElementById("ocena").innerHTML = grade;
const results = myAnswers.map(checkAnswer);
const output = "<div class='output'>" +
"Резултат<br>" + results.join("") +
"<p>Имате " + correct + " од " + totalQuestions + " тачних одговора.</p>" +
"</div> ";
document.getElementById("quizContent").innerHTML = output;
}
There are two clearly different parts to that function, that can be easily extracted out to separate functions. One is showGrade(), and the other is showResults()
Creating showGrade() function
The showGrade function only needs to receive the myAnswers array.
function showGrade(myAnswers) {
const correct = myAnswers.filter(isCorrect).length;
const grade = scoreGrade(correct, totalQuestions);
document.getElementById("ocena").innerHTML = grade;
}
totalQuestions can be removed from there too, as the guard-clause in the endQuiz() function has already ensured that myAnswers is the same length as the total questions.
We can even move counting the number of answers out to a separate function, as it’s preferable to count them over again, instead of passing them as a separate function parameter.
function countCorrect(answers) {
return myAnswers.filter(isCorrect).length;
}
function showGrade(myAnswers) {
const correct = countCorrect(myAnswers);
const grade = scoreGrade(correct, myAnswers.length);
document.getElementById("ocena").innerHTML = grade;
}
Creating showResults() function
The showResults() function is pretty simple now too, only needing to be given the myAnswers function too.
function showResults(myAnswers) {
const correct = countCorrect(myAnswers);
const results = myAnswers.map(checkAnswer);
const output = "<div class='output'>" +
"Резултат<br>" + results.join("") +
"<p>Имате " + correct + " од " + showAnswers.length + " тачних одговора.</p>" +
"</div> ";
document.getElementById("quizContent").innerHTML = output;
}
Removing totalQuestions
We can even remove that totalQuestions variable now, as it’s better to clearly indicate that it’s the length of quizcontent that we’re referring to.
// const totalQuestions = data.quizcontent.length;
...
// function scoreGrade(correct, totalQuestions) {
function scoreGrade(correct) {
const score = (correct / data.quizcontent.length) * 100;
...
}
...
function showGrade(myAnswers) {
const correct = countCorrect(myAnswers);
// const grade = scoreGrade(correct, totalQuestions);
const grade = scoreGrade(correct, data.quizcontent.length);
document.getElementById("ocena").innerHTML = grade;
}
...
function endQuiz() {
// if (myAnswers.length !== totalQuestions) {
if (myAnswers.length !== data.quizcontent.length) {
return;
}
...
}
The final endQuiz function
There’s not much left in the endQuiz function, and because of that it now clearly expresses exactly what needs to be done.
function endQuiz() {
if (myAnswers.length !== data.quizcontent.length) {
return;
}
clearInterval(endTime);
showGrade(myAnswers);
showResults(myAnswers)
}
Here’s the final code after those improvements:
function isCorrect(answer, i) {
return data.quizcontent[i].correct === answer;
}
function checkAnswer(answer, i) {
const result = (
isCorrect(answer, i) ?
"<span class='glyphicon glyphicon-ok-circle' aria-hidden='true'></span>" :
"<span class='glyphicon glyphicon-remove-circle' aria-hidden='true'></span>"
);
return "<p>Питање " + (i + 1) + " " + result + "</p> ";
}
function countCorrect(answers) {
return myAnswers.filter(isCorrect).length;
}
function scoreGrade(myAnswers) {
const correct = countCorrect(myAnswers);
const score = (correct / data.quizcontent.length) * 100;
if (score >= 85) {
return "5";
} else if (score >= 70) {
return "4";
} else if (score >= 55) {
return "3";
} else if (score >= 40) {
return "2";
}
return "1";
}
function showGrade(myAnswers) {
const grade = scoreGrade(myAnswers);
document.getElementById("ocena").innerHTML = grade;
}
function showResults(myAnswers) {
const correct = countCorrect(myAnswers);
const results = myAnswers.map(checkAnswer);
const output = "<div class='output'>" +
"Резултат<br>" + results.join("") +
"<p>Имате " + correct + " од " + myAnswers.length + " тачних одговора.</p>" +
"</div> ";
document.getElementById("quizContent").innerHTML = output;
}
function endQuiz() {
if (myAnswers.length !== data.quizcontent.length) {
return;
}
clearInterval(endTime);
showGrade(myAnswers);
showResults(myAnswers)
}
There are still more improvements that could be made, but hopefully this has helped to demonstrate how the code can be easily refactored and molded like clay, to help it explains things more clearly.
