Sunday, 28 September 2008

Keeping code clean - Decorators for Error Handling?

I read a review a few days ago of the new book by Robert C Martin (Uncle Bob) 'Clean Code'. I'm going to order a copy as soon as I clear a couple of books from my current reading list backlog as it sounds interesting and the Uncle Bob blog on Object Mentor is one that I always enjoy to read. One of the things that stuck in my mind from the review was that the 'Do one thing' principle applies even to error handling.

Hmmm, if you see the example in the review (I don't know if its' from the book) then this is shown as not evaluating the exception and acting according to its' type, but rather just logging it. This has been nagging in my mind since I read it. Its' not that I disagree with the example, using exceptions for control of flow has always struck me as being a thoroughly poor practice in every place I've seen it done. It was more the idea that perhaps I shouldn't even have the try-catch block around my code in the first place. Now I know that the where's and when's of exception handling (let alone the how's and why's - any of Kiplings six honest serving men in fact) are a very contested country and I'm certainly not positing what follows as 'the answer', just an idea.

What struck me was that I could use the decorator pattern to implement my error handling. For example I have an interface:

public interface ICandidateRepository
{
Candidate GetCandidate(Guid candidateId);

Candidate RegisterNewCandidate();

void SaveCandidate(Candidate candidate);

List<Candidate> FindCandidates(ICandidateCriteria criteria);

}

I already have a concrete class that implements this interface and uses NHibernate to perform the functions that are described. Instead of wrapping these calls in try-catch blocks I can create a new concrete class that implements this interface and looks like this:

public class CandidateRepositoryExceptionDecorator : ICandidateRepository
{
private readonly ICandidateRepository _nestedCandidateRepository;
private ILogger _logger;

public ILogger Logger
{
get { return _logger ?? NullLogger.Instance; }
set { _logger = value; }
}

public CandidateRepositoryExceptionDecorator(ICandidateRepository candidateRepository)
{
_nestedCandidateRepository = candidateRepository;
}

public Candidate GetCandidate(Guid candidateId)
{
try
{
return _nestedCandidateRepository.GetCandidate(candidateId);
}
catch (Exception ex)
{
_logger.ErrorFormat(ex, "Exception when calling GetCandidate({0}).", candidateId);
throw;
}
}

// ... etc ...

}

I use my DI framework of choice to get the parameters into the constructor and to organise the nesting of my objects. In my case this is with the Windsor container. The components section is left like this:

<components>
<component id="candidateRepositoryErrorHandler" service="Semeosis.Examinations.Model.Interfaces.ICandidateRepository, Semeosis.Examinations.Model.Interfaces" type="Semeosis.Examinations.Model.Repositories.Decorators.CandidateRepositoryExceptionDecorator, Semeosis.Examinations.Model.Repositories.Decorators" />
<component id="candidateRepository" service="Semeosis.Examinations.Model.Interfaces.ICandidateRepository, Semeosis.Examinations.Model.Interfaces" type="Semeosis.Examinations.Model.Repositories.NHibernate.CandidateRepository, Semeosis.Examinations.Model.Repositories.NHibernate" />
</components>

The order in which the components are listed determines the order in which the container chains the objects. See this post by Oren for more info.

The upshot of this is that when I call:

ICandidateRepository repository = container.Resolve<ICandidateRepository>();

I get an instance of my CandidateRepositoryExceptionDecorator and this contains my CandidateRepository class from my NHibernate repositories project.

Now my concerns are separated and I can keep the functions that deal with the creational and persistence concerns separate from the error handling concerns. If in the future I need to change the way that my exception handling is manager, or I need to perform other actions than just logging when an exception is logged then I can very easily extend this code. Also unit testing this is incredibly trivial using mocking, whilst also reducing the number of tests on the functions that actually 'do the work'. Separation of concerns in one layer helps keep things clean and tidy across many other layers of the onion! I guess that's why TDD can have such a beneficial effect on model design - but the benefits can be two way.

1 comment:

Mark H said...

I'm reading Clean Code at the moment and really like the concept of error handling being "one thing". I like your decorator pattern approach to this. It might be hard to persuade a large team to discipline themselves to work this way, but it is great for separation of concerns.