After listening to Dan Wilson’s session on Making Bad Code Good at cf.Objective() 2010, it really got me to thinking of some best practices that I wish everyone would follow in order to keep that maintenance period (which consumes a majority of a developer’s time and effort) as short as possible.
As a developer, if you have to think too long and hard about what a function is doing, or why a previous developer wrote it a certain way, it was probably coded incorrectly. Here are some things I’ve come across that may help budding developers create clearer, easier-to-manage code:
1. Name Your Variables Well
Have you ever come across a cfset statement which initializes a variable named “foo” or “bar”? What does this variable mean? What’s it supposed to do? What kind of object is it containing? Naming your variables and method names to identify its purpose is incredibly important when coming back to maintain your code.
There are generally two schools of thought on variable naming. Some people enjoy prepending the variable name with the type of element it will contain. For example, a query object may be set as follows:
<cfset qUsers = getUserService().getAllUsers() />
Whereas others (like myself) may enjoy being a little more verbose by writing out the variable names a bit more:
<cfset usersQuery = getUserService().getAllUsers() />
2. Name Your Methods Well
Sure, this goes along with #1, naming your variables well, but it needs to be said that your method names are just, if not more, important to have named well. When the next developer that comes by to read, edit, and modify your code, it should be very apparent as to what is happening throughout the page. Even if you have fantastic comments which tell people exactly what’s going on, a method named doStuff really hurts that at-a-glance reading ability.
Change this:
<cffunction name="doSomeStuff" access="public" output="false" returntype="string"> <cfargument name="someString" type="string" required="true" /> <cfset var foo = '' /> <cfset var i = '' /> <cfloop from="1" to="#Len(arguments.someString)#" index="i"> <cfif i mod 2 eq 0> <cfset foo = foo & Mid(arguments.someString, i, 1) /> </cfif> </cfloop> <cfreturn foo /> </cffunction>
To this:
<cffunction name="getEverySecondCharacterFromString" access="public" output="false" returntype="string"> <cfargument name="someString" type="string" required="true" /> <cfset var returnString = '' /> <cfset var iter = '' /> <cfloop from="1" to="#Len(arguments.someString)#" index="iter"> <cfif iter mod 2 eq 0> <cfset returnString = returnString & Mid(arguments.someString, iter, 1) /> </cfif> </cfloop> <cfreturn returnString /> </cffunction>
Notice, while there may be “more” code, it really is easier to understand what’s occurring within it without actually having to resort to going line-by-line to figure things out. Yes, I even turned the regular old i into iter because I’m a fan of verbosity.
3. Don’t Mix CFSCRIPT with Tags
One of my biggest pet peeves is the mixing of CFSCRIPT code and tag-based code. Everyone has their own style of coding and a lot of people enjoy using one over the other, but be consistent. It’s difficult to read blocks of code that switch between the two. Stick with one to make the next person’s life easier.
Bad:
<cfscript>
var firstName = arguments.firstName;
var lastName = arguments.lastName;
</cfscript>
<cfset var fullName = firstName & ' ' & lastName />
<cfscript>
if (Len(fullName) eq 6) {
// 6 character name!
return 'Your name is 6 characters!';
}
else {
// not a 6 character name
return 'Your name is either too short or too long!';
}
</cfscript>
Good:
<cfset var firstName = arguments.firstName /> <cfset var lastName = arguments.lastName /> <cfset var fullName = arguments.fullName /> <cfif Len(fullName) eq 6> <!--- 6 character name! ---> <cfreturn 'Your name is 6 characters!' /> <cfelse> <!--- not a 6 character name ---> <cfreturn 'Your name is either too short or too long! ---> </cfif>
4. Format Your Code, Use Your Tab Key!
This is a no-brainer for most developers that have any level of experience, but use your tab key to indent your code! Providing a visual structure to your code makes your code much more human readable.
Why do this?
<cffunction name="sayHello" access="public" output="false" returntype="string"> <cfargument name="name" type="string" required="true" /> <cfset var returnString = '' /> <cfset var iter = '' /> <cfif Len(arguments.name)> <cfloop from="1" to="10" index="iter"> <cfset returnString = returnString & "hello, " /> </cfloop> <cfreturn returnString & arguments.name /> <cfelse> <cfreturn "I'm sorry, I didn't catch your name..." /> </cfif> </cffunction>
When you can have this?
<cffunction name="sayHello" access="public" output="false" returntype="string"> <cfargument name="name" type="string" required="true" /> <cfset var returnString = '' /> <cfset var iter = '' /> <cfif Len(arguments.name)> <cfloop from="1" to="10" index="iter"> <cfset returnString = returnString & "hello, " /> </cfloop> <cfreturn returnString & arguments.name /> <cfelse> <cfreturn "I'm sorry, I didn't catch your name..." /> </cfif> </cffunction>
5. Don’t Instantiate Variables If You Don’t Have To
How many times have you seen code where you load something into a variable and then return it right away? All this does is add additional (unneeded) code to your functions and decreases readability at the same time. It’s just another thing that could break down the line (what if you forget to varscope?), so leave it out.
Turn this:
<cffunction name="getUserNameByID" access="public" output="false" returntype="string"> <cfargument name="userID" type="numeric" required="true" /> <cfset var userName = getUserService().getUserNameByID(arguments.userID) /> <cfreturn userName /> </cffunction>
Into this:
<cffunction name="getUserNameByID" access="public" output="false" returntype="string"> <cfargument name="userID" type="numeric" required="true" /> <cfreturn getUserService().getUserNameByID(arguments.userID) /> </cffunction>
There’s also the case where you’re returning boolean values. You can simplify code dramatically by returning the right value in the return statement rather than using independent return statements for true and false. For example, this:
<cffunction name="userExists" access="public" output="false" returntype="boolean"> <cfargument name="userID" type="numeric" required="true" /> <cfset var userQuery = '' /> <cfquery name="userQuery" datasource="#variables.dsn#"> SELECT user_id FROM Users WHERE user_id = <cfqueryparam cfsqltype="cf_sql_integer" value="#arguments.userID#" /> </cfquery> <cfif userQuery.RecordCount gt 0> <cfreturn true /> <cfelse> <cfreturn false /> </cfif> </cffunction>
Could be rewritten as:
<cffunction name="userExists" access="public" output="false" returntype="boolean"> <cfargument name="userID" type="numeric" required="true" /> <cfset var userQuery = '' /> <cfquery name="userQuery" datasource="#variables.dsn#"> SELECT user_id FROM Users WHERE user_id = <cfqueryparam cfsqltype="cf_sql_integer" value="#arguments.userID#" /> </cfquery> <cfreturn userQuery.RecordCount gt 0 /> </cffunction>
Summary
While a lot of code and how it’s laid out is really determined by personal style, there are several things that can be done that’ll really aid in future maintenance of your code. Being clear about what your variables and methods do, formatting consistently, and providing easy to understand logic are keys to decreasing future maintenance costs.
Tags: beginner, cf.Objective(), ColdFusion, maintenance