Saturday, July 6, 2013

Clean Code Question: Exception Handling

Earlier this week, I spoke about Clean Code (Clean Code: Homicidal Maniacs Read Code, Too!) and a couple of questions came up that are good to explore a little further.  In the last article, we talked about properties vs. methods.  This time, we'll think a little more about exception handling.

Exception Handling
This question came up during the refactoring portion of the session.  Here's the code before the refactoring:

Notice that there are two places where exceptions may be thrown.  This will result if the dependency injection container is not initialized properly.

As noted in the session, this is not bad code.  The method is short, and there isn't much complexity.  But it does take a little while to figure out what it's doing.  Our goal is to make this readable, so we refactored out two of the "detail" parts into separate methods.

The updated code looks like this:

Now the original "Initialize" method is easier to follow.  It performs 3 actions, and if we need to drill into the details, we can.  But if we don't need the details, we still have a good idea of what this method is doing.

This leads us to the question:
Where does the try/catch block go?
Since we know that this method could throw an exception, shouldn't we have this wrapped in a try/catch block?  And should we have the try/catch in the original "Initialize" method?  Or should it be in the detail methods?

A Global Exception Handler
So, the answer I gave is that we don't catch exceptions here, we have a global exception handler that takes care of that.  For this application (which is based on a production application), the scenario is that if we have a problem on the client-side code, there really isn't much that we can do about it other than log it and reset.

This was just the first implementation of error handling for this application.  If a workflow failed partway through, it would generally be due to a server error.  The client application really doesn't do much beyond display, basic data validation, and submitting to the server.  If there is an error, the application would alert the user that something went wrong, log the error to the logging service, and reset the user application.  This could result in lost (unsubmitted) data, but this was considered acceptable to the business.  If it became a problem, then a more robust solution with client-side storage would be put into place.

Here's an example of a global exception handler.  This is in the "App.xaml.cs" file in the WPF sample:

So, we can see that our sample application would just shut down (and this is highly simplified for this sample).  But this leads us to an important part of exception handling:
Only catch an exception if there's something you can do with it.
Let's explore this a little further.

Catching Exceptions
Back to the original question: should we have a try/catch block in our high-level "Initialize" method, or should we have the try/catch blocks in the detail methods?

The answer is neither.

In this instance, an exception will get thrown if the dependency injection container is not initialized with the required objects.  If the service (IPersonService) is not configured in the container, it will throw an error.  If the model (CatalogOrder) is not configured in the container, it will throw an error.

If the Dependency Injection container is not populated with the right objects, then what can our class (a View Model) do about it?  The answer is nothing.  Our class relies on having a correctly configured DI container.  Someone else is responsible for making sure that happens.

If we caught the exception, what would we do with it?  In this case, there really isn't much we could do.  We could alert the user that there is a problem, and we could log the error, but that's about it.  We can't "recover" from this problem; our View Model will not work.  So, we don't gain anything by trying to handle the error here versus letting the global exception handler take care of things.

We've seen how we should not catch an exception if we can't do anything with it.  But there are times when we want to try again.  For example, if we are making a service call, we may get a TimeoutException.

In that scenario, we could catch the TimeoutException locally, wait a few seconds, and then try the service call again.  If we fail again (or maybe after a few more tries), then we will simply let the exception propagate up to our global exception handler.

Wrap Up
Exception handling is a big topic.  There are a lot of different approaches.  Our primary focus is that our application behaves appropriately.  There may be times that the risk of data corruption is high and we want to simply abandon all of our uncommitted changes.  There may be times that our application is left in an inconsistent state, and we need to try to reset it somehow.

And there may be times when we can handle the exception by trying again or by alerting the user of a change that he needs to make in order for the transaction to succeed.

What our simple sample shows is that we don't always need to handle exceptions locally.  There are times where it is appropriate to let an exception propagate up to the next level.

Exceptions in .NET are an example of the Chain of Responsibility pattern.  For more information, check out "Learn the Lingo: Design Patterns" that talks specifically about Exceptions and the Chain of Responsibility.  We just need to make sure that our exceptions don't drop off the end of the chain.  Something needs to be there to pick up any unhandled exceptions.

Happy Coding!

No comments:

Post a Comment