Exception handling problems in Java, Part II

Author: Kasper B. Graversen, 05/06/08
Keywords: Exception handling, Refactoring, Java
Abstract: Java's type system has a serious flaw. It does not allow refactoring common exception handling code into methods. This article shows the various solutions people have suggested in response to the previous article in this series along with a revised proposal.

subscribe to my RSS feed

Exception handling problems in Java, Part II

This is the sequel to the Exception handling problems in Java, Part I article. If you have not yet read it, consider doing so, as it sets the scene.

Since my previous post in this series, I've had a lot of feedback. Most of which in a positive tone. Many thanks for that! It's always nice with a sober discussion. A number of possible solutions have been blogged or mailed me directly. This article summarizes these solutions and is rounded off with my new proposal of a simple extension to the Java type system to rectify the problem.

The feedback received falls into the following categories (in random order):

Return the exception rather than throwing it
Use nested try-catch blocks
Don't return inside try-catch blocks
Use BGGA closures
Defensively throw an exception
Your solution is not type safe!

Each suggestion/solution will be examined in the following. I hope it'll be a learning experience for everyone. In order minimize the article, I have removed catch-blocks from certain examples.

The problem (summary)

To quickly recapture the problem, Java does not allow one to extract into helper methods, logic that would otherwise reside in catch blocks, without requiring a confusing return null following the blocks. The problem is motivated by live code in the SuperCSV project. exemplified by the below code which tries via reflection to invoke a set method. The code takes autoboxing into consideration, and thus if a method setFoo(int) cannot be found, a setFoo(Integer) may be located.

 Method inspectClassForSetMethods(Object destinationObject, Class variableType, String variableName) {
    String methodName = "set" + variableName.substring(0, 1).toUpperCase() + variableName.substring(1);
    
    try {
        return destinationObject.getClass().getMethod(methodName, variableType);
    }
    catch(SecurityException e) {
        throwException(destinationObject, variableType, methodName, e);
    }
    catch(NoSuchMethodException e) {
        // retry again due to autoboxing in java we need to try both cases
        try {
            if( autoboxingConverter.containsKey(variableType) == false ) {
                throwException(destinationObject, variableType, methodName, e);
            }
            return destinationObject.getClass().getMethod(methodName, autoboxingConverter.get(variableType));
        }
        catch(SecurityException e1) {
            throwException(destinationObject, variableType, methodName, e1);
        }
        catch(NoSuchMethodException e1) {
            throwException(destinationObject, variableType, methodName, e1);
        }
    }
    return null; // Can never be reached.. but the compiler requires it!!
}

/** Confining exception handling code. Must always throw an exception */
private final void throwException(Object destinationObject, Class variableType, String methodName,
 Exception e) throws SuperCSVException {
            e.printStackTrace();
            throw new SuperCSVException(String.format("Can't find method '%s(%s)' in class '%s'. "
                + "Is the name correctly spelled in the NameMapping? "
                + "Have you forgot to convert the data so that a wrong set method is called?", methodName,
                variableType, destinationObject.getClass().getName()), e);
    }
}

Return the exception rather than throwing it

The most frequently suggested solution received, was the suggestion to let the helper methods return the exception rather than throwing it. Publicly available examples can be found on www.dzone.com and Jesse Wilson's blog. The underlying idea is, that when the helper method returns an exception rather than throwing it, the throw clause is always present directly inside the catch-block, rendering the return null unnecessary. This is possible since the exception object is just another object in the system, and correspondingly, can be returned, passed to methods etc. A short version of the code is shown below with changes in bold.

Method inspectClassForSetMethods_returnException(Object destinationObject, Class variableType, String variableName) {
  try {
    return destinationObject.getClass().getMethod(methodName, variableType);
  }
  catch(NoSuchMethodException e) {
    // retry again due to autoboxing in java we need to try both cases
    try {
      if( autoboxingConverter.containsKey(variableType) == false ) {
        return noMethod(destinationObject, variableType, methodName, e);
      }
      return destinationObject.getClass().getMethod(methodName, autoboxingConverter.get(variableType));
    }
    catch(NoSuchMethodException e1) {
      throw noMethod(destinationObject, variableType, methodName, e);
    }
  }    
}

private SuperCSVException noMethod(Object destinationObject, Class variableType, String methodName,
  NoSuchMethodException e) {
  e.printStackTrace();
  return new SuperCSVException(String.format("Can't find method '%s(%s)' in class '%s'. "
      + "Is the name correctly spelled in the NameMapping? "
      + "Have you forgot to convert the data so that a wrong set method is called?", methodName, variableType,
      destinationObject.getClass().getName()), e);
}

