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\$

protected by Community♦ Mar 12 '18 at 20:35

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

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