Major Tom! Don't you dare leave that capsule!

Sing along:

This is Ground Control to Major Tom
You've really made the grade
And the papers want to know whose shirts you wear
Now it's time to leave the capsule if you dare

In all seriousness, encapsulation is important. It also may be one of the most thoroughly misunderstood concepts in programming. I find myself so frequently looking at code like this:

<cfcomponent displayname="User">
...
<cffunction name="updatePassword" access="public" output="false" returntype="any">
   <cfargument name="UserID" required="yes" type="string">
   <cfargument name="UserPassword" required="yes" type="string">
   
   <cfstordproc procedure="dbo.update_password" datasource="#this.DSN#">
      <cfprocparam value="#UserID#" cfsqltype="cf_sql_integer" />
      <cfprocparam value="#trim(UserPassword)#" cfsqltype="cf_sql_varchar" />
      <cfprocresult result="1" name="qAuth" />
   </cfpstoredproc>

   <cfreturn qAuth />
</cffunction>
</cfcomponent>

Now there are a number of problems with this code. One of the more glaring problems is that the query result name qAuth isn't scoped using the var keyword at the top of the function. In this particular case, the variable name qAuth isn't very likely to collide with anything else in this component. But for the cost of one very simple line of code (<cfset var qAuth = 0 />), you can go from "unlikely to cause a problem" to COMPLETELY BULLETPROOF. That being said, it should be done every, every EVERY EVERY EVERY TIME. Did I mention every time?

The problem here is mostly the fact that since they've left this function in this sloppy, potentially race-condition creating state, the odds are that the programmer is in the habit of writing functions this way. That will bite you in the ass, it's just a matter of time.

That one's easy though. That's the one that most of us who've been doing this for a while make a point of bringing up again and again and again when we see it. The other issue is more subtle and harder to explain, but no less important. It involves the arguments. This is a User.cfc which manages data for a specific user account and handles password authentication and a few other things. Most of us in the ColdFusion community these days would have separated this out into a bean and a DAO and are probably expecting me to make some mention of that, but I won't. Internally within the function the component can execute a query or stored procedure or it can call a DAO to do that, I don't really care.

What I do care about however is that I don't have to repeat myself... you know, DRY. Thing is, if I've already created a User.cfc and called the init() method, then I'm sure I've already given it a userID. Then WHY on god's green earth am I telling it which user's password is being set?!

To give you an analogy, let's say that I work with two guys, Bob and Jeff. Now these two guys are very punctual and are always at their desks (and their desks are equidistant from mine). So when I need to ask Jeff to do something for me, do I go through Bob? Do I say "hey Bob, you know, Jeff's desk is right there and I could ask him myself, but I'd rather ask you to ask him to have a look at bug #73, could you do that for me?" ... Or do I ask Jeff, "hey Jeff, could you look at Bug #73 for me?"

So here I have a User component (Bob) and I'm asking bob to set a password, but rather than just saying "okay, my password is now BLUE", the Bob object DEMANDS that I expressly tell Bob that I want to change Bob's password. This would be like asking Jeff, "hey Jeff, could you look at bug #73 for me?" and having Jeff say "Wait! Who do you want to look at bug #73, me or Bob?"

Don't leave the capsule.

If the component is User.cfc and you're using it to set a password and users have passwords, then there should be ONE and only ONE argument in that function: the password. It already knows who it is. If you need to change the password on the Bob record, change it with a Bob instance of User.cfc -- if you need to change the password on the Jeff record, change it with a Jeff instance of user.cfc.

This particular function individually wouldn't even be a problem. Again, the problem is because it probably indicates a habit of designing functions this way, forcing repetition and strong-coupling.

In this particular case, the instance variables for the userid are unfortunately also stored in the "this" scope instead of the variables scope. There are cases (rare) in which I've chosen to do that for the purpose of expedience, but this isn't one of them. Boy do I wish "this" meant private variables.

Again... the problem isn't an individual component, it's the habit.

Don't get in the habit of leaving the capsule.

Comments
BlogCFC was created by Raymond Camden. This blog is running version 5.5.006. | Protected by Akismet | Blog with WordPress