Comment: While the suggestion enables the programmer to extract common exception handling code, I think the code is bad for a number of reasons. Firstly, the code may be a bit confusing as the method noMethod returns an object rather than throwing the exception. One can not just perform a extract method refactoring.

The problem is further compounded by the big semantic difference between throwing an exception and returning a value. For example, a return value from a method can be ignored, whilst an uncaugth exception will traverse up the call stack. This is the raison d'être of exceptions! Hence more complicated helpers may accidently return null, which results in a NullPointerException being thrown rather than the SuperCSVException.

Alternatively, the helper method may throw the exception instead of returning it, whilst still declaring in the calling code, that the code will throw an exception. This looks like

Method inspectClassForSetMethods_returnException(Object destinationObject, Class variableType, String variableName) {
    try {
        return destinationObject.getClass().getMethod(methodName, variableType);
    }
    catch(NoSuchMethodException e) {
        // retry again due to autoboxing in java we need to try both cases
        try {
            if( autoboxingConverter.containsKey(variableType) == false ) {
                throw noMethod2(destinationObject, variableType, methodName, e);
            }
            return destinationObject.getClass().getMethod(methodName, autoboxingConverter.get(variableType));
        }
        catch(NoSuchMethodException e1) {
            throw noMethod2(destinationObject, variableType, methodName, e);
        }
    }    
}

private SuperCSVException noMethod2(Object destinationObject, Class variableType, String methodName,
    NoSuchMethodException e) throws SuperCSVException { 
    e.printStackTrace();
    throw  new SuperCSVException(String.format("Can't find method '%s(%s)' in class '%s'. "
        + "Is the name correctly spelled in the NameMapping? "
        + "Have you forgot to convert the data so that a wrong set method is called?", methodName, variableType,
        destinationObject.getClass().getName()), e);
}

Comment: While it may be a bit confusing to declare a throw in both inspectClassForSetMethods_returnException and in noMethod2, the aforementioned problems have been catered for. However, the solutions are not easily grasped by everyone. Blogger lowellk, a reader on http://publicobject.com/2008/05/calling-method-that-always-throws.html notes

I don't get the point of making the method have a non-void return type. It's a lie. It never returns that type, only throws it. And then those 'throw ...' statements in the caller are not actually throwing anything, because it's the argument to the throw which actually throws the exception. It's all quite confusing.

Use nested try-catch blocks

Using nested try-catch blocks is not exactly a solution to the problem, however, it generally helps reducing the number of blocks needed, and often it structures the code such that the business logic and retry logic stands more out. I thus found it interesting for the present discussion. Gareth Boden provided this code as one of his many suggestions. Perhaps the principle is best first described in pseude-code

try {
    try {
        // do scary stuff
        return result;
    } catch (RetryException e) {
        // do other equally scarry stuff
        return result;
    }
} catch (Exception e) {
    if(e instanceof RuntimeException || e instanceof Error) 
        throw e;
        
    throw translatedException(obj, type, name, e);
}

A number of things can further improve the code, such as extracting the try and the retry blocks as suggested above. And If you don't like the supertype Exception catch, you can have multiple catch blocks calling the exception-returning method. Applying this pattern to our code example, the code becomes

Method inspectClassForSetMethods_nested_try_catch(Object destinationObject, Class variableType, String variableName) {
    try {
        try {
            return destinationObject.getClass().getMethod(methodName, variableType);
        }
        catch(NoSuchMethodException e) {
            // retry again due to autoboxing in java we need to try both cases
            try {
                if( autoboxingConverter.containsKey(variableType) == false ) {
                    e.printStackTrace();
                    throw new SuperCSVException(String.format("Can't find method '%s(%s)' in class '%s'. "
                        + "Is the name correctly spelled in the NameMapping? "
                        + "Have you forgot to convert the data so that a wrong set method is called?", methodName,
                        variableType, destinationObject.getClass().getName()), e);
                }
                return destinationObject.getClass().getMethod(methodName, autoboxingConverter.get(variableType));
            }
            catch(NoSuchMethodException e1) {
                e.printStackTrace();
                throw new SuperCSVException(String.format("Can't find method '%s(%s)' in class '%s'. "
                    + "Is the name correctly spelled in the NameMapping? "
                    + "Have you forgot to convert the data so that a wrong set method is called?", methodName,
                    variableType, destinationObject.getClass().getName()), e);
            }
        }
    }
    catch(SecurityException e) {
        e.printStackTrace();
        throw new SuperCSVException(String.format("Can't find method '%s(%s)' in class '%s'. "
            + "Is the name correctly spelled in the NameMapping? "
            + "Have you forgot to convert the data so that a wrong set method is called?", methodName, variableType,
            destinationObject.getClass().getName()), e);
    }
}

