views:

69

answers:

2

Hi there, I have written a class which loads configuration objects of my application and keeps track of them so that I can easily write out changes or reload the whole configuration at once with a single method call. However, each configuration object might potentially throw an exception when doing IO, yet I do not want those errors to cancel the overall process in order to give the other objects still a chance to reload/write. Therefore I collect all exceptions which are thrown while iterating over the objects and store them in a super-exception, which is thrown after the loop, since each exception must still be handled and someone has to be notified of what exactly went wrong. However, that approach looks a bit odd to me. Someone out there with a cleaner solution?

Here is some code of the mentioned class:

public synchronized void store() throws MultipleCauseException
    {
    MultipleCauseException me = new MultipleCauseException("unable to store some resources");
    for(Resource resource : this.resources.values())
        {
        try
            {
            resource.store();
            }
        catch(StoreException e)
            {
            me.addCause(e);
            }
        }
    if(me.hasCauses())
        throw me;
    }
+3  A: 

If you want to keep the results of the operations, which it seems you do as you purposely carry on, then throwing an exception is the wrong thing to do. Generally you should aim not to disturb anything if you throw an exception.

What I suggest is passing the exceptions, or data derived from them, to an error handling callback as you go along.

public interface StoreExceptionHandler {
    void handle(StoreException exc);
}

public synchronized void store(StoreExceptionHandler excHandler) {
    for (Resource resource : this.resources.values()) {
        try {
            resource.store();
        } catch (StoreException exc) {
            excHandler.handle(exc);
        }
    }
    /* ... return normally ... */
]
Tom Hawtin - tackline
Considering reloading, the objects which throw an exception then represent the latest possible consistent state (i.e. keep a stale copy of the configuration).
+1  A: 

There are guiding principles in designing what and when exceptions should be thrown, and the two relevant ones for this scenario are:

  • Throw exceptions appropriate to the abstraction (i.e. the exception translation paradigm)
  • Throw exceptions early if possible

The way you translate StoreException to MultipleCauseException seems reasonable to me, although lumping different types of exception into one may not be the best idea. Unfortunately Java doesn't support generic Throwables, so perhaps the only alternative is to create a separate MultipleStoreException subclass instead.

With regards to throwing exceptions as early as possible (which you're NOT doing), I will say that it's okay to bend the rule in certain cases. I feel like the danger of delaying a throw is when exceptional situations nest into a chain reaction unnecessarily. Whenever possible, you want to avoid this and localize the exception to the smallest scope possible.

In your case, if it makes sense to conceptually think of storing of resources as multiple independent tasks, then it may be okay to "batch process" the exception the way you did. In other situations where the tasks has more complicated interdependency relationship, however, lumping it all together will make the task of analyzing the exceptions harder.

In a more abstract sense, in graph theory terms, I think it's okay to merge a node with multiple childless children into one. It's probably not okay to merge a whole big subtree, or even worse, a cyclic graph, into one node.

polygenelubricants
You can't genericise exceptions in Java.
Tom Hawtin - tackline
Wow, that is a surprise. Thanks for pointing that out.
polygenelubricants
Already tried it and was surprised alike. Yet I don't see any reason since generics just expand into some implicit casts which would be really helpful here. Maybe someone thought "we have type erasure and something like catch(GenericException<FooException> e) { }catch (GenericException<BarException> e) { } won't work, so we prevent programmers from even trying"