An Aspect is Part of Your Implementation

When implementing parts of your application with AOP, you are magically applying additional behavior to your code. Although technically cool and in terms of reusability great, it might lead to a maintenance nightmare.

Think of an API, let's call it CustomerService. It defines CRUD methods to load and save Customer instances from some backend service. It might define methods like the following:

/**
 * blah
 *
 * @throws CustomerServiceException with following causes:
 *    - IllegalArgumentException if anything is wrong with the passed Customer instance
 *    - NullPointerException if anything is wrong with a collaborator
 *    - FancyPersistenceException if anything is wrong your persistence service
 */
public Customer save(Customer argToSave) throws CustomerServiceException;

Note the standard Javadocs, which define precisely the causes of a CustomerServiceException, which the client of this API can expect. These defined causes as well as the CustomerServiceException are part of the contract defined by the interface. Clients can program against those causes e.g. display different error messages or inform another system.

An implementation of this interface might look as the following:

@Override
public Customer save(Customer argToSave) throws CustomerServiceException {
  try {
    // do some fancy stuff
  } catch (IllegalArgumentException e) {
    
    // do some logging here
    throw new CustomerServiceException(e);  
  
  } catch (NullPointerException e) {
    
    // this is programming error
    throw new CustomerServiceException(e, "Dude where's your head?");  
  
  } catch (FancyPersistenceException e) {
    
    // you might want to do some transaction handling stuff here
    throw new CustomerServiceException(e);  
  
  } catch (Exception e) {
    throw new CustomerServiceException();  
  }
}

This is pretty ugly isn't it? Especially because of the verbose Exception handling. However it's complying with the contract and a valid implementation.

In my current project we followed this pattern in every service class throughout the application. This works fine and provides a lot of consistency for clients of those APIs. However making a commitment like this requires not only a lot of discipline, when implementing those contracts, but also a lot of test code. You have to make sure that each implementation complies with the contracts promised in its interface.

A new member of the team immediately pointed out the drawbacks of our approach: a lot of test code to maintain, a lot of verbose Exception handling polluting the implementation methods and no centralized way to manage the same recurring code. His idea was to use AOP. You can refactor the Exception handling into an Aspect and thus get rid of all the drawbacks mentioned before.

The implementation of the CustomerService interface boils down to the following:

@Override
public Customer save(Customer argToSave) throws CustomerServiceException {
     // do some fancy stuff
}

For a moment this might look like a perfect solution. The ugly technical Exception handling is gone and we are left with the code that actually gets the job done. That makes reading the code so much easier and don't think I'm the only fan of that.

Despite of all the drawbacks we go rid of, we silently introduced new ones. The fact that the implementation doesn't contain any hard reference to those Exceptions we claim to throw in the interface, makes understanding the implementation of the contract a lot harder. Not to mention the code for the Exception handling itself. Another drawback is refactoring the interface. Imagine someone unfamiliar with the code has to change one of the Exception causes. It is merely impossible to understand what's going on.

You have to consider the Aspect containing the logic to handle the Exception as of your implementation. Without it your implementation doesn't comply with the contract.

So what to do now? We have a great solution, which relieves us from all the drawbacks but brings along another bag of drawbacks. Should we gauge which drawbacks are easier to deal with? I remembered a solution I read in Applying UML and Patterns, which suggests a Singleton based centralized error handling. So I came up with the following solution:

public final class ExceptionHandler {
  // private constructor
  public static final <T extends BaseException> transform(Exception argCause, Class<T> argType) {
    try {
      if (argType.isAssignableFrom(argCause.getClass())) {
        return (T) argCause;
      } else if (argCause instanceof NullPointerException) {
        return argType.getConstructor(Throwable.class, String.class).newInstance(argCause,"Dude where's your head?");
      } else if (argCause instanceof FancyPersistenceException) {
        return argType.getConstructor(Throwable.class).newInstance(argCause);
      } else {
        return argType.getConstructor(Throwable.class, String.class).newInstance(argCause,"Unkown!");
      }     
   } catch (Exception e) {/** do some reflection error handling **/}
  }
}

The implementation of the CustomeService interface using the centralized ExceptionHandler looks like this:

@Override
public Customer save(Customer argToSave) throws CustomerServiceException {
  try {
     // do some fancy stuff
  } catch (Exception e) {
    throw ExceptionHandler.transform(e, CustomerServiceException.class);  
  }
}

I believe this is a great trade-off. You have a single point in your code responsible for handling Exceptions. It's easy to test in isolation, which means the test code for your services decrease. The overhead in your service methods is minimal. You have to implement a try-catch block, but I think that's reasonable considering the benefits of this approach.

However, I think the biggest advantage of this solution is the fact that it works without any framework magic whatsoever. It's just plain old Java!

NOTE: Over the next few weeks, I'm moving my feed from feedburner back to blogspot so please subscribe to http://dlinsin.blogspot.com/feeds/posts/default/ instead!