Performance tuning for ColdFusion applications and Comment

Kunal Saini, an Adobe employee, recently posted an article on Adobe Developer Connection about Performance tuning for ColdFusion applications. This is a well written article full of useful tips and practices and should be a must read on the topic.

I will raise a counter point to one of the minor tips Kunal raised. He says:

compare() and compareNoCase()

Use compare() or compareNoCase() instead of the is not operator to compare two items. They are a bit faster.

I trust Kunal has insider knowledge about the implementations of these two compare functions, because I fail to see how a straight evaluation (<cfif dan IS 1337>) can be slower than a function call ( compare(dan, 1337) IS 0 ). Maybe it has to do with the type inference and type conversion ColdFusion does as a dynamically typed language, maybe it is something else. Regardless I avoid using compare() and compareNoCase() because both functions reduce the readability of the code.

Whereas all boolean comparisons in ColdFusion treat 1 as true and 0 as false, the compare and compareNoCase functions return 0 if the comparison is true. This means compare( 1, 1) will return 0, which doesn't follow the boolean rules. Since this does not follow the rules, code using compare and compareNoCase is harder to read, harder to follow, and generally uglier than straight comparisons.

So Kunal, I don't take anything away from your statements and I appreciate you writing the article. I want to point out that software isn't all about micro-performance, it is also about long-term maintainability. Always write your code to be readable by others.

Of course, if you happen to write the next Facebook and you need to squeeze every possible fraction of a millisecond out of a routine, then throw this advice right out the window. But then again, you'd have already tuned every single query permutation, added a clustered caching layer, offloaded your static files to a Content Delivery Network and clustered your infrastructure Horizontally and Vertically, haven't you?

The Art Of Method Names

I write from time to time on code quality and structure because it is a topic of interest to me. Clean code and well named logical structures, methods and objects really pay off during the infinitely long Support And Maintenance phase of software development.

Some could accuse me of having too many opinions on the topic, and I'd guess they could be on to something. Heck, I'll probably disagree with something I've said today, tomorrow, just because I'm always refining and learning.

While some of what I think/advocate/do is opinions, and could be subjective, I'd like to share some code I found on a project today and talk about the importance of method names.

A method should describe it's intent or behavior at the level of where it is inside the program. For example, a method named load() might be sufficiently descriptive to represent the behavior and be flexible enough to withstand a refactor or two. In other places in the program, perhaps the right method name is loadShippedOrders() since there will always be the concept of a shipped order in our proverbial system.

You get the point, right? There is a wide range of OK-ness for method names, with behavioral descriptiveness and refactorability as being two made up words that really judge the method name quality.

I found code today that really flies in the face of any of these principles. The names of these methods do not in any way describe any behavior of any system I've ever written, nor will probably be lucky enough to write.

method bodies removed to protect client interests

Be the Judge Yourself:

view plain print about
1<cfcomponent name="ET">
2 <cffunction name="phoneHome" output="yes">
3 </cffunction>
4
5 <cffunction name="createDir">
6 </cffunction>
7
8 <cffunction name="createDirImpl">
9 </cffunction>
10
11 <cffunction name="beamMeUp">
12 </cffunction>
13
14 <cffunction name="energize">
15 </cffunction>
16
17 <cffunction name="setPhaserToKill">
18 </cffunction>
19</cfcomponent>

Before you ask, this code was found in a production eCommerce system that is currently running that has nothing to do with Phasers, Energization nor beaming anything to any location.

Hard Coding Scopes In CFCs Is A No No!

Now Hear This!

This is a public service announcement. If you hard code scopes inside your CFCs (request, application, session), stop today.

I know it might be 'easier' or 'cleaner' or less lines of code, but you are really painting yourself into a corner when you do this.

An object (CFC) should not know or have access to ANYTHING outside of itself, its configuration and its immediate dependent objects.

If you want to question/argue with me on this, go for it. (Just go Read Up On Information Hiding before you do).

That Is All

I Present Making Bad Code Good To The CFMeetup March 5th @ 6:00 EST

At 6:00 EST this Thursday, March 5th, I present Making Bad Code, Good to the Online ColdFusion Meetup. You can attend this presentation virtually, by visiting the Online ColdFusion Meeting Room at 6:00 EST.

