NOthingyoumissed

George Mauer is on the Net

Error Handling and the Message Repackaging Anti-Pattern

I currently have an interesting responsibility at work.  I am functioning as the manager and single-point-of-contact for a team in India working on imporving the codebase for one of our more important ASP.Net sites.  I know what you’re thinkin.  Did I say interesting?  I meant infuriating.  At the very least I should get a good “Lessons Learned” post out of this.

Overall the experience hasn’t been too bad though.  I’ve been managing to maintain a decent rapport with their project manager and team lead, deadlines have been missed, but no more than I secretly expected, and overall they seem like fairly competent guys.  Their work isn’t innovative, Agile, testable, or well factored, but its decent enough.

Except for the exceptions.

Browsing one of their recent commits to our SVN lately (ok, so technically speaking they couldn’t figure out Subversion so they ftp-ed and I did the commit for them) I was shocked to discover the following error-handling anti-pattern repeated 41 times!

try {
  // Do Stuff
}
catch(Exception ex) {
  throw new Exception(ex.Message);
}

Do I get to name it?  Let’s call it the message repackaging anti-pattern.  And in case you’re not already reeling from this, the following is a mildly edited-down version of the letter I sent (Notice the eschewed profanity.  I believe a not insignificant achievement.)


Dear Team,

While looking through some of your committed code I noticed quite a few (a quick search shows 41) places where you have code of the following form.
try {
 // ...
}
catch(Exception ex) {
 throw new Exception(ex.Message);
}
Let me be absolutely clear: This is the worst possible way of handling exceptions!
Let’s go over why:
  1. Not doing anything inside the exception. If nothing happens within the catch block then what purpose does it serve? I have seen people prepare their code like this in anticipation of going back and adding logging at some later date. Although this is not necessarily the best logging solution, it is acceptable when you actually implement the code! However, preparing your method like this in anticipation of some future date does nothing but make the code difficult to read. It’s not like it is that hard to go back later and add a try…catch block; if you’re using a refactoring tool like CodeRush it is literally four keystrokes. 
  2. Catching the base Exception. Not every exception can have something be done about it. The canonical example is an OutOfMemoryException or one implying the database might be down. Is there anything to do in this case other than drop the user to an error page? I would even go so far as to say that most exceptions fall in this category – you just can’t do anything about them.  I strongly encourage you to read Stop Catching Exceptions for a more in depth discussion.
  3. Treating the message as if it were the most important thing. Exceptions usually carry a heck of a lot more information than just their message. They carry their type and a StackTrace and a list of any inner exceptions. Many specializations of the Exception base have even more information specific to that type of error. Out of all that information, the message is arguably the least important – for debugging I would take a StackTrace over a message any day. But by creating an exception off of the message you are effectively stripping out all this useful stuff! If for some reason, you must create a new exception, at least use the constructor overload that includes the original error as an InnerException: new Exception(“I don’t know why I’m creating this”, ex);
  4. Why create a new Exception object at all? You already have a perfectly valid exception object. It is a simple matter to do:
    try {
     // ...
    }
    catch(Exception ex) {
     // Log or otherwise handle the exception
     throw; // Same as throw ex;
    }
So in conclusion
  • Don’t catch an exception if you aren’t going to do anything with it.
  • Try to catch exceptions at the appropriate level and for the appropriate task. (logging inside private methods is probably unnecessary)
  • Do not create a new exception, throw the old one.
Please take this as healthy constructive criticism. I hope that you agree with me on these points and that we can take care of this issue properly from now on.

 


A bit harsh perhaps, but I feel like for all the reasons above the immensity of this mistake cannot be overstated.  If you are doing this in your code STOP NOW and apologize to the maintenance gods!  You might even need to sacrifice a goat to appease them.  It’s that bad.

In any case, the recent commit has only 38 instances of this error.  So at least we’re getting better!

Advertisements

January 16, 2009 Posted by | Programming | , , , | 2 Comments

Oooh! I Wrote Something for Upper Management

Today I was asked to write up a short justification for a rewrite of a godawful piece of software that I have been complaining about to anyone who would listen.  Yay!  Inspired by essays I have been reading by Jim Shore I might have been a bit too agressive.  But here’s the final(?) version:

