Saturday, February 13, 2010

Factory method pattern for Java exceptions

I'm not sure if this is a new idea. I did a brief survey and I didn't find anything like it out there. I'm going to get right to the point, and then incrementally explain the rationale of my approach.
void before(int index, int lo, int hiX) {
   try {
      throw new IndexOutOfBoundsException(
         String.format("%d out of [%d,%d)", index, lo, hiX)
      );
   } catch (IndexOutOfBoundsException e) {
      throw (NoSuchElementException)
         new NoSuchElementException().initCause(e);
   }
}

void after(int index, int lo, int hiX) {
   try {
      throw Exceptions.indexOutOfBounds(index, lo, hiX);
   } catch (IndexOutOfBoundsException e) {
      Exceptions.translate(e, Exceptions.noSuchElement());
   }
}

What are index, lo, and hiX?

They are an index and the lower & upper bounds for some sort of collection. Upper bound parameters in Java APIs are almost always exclusive; the suffix "X" literally makes this explicit. As formal parameters, this ordering is consistent with the order in which the words "index" and "bounds" appear in e.g. "IndexOutOfBoundsException".
What are you doing in line 4?
String.format("%d out of [%d,%d)", index, lo, hiX)
I will answer by quoting Joshua Bloch in Effective Java [EffJ]:
Include failure-capture information in detail messages
For example, the detail message of an IndexOutOfBoundsException should contain the lower bound, the upper bound, and the index value that failed to lie between the bounds.
Like some exceptions, IndexOutOfBoundsExceptions only has 2 constructors, () and (String msg). [EffJ] argues the need for an (int lowerBound, int upperBound, int index) constructor, and even presents the full code for such hypothetical constructor.
Unfortunately, the Java platform libraries do not make heavy use of this idiom, but it is highly recommended. It makes it easy for the programmer throwing an exception to capture the failure. In fact, it makes it hard for the programmer not to capture the failure! In effect, the idiom centralizes the code to generate a high-quality detail message for an exception in the exception class itself, rather than requiring each user of the class to generate the detail messages redundantly.
Fact: the following snippet appears 3 times in the source code for java.util.ArrayList (v1.56).
throw new IndexOutOfBoundsException(
   "Index: " + index + ", Size: " + size);

What are you doing in lines 7-8?
throw (NoSuchElementException)
   new NoSuchElementException().initCause(e);
I will answer with another quote from [EffJ]:
Throw exceptions appropriate to the abstraction
A special form of exception translation called exception chaining is appropriate in cases where the lower-level exception might be helpful to someone debugging the problem that caused the higher-level exception. [...] Most standard exceptions have chaining-aware constructors. For exceptions that don't, you can set the cause using Throwable's initCause method.
The following is excerpted from the documentation for java.lang.Throwable:
/*
 * Prescribed usage idiom for
 *   public Throwable initCause(Throwable cause)
 */

try {
   lowLevelOp();
} catch (LowLevelException le) {
   throw (HighLevelException)
      new HighLevelException().initCause(le);
}
Two things need to be mentioned here:
  • The return type of the method is Throwable. This means that unless you declare that your method throws Throwable (which needless to say is a really bad idea), you'd have to cast it to the original type of the just-constructed exception. This is annoyingly redundant at best...
  • ... and unnecessarily fragile at worst. It's true that Throwable.initCause is documented to @return this, but since it's not final, there's no guarantee that a subclass couldn't @Override it with an entirely different behavior that would break the prescribed idiom.
Fact: in the source code for java.util.AbstractList (v1.52), a caught lower-level IndexOutOfBoundsException is translated to a NoSuchElementException or a ConcurrentModificationException in 5 different places. In all of them, the cause is lost in the translation. In the absence of convenient chaining-aware constructors, and perhaps avoiding the hassle that is initCause, only the nullary constructors are used. Any IndexOutOfBoundsException that was constructed with failure-capturing detail messages are simply thrown away.

When you consider that AbstractList is designed for inheritance, it's easy to argue that perhaps it should chain all lower-level exceptions to facilitate debugging.

