This is my old posting posted at planetsourcecode.com which I am re-posting on my own blog...
It may seem like you are doing good putting try/catch blocks all throughout your code, but you are probably being redundant. When you declare a try/catch block in Java you cause the compiler to create “protected zones” of execution which require the runtime to do boundary checks and create further processing for the system. You are not making the program more stable by doing this, you are making it many times slower!
If you have a try/catch block which has only one line of code inside the try { … } you are probably being inefficient.
Here is an example of a bad piece of code:
Example 1a:
public HashTable doEvent(Event event) throws CBException {
Connection conn = null;
EntityClass entClass = null;
ArrayList list = null;
try {
try {
conn = dataAdapter.getConnection(DataSource.PRIMARY);
}
catch (Exception e) {
System.out.println(“[SomeClass] An error occurred obtaining connection: “ + e.getMessage());
}
try {
entClass = new EntityClass();
list = entClass.list(conn);
}
catch (Exception e) {
System.out.println(“[SomeClass] An error occurred obtaining data from the Entity: “ + e.getMessage());
}
try {
result.put(“stuff”, list);
}
catch (Exception e) {
System.out.println(“[SomeClass] An error occurred putting data into HashTable: ” + e.getMessage());
}
}
catch (Exception e {
System.out.println(“[SomeClass] An error occurred in the method: “ + e.getMessage());
}
finally {
dataAdapter.releaseConnection(conn);
}
}
The above example is a terrible piece of code. It is inherently inefficient, represents poor error handling (despite the try/catch blocks) and is very ugly to look at to say the least. For one, doing a boundary check on dataAdapter.getConnection(DataSource.PRIMARY) is pointless, why would you create a unique boundary for this. This represents poor thinking. Consider the following example of much more “cleaned up code”.
Example 1b:
public HashTable doEvent(Event event) {
Connection conn = null;
EntityClass entClass = null;
ArrayList list = null;
try {
conn = dataAdapter.getConnection(DataSource.PRIMARY);
entClass = new EntityClass();
list = entClass.list(conn);
result.put(“stuff”, list);
}
catch (Exception e {
System.out.println(“[SomeClass] An error occurred in the method: “ + e.getMessage());
}
finally {
dataAdapter.releaseConnection(conn);
}
return result;
}
The above method in Example 1b is just as effective as Example 1a at handling exceptions, and its execution time is much faster. Think logically, if for example the dataAdapter.getConnection() method fails, the Exception will still be caught by the one single catch block. Also, you can generally rely on the message coming from the ConnectionPoolManager, and DataAdapter to pass an understandable failure message.
Lets take a look at another bad habit:
Example 2a:
try {
try {
compEntity.fetch(conn, compToolEvent.getComponentId());
ArrayList roles = genRolesEntity.list(conn);
results.put("roles", roles);
results.put("component", compEntity);
}
catch (CBException e) {
throw new CBException(“Error: “ + e.toString());
}
catch (Exception e) {
throw new CBException(“Error: “ + e.toString());
}
… Hidden Code Here …
}
catch (Exception e) {
throw new CBException(“Error: “ + e.toString());
}
First of all, if you don’t see a problem in the above block of code then you don’t understand Exceptions. The above code shows massive redundancy in the error handling. Let’s say for example that compEntity.fetch() throws a CBException, this is what happen at runtime:
1. Line: compEntity.fetch(conn, compToolEvent.getComponentId()) Throws a CBException
2. CBException is CAUGHT by first catch block.
3. A NEW CBException is THROWN.
4. CBException is caught in the Exception catch block at the bottom.
5. A NEW CBException is THROWN.
So what’s the problem you ask? There are three big problems, and they are that: Three CBExceptions are instantiated instead of one! But what does it matter? A lot actually, this type of boundary checking is expensive and foolish. Consider this revised code.
Example 2b:
try {
try {
compEntity.fetch(conn, compToolEvent.getComponentId());
ArrayList roles = genRolesEntity.list(conn);
results.put("roles", roles);
results.put("component", compEntity);
return results;
}
catch (CBException e) {
throw e;
}
catch (Exception e) {
throw e;
}
… Hidden Code Here …
}
catch (CBException e) {
throw e;
}
catch (Exception e) {
throw new CBException(“Error: “ + e.toString());
}
The above example makes much more sense. When we catch a CBException, we just throw the existing CBException instance which is caught again and thrown to the higher level. In this case we only create one Exception object. The above example is once again also inherently redundant anyways, and can be even further compressed into the following example (remembering the discussion earlier):
Example 2c:
try {
compEntity.fetch(conn, compToolEvent.getComponentId());
ArrayList roles = genRolesEntity.list(conn);
results.put("roles", roles);
results.put("component", compEntity);
return results;
… Hidden Code Here …
}
catch (CBException e) {
throw e;
}
catch (Exception e) {
throw new CBException(“Error: “ + e.toString());
}
That’s more like it. But it could still be better, but that can wait until a later article.
Final Words
To re-iterate, don’t use excessive try/catch blocks. Plan more carefully, and also take advantage of stacked catches to encapsulate code rather than creating many protected areas. Performance is a very important.
Posts
