In the previous post How to log exceptions I said that you should let your logging framework handle exception logging. In this post I will explain why it is a bad idea to generate log messages in the catch block.

Lets first look at an example:

catch (SqlException e)
    var sb = new StringBuilder();
    sb.AppendFormat("Sql number: {0}", e.Number);
    sb.AppendFormat("Sql procedure: {0}", e.Procedure);
    sb.AppendFormat("Sql server: {0}", e.Server);

As you can see, the main code invokes method UpdateAccount, which we can assume will perform update in the database. In the catch block error message is constructed with exception details and logged.

The first thing which you can easily notice is that exception handling block is much bigger than the main code. When analysing this code the user will have to filter out big chunk of exception formatting.

Next, because message formatting is embedded in the method, it will have to be duplicated across code base wherever SqlException is handled. The more places it is duplicated the harder it is to update.

And finally – it breaks Single Responsibility Principle. As defined by Uncle Bob:

A class should have one, and only one, reason to change.

In the above example, the class will have at least three reasons to change:

  1. the change in handling account update (this is the primary reason for this class)
  2. change in error message formatting, including adding/removing details
  3. change in error types handled, such as adding new error type to be logged

This is problematic because adding new exception property to be logged requires re-running all tests related to this call, including regression test. Suddenly a simple change is no longer simple.

The error message formatting logic belongs to the logging framework. Use type renderer for formatting message according to your needs. To handle logging new exception types you only need to create new type renderer and register it with logging framework. You can even reuse type renderers between different applications.
And the catch block will shrinks to one line of code:

logger.Error("Updating user account failed.", e);