Comment: Notice how much clearer the business logic stands out, and how clear the re-try implementation becomes. I appreciate that some people have strong feelings against nested try-catch blocks. And with good reason. If you find yourself wanting to use nested try-catch blocks for resource cleanups, you probably should turn your attention towards the try-catch-finally strategy instead!

Don't return inside try-catch blocks

Another suggestion found on www.dzone.com is Daniel Pietraru's idea of not returtning inside try-catch blocks. He writes "By the way I am not a big fan of returning from a try block.", unfortunately not explaining why. The code now looks like

Method inspectClassForSetMethods_no_return_inside_try_catch(Object destinationObject, Class variableType, String variableName) {
    Method result = null;
    try {
        result = destinationObject.getClass().getMethod(methodName, variableType);
    }
    catch(NoSuchMethodException e) {
        // retry again due to autoboxing in java we need to try both cases
        try {
            if( autoboxingConverter.containsKey(variableType) == false ) {
                noMethod2(destinationObject, variableType, methodName, e);
            }
            result = destinationObject.getClass().getMethod(methodName, autoboxingConverter.get(variableType));
        }
        catch(NoSuchMethodException e1) {
            noMethod2(destinationObject, variableType, methodName, e);
        }
    }
    
    return result;
}

Comment: I think there are good and bad things to make note of. Firstly, I like the approach, as it circumvents the problem by never allowing it to happen. Since return statement is the last statement in the method, the compiler will never complain. The approach is also nice in that we can perform our extract method directly with no additional actions needed, e.g. we do not need any throw statements or other non-sense.

Unfortunately, the suggestion also carries a dark side. A dark side invisible to people stuck in the 1970's and 1980's methodology on only having one exit point in each method. It's far from time to rejoice! In general such a codeing strategy forces upon the programmer to declare all variables at the top of the method and intricate use of guard variables and often deeply nested branching to ensure codeblocks only are executed when needed. I've seen my fair share of such code, and I don't like it... I much prefer the Failing Fast idiom and declaring variables closest possible to where they are being used. More on this some other day though.

Use BGGA closures

Ricky Clarkson and Neal Gafter informed me of Mark Mahieu's blog on Neal Gafter's possible addition to Java 7 - the BGGA closures, which can be typed not to terminate normally. Unfortunately, at present I'm not experienced enough with the BGGA to do more than recite directly from the blog:

Sandwich makeLunch() {
    try {
        return kitchen.makeSandwich();
    }
    catch (KitchenClosedException | NoBreadException ex) {
        handleException(ex, "Couldn't make a sandwich :(");
    }
}

Nothing handleException(Exception ex, String message) throws RuntimeException {
    log.error("Something went wrong", ex);
    throw new RuntimeException(message, ex);
}

Defensively throw an exception

Quite surprisingly, no one has suggested throwing an exception as an alternative to the offending return null; statement. By throwing an exception we satisfy the constraints of the compiler, while at the same time, enable us to establish a defensive guard making the code more resilient to future changes. The pattern is illustrated by the pseudo-code

// do stuff
if( foo ) return bar;
throw new Exception("this can never happen");

If stuff (or something stuff invokes) changes making foo false, me unexpectedly do not return bar. In such a case, the program will fail, but it'll fail with a somewhat sensible error message, this can never happen, which prevents otherwise potentially disastrous unintended execution of code. Applying this pattern of code we get

Method inspectClassForSetMethods_defensive_throw(Object destinationObject, Class variableType, String variableName) {
    try {
        return destinationObject.getClass().getMethod(methodName, variableType);
    }
    catch(NoSuchMethodException e) {
        // retry again due to autoboxing in java we need to try both cases
        try {
            if( autoboxingConverter.containsKey(variableType) == false ) {
                noMethod2(destinationObject, variableType, methodName, e);
            }
            return destinationObject.getClass().getMethod(methodName, autoboxingConverter.get(variableType));
        }
        catch(NoSuchMethodException e1) {
            noMethod2(destinationObject, variableType, methodName, e);
        }
    }
    throw new IllegalStateException("This can never happen.");
}