Proposal for a Rewrite of the {Program Name} Software
The purpose of software is to make our lives easier; it is a tool like a hammer or a screwdriver and far more malleable than either. This is generally a good thing as it allows us to mold it to our specific needs and particular processes. If software is a screwdriver meant to do screwdriver-type activities it should be with a minimal amount of effort that we can get it to work on a phillips or a flat-head screw. Unfortunately even screwdrivers can be poorly made, their handles can disintegrate as we apply too much torque, they can be made to poor standards so they don’t grip the screw quite right and slip frequently, and they can be made out of cheap material so that any attempts to remedy the above only compounds the problems as the shaft bends and warps in unexpected ways. Without taking the metaphor any further, it is my professional opinion that – external appearances aside – our current {Program Name} Software is this sort of unwieldy tool and in need of an urgent re-working.
Current Problems and Future Implications
The current implementation of our {Program Name} software has a multitude of problems. First and foremost, it seems to have been created as a prototype rather than a lasting project. As such, it is not meant to be extended to work in the ways that we need it to work. It is also not possible to use it with automated testing software to check the various contingencies we need it to work under and to ensure that future features do not break already existing ones. Without the ability to write automated self-tests, keeping the system operating across the conditions at the various terminals and feed company locations will continue to become more and more of a burden that will continue depleting our already low development time.
Furthermore, under the current system, when a bug is encountered, it is notoriously difficult to troubleshoot. For a problem at a site, our only recourse is to attempt to recreate it by simulating the conditions from user testimony. This is a painstaking and highly uncertain process. Take as example the multitudes of issues caused by poor network connectivity. The current system has no plan for dealing with short-term outages which, while inexistent when the software is developed in-house, are ever-present in the field. As such, it frequently ends up acting in bizarre ways that get reported infrequently and incompletely. Under these conditions they can be nearly impossible to repair. The software was simply not produced with the idea that it would require in-the-field maintenance and diagnosis; although expedient attempts to remedy this situation have been made, deep architectural flaws severely limit their possibilities.
The bottom line is that behind-the-scenes the {Program Name} is poorly designed and implemented. Much of the inner-workings are extremely confusing and no attempt has been made to consistently use established industry or even the developer’s own standards. There is no documentation anywhere. The problems are deep-rooted enough to need to be addressed immediately. In saying this I want to make it absolutely clear that in my opinion our difficulties arise not from the inescapable need to add features, but from the fact that the scope of the initial design completely omitted considerations of clarity, extensibility and in-the-field testing.
Proposed Changes
I propose a complete redesign of the {Program Name} system with an emphasis on the creation of a system that is simple, easy to maintain, extend, and troubleshoot. We will of course address all of our current needs but we will use established techniques to create software that works well with a model of continuous development. The new system would also be designed to streamline troubleshooting and include automatic self-testing to ensure that it is shipped out with as few bugs as possible. We will achieve this by sticking close to C# .NET best practices making the application truly object oriented, using Agile planning techniques and test-driven development, and optionally integrating with a dependency injection framework to ensure extensibility. While the changes experienced on the user side will be minimal, the payoff on the development and maintenance side will be almost immediate. I fully expect development time under a redesigned system to shrink by at least a factor of four, for new deployments to become seamless instead of taking days to configure properly, and for maintenance time to fall from over 5 hours per week to well under one. As a tentative schedule I would propose the following: Half a week of design (much of it has already been done), one to two weeks of implementation and a week of testing. I believe that by working on this project 80-90% of my time I should be able to finish it and have it ready to go in three to four weeks.

Thank You Very Much,
George Mauer
Software Developer

May 5, 2008 Posted by | Programming | , , | 1 Comment

Let’s try to start this nice and simple

So since I registered this blog name I have proven empirically that I am incapable of actually writing to it so I figure I’ll start simple – with a list.  And – because this is what i’ve been reading and thinking of a lot lately – its going to be programming related.  Weeee.   So here we go.

Technologies that I HAVE to learn:

  • NHibernate – The alt.net list, podcasts, and .NET blogs have been a buzz about this for so long that its barely even mentioned anymore, but yes, through argument and experience I’ve been convinced, dynamically generated SQL is the way to go.  Time to get a crackin’.
  • MbUnit – Apparently that’s what a lot of the pros use and I want to  be a pro right?  Well, I’ve got the basic [TestFixture] – [Test] – [RowTest] stuff down but guys like Phil Haack have to love it for more than just a few neat features and a nice GUI right?  How about reading the documentation?  Also, what the fuck is Gallilo and how come I can’t get it to run?
  • StructureMap or Castle Project – Inversion of Control / Dependency injection frameworks.  This is what you use if you want an extensible application.  And I do.
  • F# – A couple months ago, I realized that I kinda like programming in Javascript. Apparently, I’m not the only person that thinks so.. I guess it was because I was finally starting to get an understanding of what this functional programming thing is all about. And now that that I know, I like it!
  • And in a similar vein – JQuery. – Yeah, I’ve gotten some practice with this amazing toolbox in working with my personal site and The Roots Of Music and I think there’s something there. I want more, a lot more.
  • Git or Mercurial – Distributed Version Control Systems.  These seem to be the new thing in version control, no pressing need to switch from SVN, but I should start getting familiar with them.  It seems like for the time being, Mercurial is the shiznit.  Not sure if any of them have nice TortoiseSVN-style support though.
  • TypeMock – Commercial mocking software.  Type mocking is really just something that I need to learn about as I’ve run into the limits of non-mocking unit tests already.  I’ve heard this product is good in a few places and they have a good learning section the web-site as well as a free download version.  Could be worth trying it out.  As an alternative, we’ve also got Rhino Mocks which is free.

May 3, 2008 Posted by | Music, Programming | , , , | Leave a comment