2
\$\begingroup\$

Is this JavaScript code robust? It is for an interactive book about a card game. When the reader clicks on the checkbox, an answer is revealed and related text is highlighted. I need to get this right before I write more chapters.

JSBin

<!DOCTYPE html>
<html>
<head>   <title>Quiz</title>        

<script>
function hlt_showhide(EEE){	
"use strict";
var xxx = EEE.getAttribute('answer');
var win_num = "win" + xxx; 
var e2 = document.getElementsByName(win_num);
var countE2 = e2.length;

 for (var i = 0; i < countE2; i++) {
e2[i].style.backgroundColor = (EEE.checked) ? "#FFCC66" : "#FFFFFF";  
}

var ans_num = "ans" + xxx;
var AAA = document.getElementById(ans_num); 

if (AAA !== null) {            
	AAA.style.visibility = (EEE.checked) ? "visible" : "hidden";        
} 
}
</script>
<style type="text/css"> .ansText {visibility: hidden}  </style> 

</head>
<form name="form1">
<!-- Question 1 -->
Q1: 3 <span id="win1" name="win1"> 5 </span> <span id="win1" name="win1"> 10 </span> 
<span class="click"> Click</span> 
<input onclick="hlt_showhide(this)" type="checkbox" name=""  answer="1"> 

<!-- Answer 1 -->
A1: <span id="ans1" class="ansText"> revealed text refers to 2nd & 3rd highlighted numbers </span>
<hr id="q_a">

<!-- Question 2 -->
Q2: <span id="win2" name="win2"> 7 </span> 28 <span id="win2" name="win2"> 37 </span> 
<span class="click"> Click</span> 
<input onclick="hlt_showhide(this)" type="checkbox" name="" answer="2"> 

<!-- Answer 2 -->
A2: <span id="ans2" class="ansText"> revealed text refers to 1st & 3rd highlighted numbers</span>
<hr id="q_a">

</form>

</html>

| improve this question | |
\$\endgroup\$
2
\$\begingroup\$

Here are my thoughts:

  1. Most importantly from the first look at the code: Use proper variable names. No one knows what you mean with EEE, xxx or e2. And while you are at it: in JavaScript it's a convention to start variable names with a lower case letter.

  2. Don't make up attributes such as "answer" - you can't say if an unknown browser can work with them. At least you should adhere to HTML5 and prefix the custom attribute with data-.

  3. name is not a valid attribute for span (or any other object other than form elements). Unfortunately there is no simple, cross-browser alternative for selecting multiple elements. The easiest soltuion with be to use a class and getElementsByClassName, however the latter isn't supported by all browsers yet, but there are implementations for the non-supporters easily available on the web.

  4. The "Click" text is bad for two reasons: A) You can't actually click on it and B) texts such as "click here" is bad UI. Instead wrap the text together with the checkbox in a <label> element, such allows the user to click on the text to activate the checkbox and use a text such as "Show solution" instead.

  5. Finally, there are more elegant ways to switch on the answers. Instead of looking of a specifc ID, set (or remove) a class on an element surrounding the question and use CSS to highlight/show the sub elements. Example: http://jsbin.com/ayoqem/7/edit

| improve this answer | |
\$\endgroup\$

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