I'm completely new to this code and begun to questions the design choices of the original developers.
I have a multi-threaded Java application that processes a number of tasks. My job is to fix the exception handling in the code so that when a RuntimeException occurs (iBatis and/or NullPointerException) the rest of the tasks are executed instead of the thread terminating. I want to know what is the best way to deal with the exception handling below:
public List<WorkUnit> performModule(List<WorkUnit> listOfInputs)
throws ModuleException {
List<WorkUnit> listOfOutputs = new ArrayList<WorkUnit>();
for (WorkUnit mi : listOfInputs) {
WorkUnit mo=null;
try {
if (mi instanceof BulkOrder) {
mo = performModuleOperation(...);
} else if (mi instanceof Order) {
mo = performModuleOperation(...);
} else if (mi instanceof PreReleaseLoad) {
mo = performModuleOperation(...);
} else if (mi instanceof Load) {
mo = performModuleOperation(...);
}
listOfOutputs.add(mo);
} catch (OMSException e) {
if (e.shouldProcessFurther()) {
listOfOutputs.add((mo!=null) ? mo : mi);
}
//save error to database - code was removed
if ( e.getExceptionType().equals(ExceptionType.TECHNICAL)) {
if ( e instanceof ModuleException ) {
throw (ModuleException) e;
} else {
throw new ModuleException(e);
}
}
} catch (Throwable th) {
ModuleException me = new ModuleException(th);
ExceptionHandler.logException(me, (WorkUnit)mi,orderDelegate);
throw me;
}
}
return listOfOutputs ;
}
I have two major problems. 1) The catch for the Throwable object. I understand that they want to capture checked exceptions and unchecked exceptions. I guess that they want to check for Errors as well but the Sun documentation for exception handling specifically states that this is highly advised against. In the event you get a really serious error like the JVM running out of memory you may not be able to recover. This could be caught in a log file and I don't agree with it needing to be dealt with. Personally if you are tracking technical and applications exceptions errors doesn't seem like something that you would monitoring like any other exception. I could be wrong...
2) It isn't clear how the exceptions are being handled. In the code below the, which the exception throws to the code above it wraps a regular exception be it checked or unchecked into a custom exception and throws that. The code above looks for an OMSException which is the parent of every custom exception in the entire application. Is this a good design? You can see where they include a ExceptionType to the custom exception object. There already seems to be an functionality built into the existing exception handling of Java. If it is an application exception then throw a custom exception. If it is a technical exception, something is null when it isn't suppose to be or a database connection problem, then catch a unchecked exception from Java and react accordingly. This whole thing seems confusing. I just want to know what thoughts other have on the whole thing.
public Order performModuleOperation(Order order)
throws ModuleException {
try {
Map<String, Rule> rules = ...
}
catch (InductException ie) {
throw ie;
}
catch (Exception e) {
e.printStackTrace();
throw new InductException(e.toString(),e);
}
return order;
}