If you work on a legacy application, or on code built by lots of developers over the years, you likely laugh your way through this presentation. I promise to be thought provoking and challenge the way you write code. In this session, we'll look at lots of code samples and walk through making incremental changes to speed development, reduce errors and make life easier for everyone involved.

Ideas and concepts in this presentation will help you improve your existing applications and write more maintainable code.

The recorded presentation can be watched now!

Of Software Design, The Law of Demeter and Credit Card Companies

The Law of Demeter is a Software Engineering principle guiding how objects should talk to each other. From Wikipedia:

The Law of Demeter (LoD), or Principle of Least Knowledge, is a design guideline for developing software, particularly object-oriented programs ...[and] ...can be succinctly summarized as "Only talk to your immediate friends." The fundamental notion is that a given object should assume as little as possible about the structure or properties of anything else (including its subcomponents).

We Don't Need No Stinking Laws

Now, I'm no fan of laws, so I privately refer to this as the General Guideline of Demeter, even though it doesn't sound as snappy or cool. However, just because there are valid reasons to break it, doesn't take away from the validity of the intent. Let's look at 2 code examples blatantly ripped off from the CFCDEV mailing list:

Conforms To Law General Guideline of Demeter

view plain print about
1Room.canCustomizeWindow()
2Room.canSelectStyle()
3Room.hasCeilingFan()

Violates Law General Guideline of Demeter

view plain print about
1Room.getPlan().canCustomizeWindow()
2Room.getPlan().canSelectStyle()
3Room.getPlan().hasCeilingFan()

Ok, So What?

In the conforming set of statements, the room is asked directly whether or not certain things can happen. The implementation (steps required to complete the task) are hidden from the calling code. This is encapsulated and will help insulate callers from changes in the implementation.

In the violating set of statements, the calling code has to get a reference to Room, then ask Room for a Plan and then query the plan. Now calling code is expected to know about this Plan and what the plan knows. This adds another level of coupling and if Plan changes, then a whole lot of code has to change as well.

However, to the programmer, the violating syntax (Room.getPlan().canSelectStyle()) could make sense. It might be that the programmer doesn't want to refactor Room and using getPlan() is a faster way to do something. If the code works, is it wrong?

I Don't Follow All This Abstract Stuff. You Are Losing Me!

Ok, fair enough. My eyes glaze over with too much abstract stuff too. Let's look at an analogy.

I made a call to a credit card company. The essence of the call was:

CC Rep: CreditCardRep.answerPhone()
Thanks for calling Law Of Demeter Credit Card Company, how may I help you?

Me: Hi, My name is Dan Wilson. I have a question as to a charge on my latest bill.

CC Rep: Hi Dan, I'm Tracy. I can help you with that. For security purposes, what is your account number, mothers maiden name and shoe size?

Me: Acct: 333-444-555-5555 mothers maiden: Stratulat Shoe Size: 9

CC Rep: CreditCardRep.verifyAccountInformation()
     Perfect. What is the charge you wish to inquire about?

Me: I have a 17.99 charge to ILovePets.com on Feb 7th. I can't find my receipt so I don't know what this is for.

CC Rep: CreditCardRep.lookUpTransaction() From the transaction details, you purchased a red dog sweater. Do you recall that purchase?

Me: (Embarassed) Yeah. ok No problem. I also wanted to change my mailing address. Can you help me with that?

CC Rep: Of course, what is the new address?

Me: 123 ColdFusion Lane, Surf City, North Carolina.

CC Rep: CreditCardRep.updateAccountAddress()
     Ok. I've updated the address. Is there anything else I can help you with?

Me: No thanks. That takes care of me.. have a nice day!

CC Rep: OK Dan. Thanks for calling Law Of Demeter Credit Card Company. Have a nice day.

What Am I Supposed To Get From That Example?

Note how I called the CC company and talked with a representative. I only spoke to that representative and was not exposed to any implementation nor had to talk to any other objects to get my tasks done. Let's look at the converse example:



CC Rep: CreditCardRep.answerPhone() Thanks for calling Demeter Violation Credit Card Company, how may I help you?

Me: Hi, My name is Dan Wilson. I have a question as to a charge on my latest bill.

CC Rep: Hi Dan, I'm Tracy. I can help you with that. For security purposes, what is your account number, mothers maiden name and shoe size?

