Monday, 18 January 2010

Custom exceptions and old chestnuts

During my years in the freelance wilderness I saw a lot of other people’s code. I started to empathise with the plumbers who would prefix their summary of findings with an intake of breath and the stock phrase “looks like you’ve had cowboys in here mate”. That’s not to say I am the supreme best programmer in the universe – far from it – in fact, it would be accurate to say that most of the OPC I saw was written by far better programmers (real programmers even). What I didn’t quite get is that they would all follow the same anti-patterns, one of which was around exception handling.

The basic exception anti-pattern is to create a custom exception, then rethrow all exceptions by wrapping them up in this custom exception. For example:

try
{
  DoSomeVeryDodgyMethod();
}
catch (Exception ex)
{
  throw New MyCustomException(ex);
}
The usual practice is for the custom exception to have some form of logging or user notification built in:
public class MyCustomException
{
  public MyCustomException(Exception ex)
  {
      Trace.WriteLine(ex.ToString());
      MessageBox.Show("An exception has occurred!!");
  }

  // rest of class here
}
This misses the whole point of exceptions, which is to encourage defensive coding. Exceptions are either avoidable or unavoidable and we should code appropriately. This is not the obvious sounding statement you might think. To illustrate an unavoidable exception:
using (SqlConnection connection = new SqlConnection(connectionString))
{
  try
  {
      connection.Open();
      MessageBox.Show("Connection opened");
  }
  Catch (SqlException sqlException)
  {
      Trace.WriteLine(sqlException.ToString());
      MessageBox.Show("Unable to connect to database server\n
          Check your network and try again");
  }

  try
  {
      DoSomethingDatabase(connection);
  }
  Catch (SqlException sqlException)
  {
      Trace.WriteLine(sqlException.ToString());
      MessageBox.Show("Error executing Sql");
  }

  DoSomethingElse();
}
Note how the exception is handled. As a client developer I know that there are a set of conditions beyond the control of my application that can result in a connection failure. These include problems with:
  • Network
  • Security
  • Configuration
I’ve created separate try blocks in order to isolate the problem – anything going wrong in the the first block is connection related, whereas I cannot be so sure in the second block. But the point here is that no matter how well I write the code, this exception could still happen. Deciding what to do about it is an issue for my use case. Of course if the Open() method of the SqlConnection object returned a Boolean value to indicate success then we could eschew the exception – but a connection failure is an exceptional occurrence in as much as we don’t intend for it to happen. It is outside the scope of the core business logic underlying our code. So let’s look at an avoidable exception:
try
{
  int testValue = 15 / 0;
}
Catch (DivideByZeroException ex)
{
  Trace.WriteLine("Doh!");
}
The precondition for the DivideByZeroException is that a violation of mathematical logic must occur. The exception exists so that the client developer remembers to create a use case that avoids such situations - if your application ever throws this exception then you’ve written it badly. To compare it with the last example, it’s much easier to test whether a divide-by-zero is about to happen than if a connection is likely to fail. This leads me to my final point. Going back to the first example, note that we caught the base Exception class - this is very lazy coding. An application should be tested adequately and certainly not be designed to throw unexpected exceptions. The product documentation for the .net Framework Class Library indicates (for each class) what exceptions can be thrown and why they will be thrown. WCF faults take this a step further and actually require that expected exceptions are published in the metadata, otherwise they end up as a FaultException or CommunicationException on the client side. Likewise your code should only throw exceptions to stop other developers from using it incorrectly or to highlight exceptional situations.

No comments:

Post a Comment