Wednesday, June 3, 2009

Hating Try Catch Block (Successful way of cleaning code)

The title of this post is a bit weird, it is weird in the sense that it promotes hatred for try catch blocks in our code. I am not a try and catch block hatter, but too much of it will lead to code rots and very, very buggy software.

Did i just say buggy, not only will it make it difficult to maintain but very inexperienced.

In our grained experience in developing object oriented components/systems, we have met with several challenges that tells us that OO (Object Orientation) is not complete and it is not a silver bullet and answer for all. Well for the OO enthusiastic, permit me to say OO has its own bad practices and so therefore it is not complete in its own rigth (No wonder we have several design patterns all over).

Anyway, the purpose of this post is not about OO not beign complete particularly, but about a practices that can lead to very hard to read code, a difficult and un-maintainable codebase. The following contains short list of code demons :
  • Too much IF ELSE statements : This is a killer code beast that if not properly used, you end up with a code that no one will understand.
  • Separation of concern : The statement 'Separation of Concern' does not apply to components/libraries/.dlls alone. It also applies to classes, methods, properties, interfaces. Any code elements is there for a purpose (Concern). If you write a class and you have several methods within the class (even utility methods like GetDate()) , your class does not have a concern. This allows your library, componets, dlls not to conform to concerns from class levels. I will rather create a class that does one thing and do it very well, instead of creating one class that does many things. A dog is a dog and not dogcat. If your class is a dog, let it be a dog, if its a cat let it be a cat.
  • The evil of all TRY CATCH BLOCKS : Yeap too much of this leads to code rot and it does not encourage readability of codebase. Since this is the subject of this article, i will start in a separate paragraph.
The Problem Statement
Consider the following code snippets.


public delegate void SafeMethodCall();

public void DoSomeBadThing()
{
try
{
//Do bad thing
Thrower();

try
{
//Really Do some wacky thing
Thrower();
}
catch (Exception ex)
{

}
}
catch (Exception ex)
{

}
}

public void Thrower()
{
throw new InvalidOperationException("Exception message");
}


Some people will not see any bad in the above code, let us assume we are doing the same in several places. This will lead to very messy code base, i rather allow the application to fail and log the error than catching different exceptions in several places. The only way out of this situation is maintaining at least one try catch block at the application entry points. That is if you have a class that kick starts your entire application, or its an abstraction of a framework, this is the most suitable place to hook the try and catch.

An efficient approach :


public delegate void SafeMethodCall();

public void Thrower()
{
throw new InvalidOperationException("Exception message");
}

public void SomethingNeat()
{
CallSafely(Thrower);
}

public void CallSafely(SafeMethodCall methodCall)
{
try
{
methodCall.Invoke();
}
catch (Exception x)
{

}
}


The delegate can be used by several other methods to do safe call. This means your try and catch is maintained in a single location. This approach is also efficient for logging, if you are logging errors to file or different repositories.

Now, how does this approach save cleanse our code base

Let us assume we have three different parameterless methods (because our delegate is parameterless) :

public void GetName(); //Can throw string format exception
public void GetAge(); // Can throw invalid cast exception
public void GetCountry(); //Can throw country not in world map exception

These methods have a potential of throwing these errors, how can we call these methods using the above approach, instead of defining try and catch everywhere in our code base. we will now do like so :

CallSafely(GetName);
CallSafely(GetAge);
CallSafely(GetName);

For me, the following is more cleaner, than having many try catch handlers to handle the exceptional situations.

Let me know what you think.

1 comment:

Sachin Bhatt said...

We should not be handling exceptions unless we plan to do something specific with a particular exception type ( like probably request a different service ).

To perform common operations like logging, using AOP ( as you yourself have suggested ) would be a more maintainable solution