Me: Acct: 333-444-555-5555 mothers maiden: Stratulat Shoe Size: 9

CC Rep: CreditCardRep.verifyAccountInformation()
     Perfect. I can refer you to our Payment Inquiry Department. May I place you on hold?

Me: Ummm... ok.

CC Rep: Perfect. CreditCardRep.getPaymentInquiryRep() ......

CC Rep 2: Hello, this is Jessica, the Payment Inquiry ObjectRepresentative. For security purposes, what is your account number, mothers maiden name and shoe size?

Me: (Grumbles... didn't I already say this once?) Acct: 333-444-555-5555 mothers maiden: Stratulat Shoe Size: 9

CC Rep 2: CreditCardRep.getPaymentInquiryRep().verifyAccountInformation()
     Perfect. What is the charge you wish to inquire about?

Me: I have a 17.99 charge to ILovePets.com on Feb 7th. I can't find my receipt so I don't know what this is for.

CC Rep 2: CreditCardRep.getPaymentInquiryRep().lookUpTransaction()
     From the transaction details, you purchased a red dog sweater. Do you recall that purchase?

Me: (Embarassed), yeah. ok No problem. I also wanted to change my mailing address. Can you help me with that?

CC Rep 2: No, I am sorry. Our Address Change department handles that. Would you mind if I placed you on hold?

Me: (Grumbles, looks at watch) Ummm... ok.

CC Rep 2: Perfect. CreditCardRep.getPaymentInquiryRep().getAddressChangeRep() ......

CC Rep 3: Hello, this is Ann, the Address Change ObjectRepresentative. For security purposes, what is your account number, mothers maiden name and shoe size?

Me: (Grumbles... Kicks Dog) Acct: 333-444-555-5555 mothers maiden: Stratulat Shoe Size: 9

CC Rep 3: CreditCardRep.getPaymentInquiryRep().getAddressChangeRep().verifyAccountInformation() Perfect. What is the new address?

Me: 123 ColdFusion Lane, Surf City, North Carolina.

CC Rep 3: CreditCardRep.getPaymentInquiryRep().getAddressChangeRep().updateAccountAddress()
     Ok. I've updated the address. Is there anything else I can help you with?

Me: No thanks. That takes care of me.. have a nice day!

CC Rep 3: OK Dan. Thanks for calling Demeter Violation Credit Card Company. Have a nice day.

And What Shall I Gain From That Example?

I'm sure you have had a similar experience calling a credit card company. Did you feel like their processes were well designed? Wasn't it a much cleaner experience to just deal with the main object and let it handle the implementation of getting the tasks done?

Circling back to the Law of Demeter, note this passage from Wikipedia:

When applied to object-oriented programs, the Law of Demeter can be more precisely called the "Law of Demeter for Functions/Methods" (LoD-F). In this case, an object A can request a service (call a method) of an object instance B, but object A cannot "reach through" object B to access yet another object, C, to request its services. Doing so would mean that object A implicitly requires greater knowledge of object B's internal structure. Instead, B's class should be modified if necessary so that object A can simply make the request directly of object B, and then let object B propagate the request to any relevant subcomponents. Or A should have a direct reference to object C and make the call directly. If the law is followed, only object B knows its own internal structure.
So if I am Object A, should I really be exposed to the fact that the credit card company even has a Payment Inquiry Department or an Address Change department? Surely these internal details should be kept inside the CC Company Object, not sprinkled through all the various objects that interact with a Credit Card Company.

What Should I Take Away From This Nonsensical Post?

The call dialogue examples above are contrived, I admit, but think of the above dialogues as a set of design requirements for software. Now ask yourself the following questions:

  • What would have to be updated in each of the designs if the CC company added an account verification department that validated account number, mothers maiden name and shoe size?
  • What would have to be updated in each of the designs if the CC Company merged the Payment Inquiry department with the Address Change Department?
  • Which design better encapsulates the implementation from the caller?
  • Which design incurs less ripple effect from design changes?

Thoughts, concerns, comments? Add them below!

Tip to Speed Up Your Website - Compress CSS

There are a number of ways to speed up a website. An easy one would be to compress asset files and compact the files. This has been widely done for Javascript files with popular tools such as JSMIN and Packer.

The general idea behind compression/combination is to reduce the number of characters that must be sent over the wire as well as reduce the number of HTTP calls that must be made. Each time a browser gets a request to download a JS file, there is a certain amount of overhead incurred in negotiating and completing the HTTP request. Combining all JS files into one file is a great way to speed up a web application.

Everyone Already Knows This, Right?

Probably. However, CSS files can often be as numerous and verbose as Javascript files. How come there no public outcry for CSS compression/combination?

There happens to be a compressor/combinator that handles CSS files, the YUI Compressor. For most web application developers, YUI Compressor is an annoying tool to use because, as a java application, it must be installed and run from the command line. Yuck!, right?

Scriptalizer, developed by ColdFusion luminary Aaron Lynch, is a web front end for the YUI compressor. Scriptalizer has handled Javascript compression/combination for a while now and is a nicely designed, easy to use tool. Aaron recently added support for CSS compression/combination. Now, dealing with CSS files is as simple as dealing with JS files.

How well does it work?

I added all 14 CSS files from The Health Challenge and compressed/combined them with Scriptalizer. Here are the results:

  • Number of Files Before: 14
  • File Size of All Files: 35.42 KB
  • Number of Files After: 1
  • File Size of All Files: 19.96 KB

As you can see, the reduction was significant. Not only have I cut the size of my CSS assets by ~50%, I have also removed 13 HTTP connections.

Fix for: 500 Null Corrupt form data: no leading boundary

Another 500 Null Error / Solution

I ran into a strange error in the registration section of TheHealthChallenge.com where Internet Explorer users (Editors Note: Remove defamatory comments re: Internet Explorer Development Team and gratuitious comparisons about the size of their brains vs. size of their egos ) clicking a button would cause a 500 Null.

Here are the error details:

view plain print about
1500
2Corrupt form data: no leading boundary: != -----------------------------7d93d92a60680
3
4
5java.io.IOException: Corrupt form data: no leading boundary: != -----------------------------7d93d92a60680
6    at com.oreilly.servlet.multipart.MultipartParser.<init>(MultipartParser.java:174)
7    at com.oreilly.servlet.multipart.MultipartParser.<init>(MultipartParser.java:93)

Can anyone venture a guess as to what the problem was? You want more information? Take a look at the form code as well:

Here is the HTML for the Form:

view plain print about
1<form action="index.cfm?x=register" method="post" enctype="multipart/form-data" id="registerForm" class="uniForm">
2    <fieldset class="inlineLabels">
3        <div class="ctrlHolder">
4            <label for="register"> Need an Account?</label>
5            <p class="formHint">registration takes only 53 seconds</p>
6        </div>
7    </fieldset>
8        <div class="buttonHolder">
9            <button type="submit" class="submitButton">Start Registration</button>
10        </div>
11</form>

By the looks of the HTML code above, a single button will be drawn on the screen along with some friendly text. So why the error?

Solution

Apparently Internet Explorer does not handle serializing the form post if there is no content and what it sends to the server is not what the server expects. Possible resolutions for this are to remove the [enctype="multipart/form-data"] attribute or change the [method="post"] to [method="get"]. Either one will work as intended.

I happened to create this set of circumstances by using the CFUniform library in a way it was not designed for. I mentioned this to Matt Quackenbush who reworked the inner workings of the CFUniform Library to intelligently figure out if a file upload control is in the form or not. If one exists, the [enctype="multipart/form-data"] attribute will be included automatically. If you experience the 500 Null problem listed in this post, and you are using the CFUniform Library, simply update your version from http://cfuniform.riaforge.org/ and you'll be all set.

Error Message FAIL

I was working on a server migration last night. In process, I set up the DNS for the mail servers. When I entered the value for the DNS server EXACTLY like Google said, I got the below error:

Alert: The domain was not added due to an error in the dns settings. Please check your dns template and verify. The message from the dns server was dns_rdata_fromtext: :26: near 'ASPMX2.GOOGLEMAIL.COM..': empty labelzone thehealthchallenge.com/IN: loading master file : empty label

Since I don't speak fluent Southern Klingon, my mistake was not immediately obvious. I, of course, tried to submit the form several more times. When I finally read the error message, I realized the Control Panel wants to be the one to add the trailing period (.). Removing my trailing period fixed it.

This is the most indirect, least helpful error message I've seen all month. I vow in 2009 to do better than these guys when alerting my users to issues.

OO Camp comes to RTP, NC

We work fairly hard at TACFUG to keep our members informed and engaged in key information about programming and ColdFusion. Recently, Jim and I, put out a request for topics and we found some challenges in meeting the need. Some of our members have a long history of programming in ColdFusion and want to branch out into Object Oriented programming, but for one reason or another just haven't. Jim and I came up with an OO Code Camp concept and floated it out to our group to gauge interest.

Here was our announcement:

The fine folks at TACFUG (me and Jim) are seriously considering doing an OO camp starting in January. OO Camp will be a crash course on OO in ColdFusion. Ideally, we'd cover the topic in 3 or 4 evenings spread over a couple of months. This crash course will be designed to teach OO concepts and how to efficiently work with ColdFusion components. If you are new to OO or do not think you are using OO effectively, this crash course is for you. There will be no cost for this event though we may take up a donation for Pizza. Who would be ready to commit to coming to 3 or 4 evenings of OO camp? Please use this email thread for comments, questions and such about the proposed OO Camp. Feel free to extend this offer to others in your company, organization, Facebook Network, etc that would find this helpful as well.

We got a good bit of interest, certainly more than enough to justify running OO Code Camp in RTP.

Jim and I will be teaching the class and while we are pretty darned good at what we do, we've never run an organized class on this topic before. To make sure we cover all the bases and deliver appropriate, encompassing training, I thought it would be a good idea to ask the multitude of talented readers of this blog for their advice. We want to deliver the core concepts of Object Oriented Programming in a practical, hands-on fashion. Please offer, by commenting below, any constructive advice, suggestions, key topics that you feel we should cover.

Thanks in advance!

Practical Refactoring In ColdFusion - Live Example

Practical Refactoring

I've done a few presentations on refactoring in ColdFusion and in those presentations, I show a lot of code samples towards the end. No matter how eloquent I am in the first part of the presentation, the light always goes on for the audience when I show code. If you think about it, it makes sense, why try to explain concrete principles in abstract terms?

I do my refactoring in sweeps. A sweep is a cycle through the code in which I make incremental improvements. At the end of a sweep, the code should (it better) still function as well as it did pre-sweep. By breaking up the refactoring up into small sweeps, we can stay focused and keep out of the weeds.

In this article, we are going to look at some code that could use a refactor. Then we'll make a few sweeps through the code to tidy it up a bit. Along the way, I'll explain why I am refactoring out a certain piece. Keep in mind, we are all still learning. None of this is meant as Final Law of Software Design. I'm always refining my viewpoints and learning new things so I'll probably disagree with something I've done here at some point in the future. In the end, as long as this article is thought provoking for you, we've both won. Can Ya Dig?

First, here is the code sample. Scan the code and fix in your mind the general idea of what is going on. We'll pick back up on the other side.

The Original Code

view plain print about
1<cffunction name="onError" output="true">
2    <cfargument name="exception" required="true"/>
3    <cfargument name="eventName" required="true"/>
4        <cfif compareNoCase(trim(arguments.eventName),"onSessionEnd") And compareNoCase(trim(arguments.eventName),"onApplicationEnd")>
5            <cfif compareNoCase( arguments.Exception.RootCause.Type, "coldfusion.runtime.AbortException") >
6                <cfoutput>
7                <CFIF isDebugMode() is false>
8                <cfinclude template="/errortemplate.htm"/>
9                <CFELSE>
10                <h2>An unexpected error has occurred.</h2>
11                <p>Error event: #arguments.eventName#</p>
12                <p>Error details:</p>
13                <cfdump var="#arguments.exception#"/>
14                </CFIF>
15                <cflog file="#this.name#" type="error" text="Event name: #arguments.eventName#"/>
16                <cflog file="#this.name#" type="error" text="Message: #arguments.exception.message#"/>
17                </cfoutput>
18            </cfif>
19        <cfelseif not compareNoCase(arguments.eventName,"onApplicationEnd")>
20            <cflog file="#this.name#" type="Information" text="Application #this.name# ended."/>
21            <cflog file="#this.name#" type="error" text="Event name: #arguments.eventName#"/>
22            <cflog file="#this.name#" type="error" text="Message: #arguments.exception.message#"/>
23        </cfif>
24</cffunction>

Summary

Ok, answer to yourself the following questions:

  • Is the code right?
  • Do you know the purpose of the code?
  • Is it easy to understand?

The first question is a bit of a slippery slope. In my book 'Right' and 'Wrong' are determined by whether or not the code works. The code above actually works just fine. Compiled down to Java Bytecode, it happily processes errors with no defects, so it is 'Right'. When dealing with fuzzy subjects like code-cleanliness and proper organization, you'll find many more opinions than hard/fast rules. Clean Code is like Good Art, you know what you like when you see it. So stay away from calling other developers' code 'Wrong'. It only makes people defensive and defensive minds are not in learning mode.

Now that we've gotten past that, did you understand the purpose of the code? If you said "this code handles errors", give yourself 2 points. (The function name helped, didn't it?)