Comment: The code has all the qualities mentioned in the previous section on Don't return inside try-catch blocks. I quite like the solution, it's straight forward, easy to read and understand and it doesn't suffer from the "only one return statement pr. method" problem. In fact, if you are used to programming defensively using the this can never happen style then this pattern of coding feels natural and stands in beautiful contrast to the eye-shattering, provoking and unnecessary return null;.

Your solution is not type safe!

The final chunk of feedback received falls under the category "Your problem cannot be solved, Your solution is not type safe!". The reasoning behind this statement is sound when regarded in isolation. The previous article was an attempt to define a solution for this problem. Arguably, it did not take the dynamic class loading into account, thus here is my second attempt at defining the semantics here :-)

Yardena describes well the class loading problem

Let's say you place handleException method in another class, and at the time you compile the code that uses it handleException always throws an Exception - you suggest compilation should pass without the need for a return statement. However you may later "refactor" and recompile handleException alone, so that it at least sometimes does not throw an exception. Now if you call handleExcepttion, this is seriously broken at run-time.

To this I have two counter arguments:

Counter argument 1: For private final: The problem with the dynamic loading of a different class with a different implementation cannot occur in my example, as I have carefully crafted it such that the helper method is private final. Correspondingly, no-one is able to fool around with the implementation. The Java compiler should to the extend possible, utilize the knowledge it can obtain from final definitions.. In other words, it should be problem-free to allow the code to work in the next generation of the Java language.

Counter argument 2: For the general case: There is nothing counter-intuitive about my proposal, it would make programming easier, as code you think should work will work. To avoid the dynamic loading problems, the type system must merely be enhanced to denote that the transitive closure of a method m always throws an exception of type TE. This presumably requires some byte codes to represent this fact combined with some extra sanity checking when loading classes. These enhancements should be problem-free (at least for users af the language :-). Presently a lot of checking and verification is taking place during both compilation and class loading. My proposal is a non-intrusive change that does not break backward compatibility (old code will still work under the new compiler).

Conclusion

Many solutions to the problem has been presented. I hope you have been inspired to review your exception handling practices. The main extension to my suggested change in part I, is that the type system be extended with a notion of methods must guarantee to always throw exceptions. This is an extension to the current definition, where a method can denote it may throw an exception (using the throws declaration). Such a guarantee must subsequently be enforce at class load-time.

Comments

If you have any comments to this article, please drop me a mail at firstclassthoughts at gmail dot com please indicate if I can't publish whole or parts of your comment on the site.

If you like this site consider

subscribing to my RSS feed or how about subscribing by Email.

Help spread the word

Share this post on your favorite social bookmarking sites:

If you enjoyed this article, found it thought provoking, educative or otherwise good, please link to this page from your page or social bookmarking page. If you have any texts you think are worth publishing on First Class Thoughts, don't hesitate to send me a mail! Quality always welcome.

The most recent contributions
28/07/09 Magic in mathematics II Fun with the number cyclic numbers, and specifically with 142857 as it is the smallest of such numbers.
13/07/09 My top 8 time-saving Firefox shortcuts This article presents my favorite top 8 time-saving shortcuts in Firefox 3.0 and Firefox 3.5. Get to know these and you'll be saving a lot of time. They have been ordered by "the element of most surprise"
20/05/09 Board Game Jungle speed / Arriba Review of the cool game "Jungle Speed" aka. "Arriba".
16/05/09 Danish Twin words "Twin words" are words that not only have multiple meanings, they must be composed next to each other in meaningful sentences. This article explores the concept of twin words.
Nothing of interest? Try browsing the entire article archive...

First Class Thoughts

First Class Thoughts

Home

Topics

Traps

Blogs

KBG

Article Indexes

Articles by date

Article Keywords

ANT Actions Backup Bat file BigDecimal BookReview Build script C CPP CSV CSharp Cmd file Compile DB DBA Danish Documentation Double ESB Eclipse English Enterprise Service Bus Exception handling Firefox Font Font list Foreach Forwards Free Software Free mp3 Free music Fun Functional Programming GUI Geek movie GeekPoetry Help IEnumerable IEnumerator IQ Test Inheritance JSP Java Java traps trail Javadoc Keyboard Language Linguistics Linux Log4J Logging Logic test Method chaining Numbers Optimization ProgrammingProjectIdeas Project Management ProjectManagement Refactoring SOA SSH SWT Scriptlets Search-replace in XML Servlet Shortcut programming Software Software development Source Forge Spring Statistics Struts Struts Actions Struts Attributes Struts Dynamic attributes. Struts Forwards Testing Tomcat Twin Words Unix Web Service XML XSLT Yield movie review