[Code Review] Quick Question Script

JavaScript
1

I’ve just finished working on a simple (although it wasn’t that straightforward to get working properly!) script that asks a “Quick Question” to the user. The user inputs a correct answer, hits the “Submit” button, and text is then displayed in green saying “Correct”. If the user inputs a wrong answer and hits the “Submit” button there is red coloured text displayed “Wrong”. And finally, if the user enters a wrong answer on their first attempt and then enters the correct answer on their second attempt the previous “Wrong” text will disappear and be replaced with the “Correct” text.

As far as I know (which isn’t a lot) all of the script uses standard JavaScript and the appropriate DOM standards. I’ve checked that it works in Chrome 20, Internet Explorer 9, Opera 11.11, Safari #, and Firefox 14.

So, what could I have done better? Could I have scripted it in a more logical way? A way in which it would be able to perform better and faster? And, although I said as far as I know about the standard JS and appropriate DOM standards, I can’t tell for sure, so, is it all standard JS and using the appropriate DOM standards…?

Finally, if there are any bugs or glitches you find, it would be great if you could describe them to me and possibly present a fix for it too! (If you don’t ask, you don’t get!)

HTML

<!DOCTYPE html>
<html lang="en">
	<head>
		<meta charset="utf-8">
		<title>Quick Question</title>
		<link rel="stylesheet" media="screen" href="quick-question.css">
	</head>

	<body>
		<div id="quick-question">
			<p class="header">Quick Question</p>

			<p class="question">Who invented the World Wide Web?</p>

			<form name="questionOne" method="post" action="#">
				<label for="answer">Answer:</label>
				<input type="text" value="" id="answerOne" name="answerOne" class="required" />
				<input type="submit" value="Submit" id="btnSubmit" name="btnSubmit" />
				<div id="answerPlacement"></div>
			</form>
		</div>

		<script src="quick-question.js"></script>
	</body>
</html>

JavaScript

function addLoadEvent(func) {
	var oldonload = window.onload;
	if (typeof window.onload != 'function') {
		window.onload = func;
	} else {
		window.onload = function() {
			oldonload();
			func();
		}
	}
}

function fnAnswers(event) {
	if (!document.getElementById) return false;

	// Answer Response

	function fnAnswerResponseCorrect() {
		var answerParagraph = document.createElement("p");
		var answerPlacement = document.getElementById("answerPlacement");
		answerPlacement.appendChild(answerParagraph);
		var answerText = document.createTextNode("Correct");
		answerParagraph.appendChild(answerText);
		answerParagraph.style.color = "Green";
	}

	function fnAnswerResponseWrong() {
		var answerParagraph = document.createElement("p");
		var answerPlacement = document.getElementById("answerPlacement");
		answerPlacement.appendChild(answerParagraph);
		var answerText = document.createTextNode("Wrong");
		answerParagraph.appendChild(answerText);
		answerParagraph.style.color = "Red";
	}

	function fnRemoveElement() {
		var answerPlacement = document.getElementById("answerPlacement");

		while (answerPlacement.firstChild) {
			answerPlacement.removeChild(answerPlacement.firstChild);
		}
	}

	// Question & Answer One

	if (document.questionOne.answerOne.value == "Tim Berners-Lee") {
		event.preventDefault();
		fnRemoveElement();
		fnAnswerResponseCorrect();
	}
	else {
		event.preventDefault();
		fnRemoveElement();
		fnAnswerResponseWrong();
	}
}

function fnSubmitAnswer() {
	if (!document.getElementById) return false;
	var btnSubmit = document.getElementById("btnSubmit");
	btnSubmit.onclick = fnAnswers;
}

addLoadEvent(fnSubmitAnswer);
2

Please note, this is a quick analysis and probably could be done better…

The HTML portion is ok, but needs a few fixes:

Javascript portion:

  • Duplicated code:

  1. You can probably do without the createElement/appendChild/createTextNode if you made the “answerPlacement” div to a p, then did an innerHTML on the id.
    https://developer.mozilla.org/en/DOM/element.innerHTML

  2. “// Question & Answer One” Section could probably go with the onsubmit portion of the code. Remove the text, check the answer, then return false. OnSUMBIT instead of onclick, since you are using a form.
    http://reference.sitepoint.com/html/event-attributes/onsubmit

  3. answerParagraph.style.color could be answerParagraph.setAttribute(‘class’, ‘???’). Here I can set the color in CSS instead of javascript. I can also remove the attribute as well in the remove method.
    http://reference.sitepoint.com/javascript/Element/setAttribute
    http://reference.sitepoint.com/javascript/Element/removeAttribute
    http://reference.sitepoint.com/javascript/Element/hasAttribute

Look at my rewrite of your code based on the above notes:
HTML

<!DOCTYPE html>
<html>
	<head>
		<style>
			.wrong { color: red; }
			.correct { color: green; }
		</style>
	</head>
	<body>
		<div>
			<p>Quick Question</p>
			<p>Who invented the World Wide Web?</p>

			<form id="questionOne" method="post" action="#">
			
				<label for="answerOne">Answer:</label>
				<input type="text" value="" id="answerOne" name="answerOne" />
				<input type="submit" value="Submit" id="btnSubmit" name="btnSubmit" />
			
				<p id="answerPlacement"></p>
			
			</form>
		</div>
	</body>
</html>

Javascript


var frm = document.getElementById('questionOne'),
	answerPlacement = document.getElementById("answerPlacement"),
	answerOne = 'Tim Berners-Lee',
	fnAnswers = {
		AnswerResponseCorrect : function() {
			answerPlacement.setAttribute('class', 'correct');
			answerPlacement.innerHTML = 'Correct';
		},
		AnswerResponseWrong : function() {
			answerPlacement.setAttribute('class', 'wrong');
			answerPlacement.innerHTML = 'Wrong';					
		},
		RemoveAnswer : function() {
			if( answerPlacement.hasAttribute('class') ) {
				answerPlacement.removeAttribute('class');
				answerPlacement.innerHTML = '';							
			}
		}
	};
	
frm.onsubmit = function() {
	fnAnswers.RemoveAnswer();
	if( frm.answerOne.value !== answerOne ) {
		fnAnswers.AnswerResponseWrong();
	} else {
		fnAnswers.AnswerResponseCorrect();
	}
	return false;
};