Was the code easy to understand? Can you verbalize what the code is doing? Try... no seriously, read this code from top to bottom. Can you do it? Did you have to reread parts of it?

It is a known fact that code we write personally instintively makes more sense to us than code other people write. This holds true even months after the code has been written. Good programmers write code that can be easily be understood by others. We are going to refactor this code to be more easily read.

First Analysis

A few easy things to point out... compare() and compareNoCase() are two functions that work opposite of how we read. If you use compare() to compare two identical strings, you get a return of 0, which ColdFusion treats as false, meaning we have to reverse our boolean logic in conditionals to accomodate this. Some people will tell you that compare() is faster than using an operator like IS or IS NOT. They might be right, I'm going to refactor the use of compare() and compareNoCase() out of the code anyways. If it turns out we needed the extra 'performance' of these functions, we should have implemented this part of the site in Hardware.

Next, the case is mixed inappropriately. Some operators are lower case, some are mixed case. Some of the conditionals are upper case, some are lower case. We'll normalize this and get it all looking consistant.

Also, this code has nested conditionals. Some times the only way to represent logical flow is to nest conditionals though most often it can be simplified for better readability. In this case, it looks like we truly care about a few conditions so I'll look at untangling the nest as well.

I'll put a few comments in as well, since that will help us remember what we were thinking in the middle of the refactor. Without further preamble...

