views:

298

answers:

7

I have a function that loops while doing something that could throw an exception. Looks something like this:

public void myFunction() throws MyException {
    while(stuff) {
        try {
            DoSomething() // throws an exception
        }
        catch (Exception ex) {
            throw new MyException(some, stuff, of, mine, ex);
        }
    }
}

The error causing the exception is recoverable. It can be something like an SQL error in a single update statement where the while loop executes a series of update statements. Or a parsing error in a single piece of data, where the loop is processing multiple pieces of data. I need to pass the exception further up the chain so the GUI part of the program can process it, handle it and pass on the error to the user. But I don't want to kill the loop in this particular function. The other things it's doing might not be invalid. The error that caused the exception might not be fatal to the function.

So my question is this: Is it an acceptable practice to build linked lists of custom exceptions (where each exception is a node, and the exception thrown is the head of the list) and then throw the head of the list (if any) once the loop finishes?

Has anyone ever seen this done? Can anyone think of any potential problems with doing this? Can anyone think of other, better ways to handle the root problem: the need to pass up multiple unrelated exceptions with out exiting the function until it is done?

Here's an example of how the linking and throw might be implemented very simply:

public void myFunction() throws MyException {
    MyException head = null;
    while(stuff) {
        try {
            DoSomething() // throws an exception
        }
        catch (Exception ex) {
            MyException tmp = new MyException(some, stuff, of, mine, ex);
            tmp.next(head);
            head = tmp;
        }
    }
    if(head != null)
       throw head;
}
+11  A: 

My intial thought (other than I've not seen this) is that an exception is potentially quite a large object (containing the stacktrace) and I'd prefer not to store a lot of them.

I would instead build a list of the erroneous parameters/arguments as exceptions occur, and upon completion of the loop, throw a custom exception populated with this list (if the list has more than 0 elements). That would seem a more manageable way of handling this scenario.

public void myFunction() throws CustomException {
    List<Error> errors = new ArrayList<Error>();
    while(stuff) {
        try {
            DoSomething() // throws an exception
        }
        catch (Exception ex) {
            errors.add(new Error(some, stuff, of, mine, ex));
        }
    }
    if (errors.size() > 0) {
       throw new CustomException(errors);
    }
}
Brian Agnew
The existing function signature returns void. You could simply return the list of failed objects, or some other structure that collects success and failure, etc.
TREE
@TREE I'm fairly certain that is an example, not the actual method
Kevin
+1, this is neater than mine. I fixed your code to declare `throws CustomException`. Oh, be aware that `Error` is an existing class in `java.lang`. You would maybe like to change the name to avoid confusion.
BalusC
+3  A: 

Do you really need to throw all the exceptions? How do you expect to the individual, unrelated exceptions to be handled? Generally in cases like this, the system will just report the errors and be done with it.

If so, you might want to just collect the error messages and add them to a custom Exception class and throw that.

Kevin
+1  A: 

If the exception thrown by DoSomething(); could cause the very same method to throw another exception; this might be a problem. In other words, if DoSomething(); throws an exception because you didn't handle the previous one, there might be unnecesarry error to handle.

Erkan Haspulat
True. In the scenario I'm asking about this wouldn't (theoretically) be the case, however, this is definitely something to keep in mind.
Daniel Bingham
+3  A: 

If those exceptions are really unrelated to each other so that you can't take benefit of get/setCause(), then I would rather collect this information in one MyException.

E.g.

public void myFunction() throws MyException {
    MyException myException = null;
    while(stuff) {
        try {
            DoSomething() // throws an exception
        }
        catch (Exception ex) {
            if (myException == null) {
                myException = new MyException();
            }
            myException.addException(some, stuff, of, mine, ex);
        }
    }
    if (myException != null) {
        throw myException;
    }
}

Update: Brian handles exactly this approach in a more neat manner. I would opt for that instead :)

BalusC
A: 

IMO, an exception should be the last resource you have for handling an error. It should be avoided if possible. So, you might want to pass the error description (create error codes, pass the message, or something meaningful) to the GUI, and not the exception itself.

tou
+1  A: 

Actually throwing any exceptions from such a function is probably not the right way to handle this if it is expected that there will be errors. I'd suggest to either return a List (Array) of all exceptions/errors that occured or better to provide a error handler object to the function that can deal with the exceptions. i.e.:

public interface ErrorHandler
{
    public void handleError( Throwable ex /*, add some context info */ );
}

public void myFunction( ErrorHandler eh )
{   
    while(stuff) {   
        try {   
            DoSomething() // throws an exception   
        }   
        catch (Exception ex) {
            if( eh != null )
                eh.handleError( ex );
        }   
    }   
}   

This also lets the error handler either collect the errors to present them to the user or to decide that the whole batch operation has become void because of some error and to throw a exception of it's own to abort the processing early.

x4u
+1  A: 

I think you can pass some callback or listener to the method, or set in a class variable and instead of a throw the list, like x4u did.

In Java there is an interface for this already: java.beans.ExceptionListener

Dudaskank