ColdFusion Contest Entry Examined - Part 4
Welcome to the fourth (getting bored yet?) entry in the ColdFusion contest I am running. If you haven’t yet, take a look at entries one, two, and three. This entry is a bit simpler so I won’t have much to say with it. You can demo it here, and the source code follows:
Application.cfm:
<cfapplication name="GuessaNumber" applicationtimeout="#CreateTimeSpan(1,0,0,0)#" sessionmanagement="true" sessiontimeout="#CreateTimeSpan(1,0,0,0)#" setclientcookies="true">
index.cfm
<script>
var xmlhttp=false;
/*@cc_on @*/
/*@if (@_jscript_version >= 5)
// JScript gives us Conditional compilation, we can cope with old IE versions.
// and security blocked creation of the objects.
try {
xmlhttp = new ActiveXObject("Msxml2.XMLHTTP");
} catch (e) {
try {
xmlhttp = new ActiveXObject("Microsoft.XMLHTTP");
} catch (E) {
xmlhttp = false;
}
}
@end @*/
if (!xmlhttp && typeof XMLHttpRequest!='undefined') {
xmlhttp = new XMLHttpRequest();
}
// function getfile will perform our request using our created xmlhttp object // <cfoutput> function guess(number) { var Current = “guess.cfm?guess=”+number+”&RandomKey=” + Math.random() * Date.parse(new Date()); xmlhttp.open(“GET”, Current, true); xmlhttp.onreadystatechange=function() { if (xmlhttp.readyState==4) { document.getElementById(‘results’).innerHTML = document.getElementById(‘results’).innerHTML + xmlhttp.responseText; } } xmlhttp.send(null) } </cfoutput> </script>
<body>
<cfif NOT isNumeric(“session.HiddenNumber”)> <cfset session.HiddenNumber = RandRange(1,100)> </cfif>
<style> #results { position: absolute; width: 400px; border: 1px solid black; }
#guess { position: absolute; margin-left: 410px; width: 300px; height: auto; padding: 10px; border: 1px solid black; }
.low { color: blue; }
.high { color: red; } .win { color: purple; } </style>
<div id=”results”> I am thinking of a number between 1 and 100! Guess what it is!<BR> </div>
<div id=”guess”> <h1>Guess the Number!</h1> <i>Please key in the number you wish to guess, then click the link!</i> <form name=”myform”> <input name=”myguess”> <a href=”javascript:void(0);” onclick=”guess(myform.myguess.value);” onkeypress=”checkenter();”>Click to Guess!</a> </form> </div> </code>
guess.cfm
<cfif NOT IsNumeric(URL.Guess)>
I am sorry, you must enter a number to guess. Please try again!
<cfabort>
</cfif>
<cfset yourguess="#session.Hiddennumber - URL.guess#">
<cfoutput> <cfif yourguess eq 0> <p class=”win”>You got it! The number was indeed #Session.Hiddennumber#!<BR> <cfset Session.Hiddenmenu = ‘done’> <a href=”index.cfm”>Click to Play Again!</a></p> <cfelseif yourguess LT 0> <p class=”high”>You guessed #URL.Guess# - that is too high!</p> <cfelseif yourguess GT 0> <p class=”low”>You guessed #URL.Guess# - that is too low!</p> </cfif> </cfoutput> </code>
Here are my comments in no particular order.
He uses a simple form of AJAX, which means he loses points for applying a buzzword. (Kidding.) For a simple reload, it seems like overkill, but I kinda like it.
As I’ve mentioned in the previous entries, I did not expect great design, but I have been very pleased with the simple things these beginners are doing. In this demo, he uses two different colors for the messages for “Too High” and “Too Low”. Simple - yet very effective.
He uses a timeout value for his application, and session, of one day. I’d say that is a bit of a security risk for his session data. Not a big huge deal, obviously, but I’d question why a session needs to stay around for a full day. I’d recommend changing it to the default 20 minute timeout unless he has a good reason.
He does validation for the guess number (yeah!), but only part way. He checks to ensure the guess is numeric, but does not check to see if it is an integer (3.14159 is numeric as well) and does not check to see that it is between one and a hundred. This brings me back to my last Macrochat about being sure to go the extra mile in validation.
Lastly, there is another bug he missed, and I missed it as well. I had noticed this block:
<cfif NOT isNumeric("session.HiddenNumber")>
<cfset session.HiddenNumber = RandRange(1,100)>
</cfif>
And I had asked myself - why isn’t this code throwing an error since he never defaulted session.HiddenNumber? It took me a minute or so before I noticed the quotes around session.hiddenNumber. He wasn’t checking the variable, but the actual string. What he should have had was this:
<cfif NOT structKeyExists(session,"hiddenNumber") or not isNumeric(session.hiddenNumber)>
<cfset session.HiddenNumber = RandRange(1,100)>
</cfif>
Bugs that don’t throw errors are always the hardest to debug, luckily this one was a bit easier once my eyes actually focused. (Need…more…coffee…) I kept the isNumeric check in there since his “win” state sets the value to “done” as a way to flag index.cfm to reset the value. You could change that to simply remove the value as well as an alternative.
Comments