Human anatomy quiz

Question

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?

<html>
 <head>
  <title>Quizz</title>
  <script src="http://code.jquery.com/jquery-1.11.0.min.js"></script>
  <script>
  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.length; i++) {
			     choicesHtml += "<input type='radio' name='quiz" + currentQuestion +
			     "' id='choice" + (i + 1) +
			     "' value='" + choices[i] + "'>" +
			    " <label>" + choices[i] + "</label><br>";
             }
		     	  		  		  		  
	  		 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 < pick.length; i++) {
			       if (pick[i].checked) { // if this radio button is checked
				   userpick = pick[i].value;					    
       }         	  	      	  	  
   }
   if(userpick=='')
       alert('You havenot selected an answer');
   else{	
    if(userpick==quiz[currentQuestion].answer){
      score++;
			    document.getElementById('answers').innerHTML='you answer the question   correctly';
				
    $('#answers').css('background-color','green');
    $('#check').replaceWith( '<button  id="nextanswer" onclick="nextanswer()">Next  Question</button>' )						 
   }
   else{
     document.getElementById('answers').innerHTML='you answer the question  incorrectly. Answer:'+quiz[currentQuestion].answer; 					
     $('#answers').css('background-color','red');
                     $('#check').replaceWith( '<button  id="nextanswer"   onclick="nextanswer()">Next Question</button>' );
					 
					 
  }

  if(currentQuestion+1==quiz.length)
  {
     $('#nextanswer').css('display','none');
 
     $('#overrallresult').text('You answered correctly '+score+' out of '+    quiz.length+ ' questions');
  }
  } 
  }
    function nextanswer(){
    
      $('#nextanswer').replaceWith('<button  id="check"    onclick="checkanswers()">Submit</button> ');
 $('#answers').text('');
  currentQuestion++;
loadquestions();
  
  
  }
  window.addEventListener("load", loadquestions);
   
  
  
  </script>
</head><header> <embed src="logo.swf"  height="10%" width="100%" />  </header>
<body>
   <div id='questions'>





   </div>
   <br>
   
   <div id='choices'>





   </div>
   <div id='answers' >



   </div>
   <div id='overrallresult'>



   </div>
   <br>
   <button  id='check' onclick='checkanswers()'>Submit</button> 	   
 </body>
</html>

And please note that one could easily look up the answers from the source code. So maybe the correct answer should be decided by a server-side script. — jeff, Commented Nov 1, 2014 at 20:31
@halilpazarlama Since he is using jQuery, he could use the method jQuery.getJSON() for this. It doesn't even need to be a script, it could be the json file itself stored in the server. — Ismael Miguel, Commented Nov 2, 2014 at 2:53