First Refactor

view plain print about
1<cffunction name="onError" output="true">
2    <cfargument name="exception" required="true"/>
3    <cfargument name="eventName" required="true"/>
4        
5        <!--- Exit conditions --->
6        <cfif arguments.Exception.RootCause.Type IS "coldfusion.runtime.AbortException">
7            <cfreturn />
8        </cfif>
9        
10        <cfif trim(arguments.eventName) IS "onSessionEnd">
11            <cfreturn />
12        </cfif>
13        <!--- Log and Exit --->
14        <cfif trim(arguments.eventName) IS "onApplicationEnd">
15            <cflog file="#this.name#" type="Information" text="Application #this.name# ended."/>
16            <cflog file="#this.name#" type="error" text="Event name: #arguments.eventName#"/>
17            <cflog file="#this.name#" type="error" text="Message: #arguments.exception.message#"/>
18            <cfreturn />
19        </cfif>
20        <!--- use error template file or print to screen --->
21        <cfoutput>
22            <cfif isDebugMode() is false>
23                <cfinclude template="/errortemplate.htm"/>
24            <cfelse>
25                <h2>An unexpected error has occurred.</h2>
26                <p>Error event: #arguments.eventName#</p>
27                <p>Error details:</p>
28                <cfdump var="#arguments.exception#"/>
29            </cfif>
30            <cflog file="#this.name#" type="error" text="Event name: #arguments.eventName#"/>
31            <cflog file="#this.name#" type="error" text="Message: #arguments.exception.message#"/>
32        </cfoutput>
33</cffunction>

