HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptModerate

Human anatomy quiz

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
humananatomyquiz

Problem

My code works fine. I just want to have your insight and opinions about the way I did it. For example, is it easy to maintain? Does it have good architecture and design?





Quizz


var quiz = [{
"question": "What is function of respiratory system?",
"choices": ["help u gain nutrients", "to filter and eliminate wastes", "to bring the oxygens into the body and expel carbon dioxide"],
"answer": "to bring the oxygens into the body and expel carbon dioxide"
}, {
"question": "What body system is responsible for protections of body tissues and enabling human body movements",
"choices": ["Skeleton System", "Digestive system", "Circular system"],
"answer": "Skeleton System"
}, {
"question": "How many body systems are there in total",
"choices": ["4", "5", "6"],
"answer": "6"
}, {
"question": "What system is responsible for bringing energy to the body for cell growth and repair?",
"choices": ["Digestive", "Respiratory", "Circulatory"],
"answer": "Digestive"
}];
var currentQuestion = 0,
score = 0;
function loadquestions(){
var choices = quiz[currentQuestion].choices,
choicesHtml = "";
for (var i = 0; i " + choices[i] + "
";
}

document.getElementById('questions').textContent = "Q" + (currentQuestion + 1) + ". " +quiz[currentQuestion].question;
// load the choices
document.getElementById('choices').innerHTML = choicesHtml;
}
function checkanswers(){
var userpick='';
var pick=document.getElementsByName('quiz'+currentQuestion);
alert(pick.length);
for (var i = 0; i



















Submit

Solution

-
I'm not a fan at all of the mixed spacing and indentation. I'm hopeful (but doubtful) that this is due to something like mixed spaces and tabs. I can't easily skim your code, because you have distinct levels of indentation matching up unfortunately well.

-
I'm usually a fan of wrapping code in an IIFE, but it's not a huge deal for a small app. You might want to look into it anyway.

-
The kind of variable declaration you use is potentially dangerous. If you try to add something in the middle and add a semicolon instead of a comma, you'll be introducing a new global. In this case, it's not that relevant since youre using globals anyway, but you might want to think about it.

var x = 0, // dangerous! if I add "z;" after this, y becomes a global
y = 0;
// prefer:
var x = 0, y = 0;
// or:
var x = 0;
var y = 0;


-
Convention dictates function names be in camelCase. It should be loadQuestions().

-
On that note, loadQuestions() is a bad name; it implies to me that you're loading all the questions, when in fact you're only putting the current question onto the page.

-
In loadQuestions(), you're generating an ` followed by a . For improved useability, consider setting a for attribute on the , or wrapping the inside the . (This will let the user click the text to select the option, rather than just the radio button.)

-
You're using jQuery. Rather than looping through the radio buttons to find which is
checked, you can get this easily with something like $("[name=quiz" + currentQuestion + "]:checked").val(). Furthermore, rather than document.getElementById("answers"), you can use $("#answers") and the like — the performance difference here should not be a big concern.

-
You're duplicating your
$("#check").replaceWith ... code — you have it in both an if and its corresponding else`. Just move it out of the conditional blocks.

-
Personally, I'm not a fan of replacing the Check Answers and Next Question buttons with each other. Instead, it might be better to have both buttons, hiding one at a time. Then you can bind the events once instead of setting an onclick every time, which is somewhat inelegant.

-
You have a rather significant amount of vertical whitespace that serves no purpose, particularly in your HTML.

Code Snippets

var x = 0, // dangerous! if I add "z;" after this, y becomes a global
y = 0;
// prefer:
var x = 0, y = 0;
// or:
var x = 0;
var y = 0;

Context

StackExchange Code Review Q#68531, answer score: 13

Revisions (0)

No revisions yet.