Schism · Accepted Answer · 2014-10-31 15:16:54Z

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 <input> followed by a <label>. For improved useability, consider setting a for attribute on the <label>, or wrapping the <input> inside the <label>. (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.

Marc-Andre · Accepted Answer · 2014-11-02 18:16:06Z

12

Sorry for being a bit harsh here, but if you're making a quiz, please be sure to proof-read everything! I will not play a quiz game that suggests this kind of answer:

help u gain nutrients

Sometimes you're using "?" to end your questions, but not always. If you're not sure about grammar, you should ask someone to help you with it. I have some difficulty with this myself (especially with English), so we have a professional who proof-reads what we show to the user.

Your code could be the best in the world, but if your questionnaire has many grammatical problems in it, people will probably not hang around your game for a long time.

edited Nov 2, 2014 at 18:16

answered Oct 31, 2014 at 14:45

Marc-Andre

6,7115 gold badges38 silver badges65 bronze badges

\$\begingroup\$ Your code should still be reviewed by someone, but I'm knowledgeable enough in JavaScript. \$\endgroup\$
– Marc-Andre
Commented Oct 31, 2014 at 15:02
1

\$\begingroup\$ this is one of the things i do for the project prototype. This is yet to be the product i am going to present to the class or users. Thus i am not checking my grammar. Yes if this is to be present to the audience, i will have to do a thorough grammar check up to make sure that it looks friendly to the users \$\endgroup\$
– mmm
Commented Oct 31, 2014 at 15:02
8

\$\begingroup\$ Honestly, we are an audience and if you were to present a prototype with grammatical errors all over the place in the prototype it the only thing I will concentrate on! Prototype should be deleted after the proof of concept, but they are not in practice, so do it well from the start. \$\endgroup\$
– Marc-Andre
Commented Oct 31, 2014 at 15:07
\$\begingroup\$ @Marc For example, you've said "I will not play a quiz game that suggests me this kind of answer." When you should omit me: "I will not play a quiz game that suggests this kind of answer." Just a common error involving suggests that I often see. \$\endgroup\$
– JYelton
Commented Nov 2, 2014 at 17:46
\$\begingroup\$ That's exactly the idea, if you're not good at something get help, but it's really important to get it right! \$\endgroup\$
– Marc-Andre
Commented Nov 2, 2014 at 18:15

Add a comment |

konijn · Accepted Answer · 2014-11-03 13:40:50Z

From a once over:

You have the question and answer as a string, I would store the answer as a number:

{
  "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": 2
}

Do not use alert, try to use a message div instead, it is so much more user friendly
You are overdoing the newlines in the HTML, use a beautifier and remove most of the newlines
Dont assign click handlers like this: onclick='checkanswers()', do it thru JavaScript with addEventListener
In function loadquestions() { you use a global variable currentQuestion, it would be far more modular to have this : function loadQuestion( question ) { this way you do not need the global variable, you know where the question value comes from, and since you load 1 question, I dropped the final s. And finally I lowerCamelCased the name.
This:
```
if(userpick=='')
  alert('You havenot selected an answer');
else{   
```
is considered a dangerous practice, if you else block has curly braces, then your if block should as well. In fact, when learning JavaScript, you should never skip the curly braces.
Since your design is to show only 1 question at a time, you could simplify quite a bit by just giving the name 'quiz' instead of 'quiz' + question. In fact I might call it possibleAnswers.
You are using jQuery, it does not make sense to call document.getElementById('questions'), when you call $('#questions'), which means you should cache those lookups!
Always try to turn common lines of code into a function, creating the function displayMessage in the snippet made things more manageable.

I will get back to this question, I am trying to build a working snippet and write review comments as I go.

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": 2
}, {
  "question": "What body system is responsible for protections of body tissues and enabling human body movements",
  "choices": ["Skeleton System", "Digestive system", "Circular system"],
  "answer": 0
}, {
  "question": "How many body systems are there in total",
  "choices": ["4", "5", "6"],
  "answer": 2
}, {
  "question": "What system is responsible for bringing energy to the body for cell growth and repair?",
  "choices": ["Digestive", "Respiratory", "Circulatory"],
  "answer": 0
}];
currentQuestion = 0,
score = 0,
$choices = $('#choices'),
$questions = $('#questions');

function loadQuestion( question ) {

  var choices = quiz[question].choices,
      html = "";

  for (var i = 0; i < choices.length; i++) {
    html += "<input type='radio' name='possibleAnswers' value='" + i + "'>" + " <label>" + choices[i] + "</label><br>";
  }

  $questions.text( "Q" + (currentQuestion + 1) + ". " + quiz[currentQuestion].question );
  $choices.html(html);
}

function checkanswers() {
  var selectedAnswer = '',
      possibleAnswers = document.getElementsByName('possibleAnswers'),
      rightAnswer = quiz[currentQuestion].answer;     

  for (var i = 0; i < possibleAnswers.length; i++) {
    if (possibleAnswers[i].checked) { // if this radio button is checked
      selectedAnswer = possibleAnswers[i].value;
    }
  }
  if (selectedAnswer == ''){
    displayMessage( 'yellow' , 'You have not selected an answer' );
    return;
  }

  if (selectedAnswer == rightAnswer) {
      score++;
      displayMessage( 'green' , 'You answered the question correctly' ); 
    } else {
      displayMessage( 'red' , 'You answered the question incorrectly. Answer:<br><b>' +                                                     quiz[currentQuestion].choices[rightAnswer]);
    }

    if (currentQuestion + 1 != quiz.length) {
      $('#check').replaceWith('<button  id="nextanswer"   onclick="nextanswer()">Next Question</button>');
    }else{
      $('#check').css('display', 'none');
      $('#overrallresult').text('You answered correctly ' + score + ' out of ' + quiz.length + ' questions');      
    }
}

function displayMessage( color , message ){
  $('#answers').css('background-color', color ).html( message );
}


function nextanswer() {

  $('#nextanswer').replaceWith('<button  id="check" onclick="checkanswers()">Submit</button> ');
  $('#answers').text('');
  currentQuestion++;
  loadQuestion( currentQuestion );
}

//At load, we will display the first question (zero)
window.addEventListener("load", loadQuestion.bind(this, 0) );

<script src="https://code.jquery.com/jquery-1.11.0.min.js"></script>

<body>
  <div id='questions'></div>
  <br>
  <div id='choices'></div>
  <div id='answers'></div>
  <div id='overrallresult'></div>
  <br>
  <button id='check' onclick='checkanswers()'>Submit</button>
</body>

200_success · Accepted Answer · 2014-11-01 08:34:44Z

Use `<label>` elements properly to improve usability

Here is the HTML generated by your code. Clicking on the words has no effect; you have to click precisely on the radio buttons themselves.

<input type='radio' name='quiz3' id='choice1'><label>Digestive</label><br>
<input type='radio' name='quiz3' id='choice2'><label>Respiratory</label><br>
<input type='radio' name='quiz3' id='choice3'><label>Circulatory</label><br>

If the label encompasses the <input type="radio"> tag, then the user can click anywhere on the answer to activate the radio button.

<label><input type='radio' name='quiz3' id='choice1'>Digestive</label><br>
<label><input type='radio' name='quiz3' id='choice2'>Respiratory</label><br>
<label><input type='radio' name='quiz3' id='choice3'>Circulatory</label><br>

Alternatively, you can associate the label with its radio button by using <label for="id_of_the_radio_button">.

<input type='radio' name='quiz3' id='choice1'><label for='choice1'>Digestive</label><br>
<input type='radio' name='quiz3' id='choice2'><label for='choice2'>Respiratory</label><br>
<input type='radio' name='quiz3' id='choice3'><label for='choice3'>Circulatory</label><br>

Partial use of jQuery

You use jQuery, but there are many places where you still do things the hard way with document.getElementById() and document.getElementsByName(). I recommend that you take full advantage of jQuery to simplify your code and make it feel coherent.

Chrillewoodz · Accepted Answer · 2014-11-01 10:34:21Z

3

From looking over your code briefly there is one thing that I don't understand. You are using Jquery without really using it.

Implementing an entire Jquery library for using things like this:

$('#answers').css('background-color','green');

or

$('#overrallresult').text('You answered correctly '+score+' out of '+    quiz.length+ ' questions');

really seems pointless, it will just cause your application to be loaded slower. If you're going to use Jquery, try to use it for as much as possible or don't use it at all. The two examples could easily be written in Javascript as

document.getElementById('answers').style.backgroundColor = 'green';
document.getElementById('overrallresult').innerHTML = 'insert text here';

What's your argument for using Jquery in this project?

answered Nov 1, 2014 at 10:34

Chrillewoodz

8071 gold badge8 silver badges21 bronze badges

\$\begingroup\$ I am newbie so i just try to practice jquery as much as i can :) \$\endgroup\$
– mmm
Commented Nov 8, 2014 at 20:13
\$\begingroup\$ Then don't do it the hard way and write the pure javascript functions, use the Jquery functions! \$\endgroup\$
– Chrillewoodz
Commented Nov 8, 2014 at 20:28

Add a comment |

Stack Exchange Network

Human anatomy quiz

5 Answers 5

Use `<label>` elements properly to improve usability

Partial use of jQuery

Your Answer

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

Hot Network Questions

Human anatomy quiz

5 Answers 5

Use <label> elements properly to improve usability

Partial use of jQuery

Your Answer

Sign up or log in

Post as a guest

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

Related

Hot Network Questions

Use `<label>` elements properly to improve usability

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