Second Analysis

Ok, That is a pretty good start. We got rid of compare() and compareNoCase() so the code reads a little nicer. We got rid of the nested conditionals so we don't have to keep track of several layers of boolean logic and we cleaned up the inconsistent use of case.

Another thing that I try to stay away from is using negative boolean logic. See where we check for the isDebugMode() function? I'd much rather have the TRUE case up top and have the negative case be the ELSE portion.

Also, some of the logging is duplicated. Looks like someone was Copy/Pasting :). Let's clean that up as well.

Lastly, we are going to put in good comments all the way down the line and use appropriate whitespace for readability.

Second Refactor

view plain print about
1<cffunction name="onError" output="true">
2    <cfargument name="exception" required="true"/>
3    <cfargument name="eventName" required="true"/>
4    
5    <!--- We don't want to deal with the cflocation as an error, so bail --->
6        <cfif arguments.Exception.RootCause.Type IS "coldfusion.runtime.AbortException">
7            <cfreturn />
8        </cfif>
9    <!--- Lets deal with events next. --->
10        <!--- We don't want to deal with the onSessionEnd as an error, so bail --->
11        <cfif trim(arguments.eventName) IS "onSessionEnd">
12            <cfreturn />
13        </cfif>
14        
15        <!--- From this point onward, we want to log the errors --->
16        <cflog file="#this.name#" type="error" text="Event name: #arguments.eventName#"/>
17        <cflog file="#this.name#" type="error" text="Message: #arguments.exception.message#"/>
18
19        <!--- Log as Information and Bail --->
20        <cfif trim(arguments.eventName) IS "onApplicationEnd">
21            <cflog file="#this.name#" type="Information" text="Application #this.name# ended."/>
22            <cfreturn />
23        </cfif>
24    <!--- Now, let's deal with explicit types we want to handle... --->
25        <!--- If we get here, we have something to handle --->
26        <cfif isDebugMode() is true>
27            <!--- print the error to the screen --->
28            <cfoutput>
29                <h2>An unexpected error has occurred.</h2>
30                <p>Error event: #arguments.eventName#</p>
31                <p>Error details:</p>
32                <cfdump var="#arguments.exception#"/>
33            </cfoutput>
34        <cfelse>
35            <!--- use the nice error handler --->
36            <cfinclude template="/errortemplate.htm"/>
37        </cfif>
38</cffunction>

