Welcome to the sixth entry in the Intermediate ColdFusion Contest. The earlier entries may be found at the end of this post. Today’s entry is from Daniel Ness. Before reading on, please check his application here. You can download his code from the download link at the bottom. Please respect the copyright of the creator. So - I like the background. Not important - but I thought it was cool. I’m not sure why, but this Blackjack game seemed to beat the pants off of me. I’d say the dealer won three out of four hands. I also have to admit to not getting what “Ante” met at first. As I said though, I’m not much of a card player.
Let’s now dig into the code a bit. I noticed a few problems right off the bat with his Application.cfc file. It is a bit short so I’ll paste it all here:
<!--- New application object, with timeout and sessions --->
<cffunction name="onRequestStart" returntype="void" output="false"> <!--- If we need a new session, or they want to restart the session, go here ---> <cfif not isDefined("application.highscore") or isDefined("url.AppRestart")> <cfset application.highscore=1000> </cfif> <cfif not isDefined("session.blackjack") or isDefined("url.restart")> <!--- We store all the information in the session struct. ---> <cfset session.blackjack = createObject("component","cfc.blackjack").init()> <cfset session.dealer = createObject("component","cfc.blackjackplayer").init()> <cfset session.player = createObject("component","cfc.blackjackplayer").init()> </cfif> </cffunction> </cfcomponent> </code>
The main problem I have is that he is using the Application.cfc file - but not really using the power of it. Notice how he defines variables in both the Application and Session scope - but he does so in the onRequestStart method. These values should be set in the onApplicationStart and onSessionStart method instead. I mean - sure - they do work here - but why bother using the Application.cfc framework if you don’t leverage the power of those methods?
Some nit picking from his blackjack.cfc: For the methods that don’t return anything, like newHand, he should add a returnType=”void”. You don’t need it, but it makes the code easier to read. In the same method (and in others), he does not use “arguments.” before his argument variables. Again - not necessary - but does make the code easier to read. I recommended in an earlier post that all variables should either be Variable scoped (for global CFC variables), Argument scoped, or with no scope to indicated a local variable. Another minor nit - he created a method to return the game’s status, but didn’t actually use it. Instead, he normally directly accessed the variable. I’d also say he should have added a corresponding set method if he was going to use the get method. All of the above are minor things and I’m sure some folks would disagree, so take the advice with a grain of salt.
Oh - and he didn’t var scope. I bet you thought I’d miss that, eh? -grin-