Welcome to the 8th entry in the ColdFusion Newbie contest. This is one of three remaining entries. At the end I'll be recapping the entries, talking about what I saw (both good and bad) and announcing the winners. (Did I say winners? Yes I did. I may have more than one prize to share.) As I'm getting close to wrapping up, I'd like to encourage folks to speak up and let me know what they think of these entries. Also feel free to contact me and tell me what you think of the contest.
So without further ado - lets talk about entry 8, created by Rob Makowski. This one has no name either so I'll be calling it Creature again. It uses an Access database for storage so I'll be pointing folks to a download again and not hosting an online demo.
The game did not seem to work well for me. The creature would sometimes die on the second turn after creating a creature. Because of this I didn't play it very much since there just didn't seem like there was much to do. But I did some interesting points in the code I thought I'd point out.
First off - once again I see an odd use of request variables. Consider the Application.cfm file:
<!--- Application Name --->
<cfapplication name="MonsterApp" sessionmanagement="Yes" clientmanagement="yes" sessiontimeout="20">
<cfparam name="Session.UserName" default="">
<cfparam name="Session.Password" default="">
<cfset Request.MainDSN = "creature">
Ok it isn't odd per se - but if he is starting an Application here, why not use Application.MainDSN again? I have to ask (and this is for all my newbies in the crowd), where are you guys seeing so much use of the Request scope for what (normally I'd say) should be Application scoped variables.
Another issue I see is how he handled forcing a login. The index.cfm file (which I'm happy to see he has) has one line:
<cflocation url="login.cfm">
So I get the idea here. If the user just came in to the site, make him login. But the big problem with this though is that a person may not request the index.cfm file (or no file) first.
Now he does have code to handle that. He has a file named header.cfm that checks for login status. If you aren't logged in it pushes you to the login page. All secured pages include this header.
Now this works. Don't get me wrong. But I think it's worrisome in a few ways. First off - to a new developer looking at the code (like me!), it should be more obvious what pages are secured and what are not. Secondly - this system of including a file depends on you remembering to include the header.
Imagine a page that is "Print View" for an article. The print view may not use the standard header, and therefore may not be secured.
This is why we typically use onRequestStart or Application.cfm checks. By having it one place we know ColdFusion will run, we can be more secure that our files will be truly locked down.
Moving on to a new problem. One thing I saw in a few different forms was code like the following (pseudo-code):
<cfquery>
select x from foo
</cfquery>
<cfset x++>
<cfquery>
update foo set x = #x#
</cfquery>
While this works, it isn't single threaded or within a transaction and could lead to incorrect data. It is a heck of a lot easier to just do this:
update foo
set x = x + 1
So outside of that I saw the common 'missing var scope' mistake. I was very happy to see cfqueryparam being used. It wasn't used everywhere - but he did use it in some places. Good thinking there. Get in the habit of always using it and don't forget it. (And did you know that ColdFusion 8 allows you to use both query caching and queryparam now?)
So - comments anyone?
Archived Comments
I think the Application/Request DSN stuff comes from articles like this:
http://cfdj.sys-con.com/rea...
I would hope not. That article is from 2001. Charlie is a smart guy, and he was definitely right then - but the article is way out dated now.
As one who is guilty in this contest of using the request scope for setting the DB, I can tell you that I "learned" that from some article on the internet. I'm not sure when the article was published (or where I found it, come to think of it...).
For me, it was one of those things that I learned right when I was starting out. It worked for me, I didn't have anyone to critique the usage, and I had no idea that it was a poor practice.
So I guess that is the etymology of my error, and I have to say that I appreciate being set straight!
<i>And did you know that ColdFusion 8 allows you to use both query caching and queryparam now?</i>
I saw that! I am very excited for that possibility, as i have a new project I am working on that will definitely benefit in performance from this option.
Yes, the problem is these things become noted as best practice (e.g. internally within a company), then when they change nobody's updating their guidelines... and the articles remain online without any update saying 'this no longer is recommended'!
To be fair though - the reader has to take some responsibility and note the date of a technical article.
yes, but to be fair, in my opinion and experience most CF developers aren't reading the blogs, subscribing to mailing lists and forums etc. Instead their organisation will have some coding guidelines, probably written down by some developer years ago. They've probably never seen Charlie's original article, but at some point they've been told the best practice is request.dsn, and they've stuck with it since. And in turn they pass snippets of knowledge like that on to their colleagues. This is probably done without any understanding of why it was a good idea at the time, or why it doesn't apply any more.
Well, I'll add to exists' comments - as a Newb working and learning by himself, I have learned by reading the CFMX Web Ap kit (5th Edition) and by skimming code from whatever I can find online. (how to's, examples and houseoffusion.com's Newb Forum.) And of course - by stumbling in the dark. I have seen several different ways of doing stuff - especially setting datasource - and was never sure of what the difference was or what was better - or what my standards "should" be. And, all things being "equal" - I sort of used the method I learned initially and stayed with that. Keeping with the old standby "If it aint broke..."
However, entering this contest is my way (and probbaly others way) of fixing what aint broke - but may not be running as smoothly as possible.
With all that in mind, I think it is important for all the mentors to keep an open mind about our ignorance. We can't always control the quality of our teaching, especially considering the sources. i.e. The Internet. Speaking for myself - when I see posted code on the internet, I always assume they know more than I do - or more importantly, know what is right - and sooner or later I latch onto "their" practice. Right or wrong, because, I don't know any better. After awhile you sort of create your own Frankenstien's Monster of internal standards. Frieghtening, I know.
And - I'll admit, that it's sometimes difficult for me to keep up with the suggestions. (I am hoping this is true for some of the other contestants as well.) "The why did we do that?" is easy - because it's what we know or knew how to do when we wrote it. The why didn't we use this scope or that transaction with this .foo - well - criminey - if I knew that I wouldn't consider myself a newb anymore. ;)
It is sort of funny, that we're in a contest that will essentially reward "The most advanced Newb!" Does that mean you have progressed to the next level or programmer? (Novice?) That you're the Best of The Worst?
Either way - it's about learning, and Lord knows I'm learning a lot through this experience. Thanks to everyone as always.
Now, if I can just update my webhosting server from version 6, I'll be set!
You also have to remember that there are a lot of people out there still stuck running CF 5 or are just upgrading to MX7.
My understanding is that the practice of using Request.DSN was still a good practice for CF5...
Is that correct?
Nick - one big thing I'd tell you. Never assume folks know more than you. :) Always treat what you read with a critical eye. I know I've certainly made plenty of mistakes, so you can't always trust what I say as well.
Ken - yes. What I would do is create all my vars in the app scope, and do it once. Then on every request I'd do a duplicate to the request scope.
Ray,
I'm afraid I don't understand what setting these types of vars once in the App scope and then duplicating on every request actually buys you.
If anything, this seems less efficient.
If you set a var in the request scope, and the page gets hit 20 times, then the var is set 20 times.
If you set a var in app scope once and duplicate it to the request scope on every request... when the page is hit 20 times, the var is set 20 times, plus the app var is set once...
If I am way off, can you point to a resouce that can help me get up to speed?
I didn't say it was great. ;)
No - duplicate isn't the same as setting the var 20 times. It is all done behind the scenes so it should be a bit quicker.
But I've not done any testing on that.
Again - thats how I did things back in the cf5 days. If I had to do it again, I would perhaps _just_ set request vars period.
Ray asks:
> have to ask (and this is for all my newbies in the crowd), where are you guys seeing so much use of the Request scope for what (normally I'd say) should be Application scoped variables.
Well, this really is interesting... Just ran across this Application.cfc code example in CFMX7 WACK, page 530.
<cffunction name="onRequestStart"...>
<cfset REQUEST.dataSource = "ows">
<cfset REQUEST.companyName = "Orange Whip Studios">
...
</cffunction>
Comments at the top of the listing say:
...
Created by: Raymond Camden
...
;-)
Hey, I didn't say I was perfect. ;) My excuse is that is probably a chapter I updated, and didn't write from scratch.
So hey - it just so happens I'm working on that chapter this very minute, and now I remember why I kept that in. At that point in the chapter, application variables aren't yet covered, so I was trying to keep it simple. Make sense?