Final Analysis

That looks even better. Now that our conditionals use positive boolean logic they are much easier to read. We also were able to remove duplicate logging calls. Not all code duplication is bad, of course, but when we can get rid of it and improve the clarity and readability, we all win.

The comments are also very descriptive and tell us the intent and reasons for the code. This way, other developers can quickly understand what we were trying to do, even if it isn't working properly.

Just for fun, I'm going to paste the finished code below without comments and whitespace. Read through the code and notice how it reads more like English language.

Final Code - No Comments/No Whitespace

view plain print about
1<cffunction name="onError" output="true">
2    <cfargument name="exception" required="true"/>
3    <cfargument name="eventName" required="true"/>
4        <cfif arguments.Exception.RootCause.Type IS "coldfusion.runtime.AbortException">
5            <cfreturn />
6        </cfif>
7        <cfif trim(arguments.eventName) IS "onSessionEnd">
8            <cfreturn />
9        </cfif>
10        <cflog file="#this.name#" type="error" text="Event name: #arguments.eventName#"/>
11        <cflog file="#this.name#" type="error" text="Message: #arguments.exception.message#"/>
12        <cfif trim(arguments.eventName) IS "onApplicationEnd">
13            <cflog file="#this.name#" type="Information" text="Application #this.name# ended."/>
14            <cfreturn />
15        </cfif>
16        <cfif isDebugMode() is true>
17            <cfoutput>
18                <h2>An unexpected error has occurred.</h2>
19                <p>Error event: #arguments.eventName#</p>
20                <p>Error details:</p>
21                <cfdump var="#arguments.exception#"/>
22            </cfoutput>
23        <cfelse>
24            <cfinclude template="/errortemplate.htm"/>
25        </cfif>
26</cffunction>

Not to shabby, yeah? The code works just like the original sample, but it is much easier to understand the flow of the function. Also, by doing our refactoring in sweeps, we simplified the code in stages, reducing the chance we'll miss something. Now go to the last sample and read it aloud. Notice how it feels natural when you read it? This code will definitely be easier to maintain in the future, won't it? There are lots of other principles to keep in mind when refactoring. We'll look at a few more in the next article.

Agree? Disagree? Tell me all about it in the comments.

P.S. Do you have code you'd like to see refactored?

If you have a code sample you would like me to refactor, send it to me using the Contact Form on this blog. Please try to keep the code chunk below 100 lines as that seems to be the limit of clarity for a blog article.