Don't hide behind a catch block

"Pragmatic Programmer tip #32: Crash Early."

I've been recently tasked with some refactoring items. Like any project that's longer than 8 weeks, you'll dig into some code and find yourself going, "What was I thinking with THIS?" Or, if the project's a bit longer, you wonder if you came up with this gem (and no, not a Ruby gem) or if somebody else on the team left this odor in the code.

The code smell of this particular blog post is the abuse of the try/catch block. I've stumbled across a few uses of the try/catch programatically, or worse, using an empty catch block to hide an issue you're not sure how to deal with.

Right off the top will go with the empty catch block. I encountered a few of these while refactoring, and had to wonder why we were hiding behind the empty catch block. Crash early, but when you crash sweep that under the rug and move on? That's not good. If you need a catch block, then isn't there something you should be catching? Take the following for an example:

That's a pretty simple little conversion function, with the dreaded empty catch block in there. It's there not because of the TryParse, which is going to give you a false if it fails its own try, but because applying ToString to a null object is going to throw an object error. Rather than the empty catch, why not a null check on o? Then you know what you're dealing with rather than not caring and returning 0 for anything you're not expecting. To me, this is a classic garbage in, garbage out scenario. You pass in a null or an object that can't be converted to a an int and you get a 0.

Next up was a bit of sheer genius, and throwing my hands up when dealing with nHibernate. Rather than find out what was going on, I resorted to using a try/catch procedurally to handle the issue. Here's a bit of code that shows my brilliant solution:

The issue there turned out to be that we were using the load method from nHibernate rather than the get, so I didn't have null object to check. I had an actual proxy object full of null properties. My solution, duck behind the catch block and make it bend to my will. If the load found something, the id check would pass and I'm doing an update. If the resulting object error from the property check threw, then I'm dealing with a new object, time for an insert.

In this case my ignorance of nHibernate's dealing with objects was the problem, but ignorance is no excuse. Using the try/catch in this manner was a hack with Paul Bunyan's ax. It got me moving foward, but was a bad solution.

The final empty catch falls in the same lines, where ignorance of what was available to us wasn't found until much later. This empty catch block example was saving us from a null reference error in a nullable data type in a data bind method.

Getting rid of this empty catch may have been the easiest of the three, because nullable data types carry a "HasValue" property that tells you, well, if they have a value. The refactored code looked like:

Using the HasValue property, we can refactor this one down to the always elegant ternary operator. We know what value we have or don't have and can cleanly bind our data to what's in this case a grid column.

If you're going to fail early, don't hide the failure. Figure out what it is and write the correct code for it. The try/catch is there to help you handle exceptions, not to hide them and move on.

Does Your Project Have a Junk Drawer?

Everybody's kitchen has a junk drawer. You know, the drawer where the tape, scissors, small jar of screws, various measuring items, and those clacker balls on a string that your mom is hiding from you. (Well, the ones my mom was hiding from me.) If you've got something in the house, and you're not sure where it goes, you fire it into the junk drawer. Oddly enough, you also find many useful items by looking in the junk drawer first when tackling something around the house.

However, I've found on a few recent projects that we're falling into the Junk Drawer model a bit. Look around your project, do you have file named "Utilities?" Maybe one called "GlobalMethods?" Or the very popular "ProjectHelper?" If so, there's a good chance you're throwing all kinds of things into the junk drawer. Could be a static class library, a little something in the App_Code directory in an ASP.Net project, or even a javascript file. Or, better yet, all three!

Now, don't misunderstand me, utility classes can be helpful. If you've got a series of string manipulations you need for a project, then a static stringUtils file makes perfect sense. Maybe a constants file for things that don't fit in an enum. As long as they are well defined and don't creep outside the scope of what you're trying to accomplish, I think they can be very helpful.

But, the junk drawer file that contains string manipulations, constants, get user name methods, some validation methods, a few odd conversion methods, a couple more string methods, and a few methods only called from your test project is not much of an asset to your project.

From a usage aspect, how do you tell other team members where your constants live? If they're scattered about your utility file, suddenly you're going to the same file for some string methods and your constants representing a product group. Or worse, your team gets so used to hitting your helper file that everything starts collecting there. Before you know it, you're looking at a maintenance nightmare with a couple thousand lines of code that should live in a number of more logical places.

Look around your projects, if you find a junk drawer take the time to refactor that thing out of there. Get all the files where they belong. You could end up with a utility file or two, but most likely you'll find you can move a number of methods right to the class that's using them because it's the only class that's using them. Turns out our friend YAGNI contributes many "global utility" functions to our projects.

Keep the junk drawer in your kitchen, though. Where else are you going to keep your SpongeBob commemorative Krusty Krab stamp?

How an AJAX guy would pack

At the request of (soon to be former) boss, Brian H. Prince, I've been tasked with explaining how an AJAX guy would pack. Even though this AJAX guy isn't involved in the moving because he's been off site for almost a year, I'll play along...

The first problem an AJAX guy is faced with when preparing to pack are the vast array of box options available to him. All the boxes appear to be roughly the same size, they are just slightly different colors of brown. There is one box that appears to be bigger, so it will hold more for the big move, but upon opening the box you see that a lot of space inside is taken up by an inner, un-openable box with "Update Panel" stamped on it. So, in the end, all the boxes hold roughly the same amount of stuff.

So, after picking a box, AJAX Packing Guy (or Gal) - henceforth referred to as APG - gets down to improving the packing user experience. APG opens up three boxes, puts one item in each box, then goes about cleaning their desk. At some point during the desk cleaning they notice that all three boxes are now full. They seal those up, and open three more boxes, then back to cleaning the desk. Oh, look at that...those three are now full, too!

Once all the items are packed away, APG uses their powers to make each box shrink and fade at the same time...only to reverse the process at the new site and have the packed boxes reappear by fading back in and growing back to normal size.

What happened to the APG who picked the larger box, you wonder? Well, the "Update Panel" carton inside looked to be very helpful at first, but once APG opened more than once box and set them to working, the update panels started running into each other, and reopening packed boxes, and taking apart boxes that had been put together. Eventually they got everything packed, and APG had to do very little in the way of opening boxes, but it took a while.