What are you doing in line 14?
throw Exceptions.indexOutOfBounds(index, lo, hiX);
It's hopefully evident now that at least some exception classes could benefit from constructors that take failure-capturing information in their native types. Instead of wishing that things were implemented differently, however, one can better solve the problem by following another good advice from [EffJ]:
Consider static factory method instead of constructors
[An] advantage of static factory methods is that, unlike constructors, they can return an object of any subtype of their return type.
These are some of the benefits offered by this approach:
  • Instead of being stuck wishing that richer constructors are available for existing exceptions, we can always move forward by adding more methods to our factory.
  • Clients should rarely have to build exception message details themselves; now that clients can provide all failure-capturing information in their native types, most eventual String-building can be done by the factory class.
    • Clients are spared from writing redundant code all over the place.
    • Conformance to any desired standard format is easier to enforce.
    • Delegating String-building to the factory class also facilitates easier internationalization/localization efforts, etc.
  • Whenever applicable, we can take advantage of the flexibility offered by factory methods and return a subtype of the requested exception type instead.
    • The hypothetical scenario presented in [EffJ] for IndexOutOfBoundsException provides not only the (int index, int lo, int hiX) constructor (with different names and in different order), but also accessors for them.
    • With the factory method, we can return an instance of a subclass that provides these accessors. For clients who need them, they can cast the exception to the the desired type; for everyone else, it's still business as usual.
    • These subclasses can also implement any additional interfaces if we want/need them to. In fact, if we expose a general failure information retrieval in the interface, the exact implementation of the subclasses mentioned in the previous bullet point can be rightfully hidden from clients.

What are you doing in line 16?
Exceptions.translate(e, Exceptions.noSuchElement());
We've already discussed the problems with initCause, and even though some exceptions already provide chaining-aware constructors, hopefully by now you're at least considering switching to factory methods instead.

Before we hastily add chaining-aware overloads for these factory methods, however, keep in mind that to capture failure information, the factory methods are potentially already taking in several arguments. Hopefully there aren't so many that a builder pattern is necessary, but in any case adding one more just to facilitate chaining is short-sighted.

Instead, we realize that 99% of the time, an exception chaining is done in the context of an exception translation, so we simply embody the idiom in one convenient utility method. Taking advantage of generics, this can be done in a type-safe manner without requiring any casts.

As a side note, since my recent blog entries mention syntactic sugars, I thought I should propose one of my own for succinctly conveying exception chaining:
throw cause >> consequence;
//== Exceptions.translate(cause, consequence);

Source code on github

Why doesn't the method chain use the one-liner return (X) exc.initCause(cause) instead?

Because that would require @SuppressWarnings("unchecked"). Besides, we know what initCause is supposed to return anyway, so why wait for it in the pipe? The caveat is that this may not work if initCause is overridden to return something else, but I think one probably shouldn't do that in the first place.
What does the filteredStackTrace method do?

Something I hadn't mentioned is the side-effect of using factory methods to construct exceptions: those methods are part of the exception's stack trace by default. While I don't see this as a nuisance, if deemed absolutely necessary, the factory can pop out any unwanted StackTraceElement from the top of the stack trace.

Do keep in mind that getStackTrace/setStackTrace methods perform defensive copies. Though this performance cost should be negligible, I'm still not entirely convinced that it's even worth it. Nonetheless, the mechanism is shown as proof-of-concept.
Why isn't the arrayIndexOutOfBounds factory method type-safe?

Because this is just a proof-of-concept of an experimental idea, and I don't feel like cluttering it with all 9 overloads for the different array types; instead I just use java.lang.reflect.Array. The idea is, just in case you'd ever want to explicitly throw an ArrayIndexOutOfBoundsException, the factory method lets you pass the array, and the detail message of the exception would contain the length of said array.

Additional references:

And to end this on a lighter note:
/* http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4730877 */

public class WhoseFaultIsItAnyway {
   public static void main(String[] args) {
      Error yours = new Error("You");
      Error mine = new Error("Me");
      yours.initCause(mine);
      mine.initCause(yours);
      mine.printStackTrace();
   }
}
You know where this is going...
Update 2/22:

One of the rejected proposals for Project Coin (which I just learned about), the rethrows clause, attempts to address the issue of exception translation in a much more aggressive way.

I still think a syntactic sugar for initCause is needed, although I would now propose a slightly different syntax:
throw consequence <<= cause;

No comments:

Post a Comment