Visitor Pattern and checked exceptions
I have a Visitor interface and I want to have 2 implementations. The only problem is one Visitor does database things so it throws SQLException and the other does IO things so it throws IOException.
What are my options when declaring the visitor methods?
Declare them to throw Exception
Declare them to throw VisitorException and catch and rethrow inside
Declare no checked excepions and handle them in the visitor (for example throwing an unchecked exception)
Other?
I guess all are valid, but maybe some are better than others.
[573 byte] By [
fedetxfa] at [2007-10-3 0:27:22]
> I have a Visitor interface and I want to have 2 implementations.
> one Visitor throws SQLException and the other throws IOException.
> What are my options when declaring the visitor methods?
>
> - Declare them to throw Exception
> - Declare them to throw VisitorException and catch and rethrow inside
> - Declare no checked excepions and handle them in the visitor (for example throwing an unchecked exception)
> - Other?
The problem with throwing exceptions from your visit methods is, who will handle them?
If you're traversing a structure, visiting each node one by one, the structure itself (or the nodes) call the visitor. If the individual visit of one node fails so much that the visitor throws an exception, how does the visitee know if it has to let the traversal go on? This probably depends on the visitor's specifics, but the visitee shouldn't have to know about the visitor's implementation.
Very likely, the visitee will not catch the exception (otherwise he would have to make the decision as to how to handle the exception), so he will let it propagate (or at most, catch but rethrow it or another exception) to the "original caller" (the one which called acceptVisit() on the root node). For a complex structure, this means the traversal will have been interrupted.
Now the original caller probably knows which kind of visitor implementation he passed to the visitee, so he can take appropriate action.
All that based on the assumption that the visitor throws the exception.
Under this design, I think the most transparent one is that the visitor throws a RuntimeException subclass, embedding a root cause of a specific exception class. You can design your VisitorException to be a RuntimeException, so as to keep a bit more type information.
The advantage is that the visitee doesn't have to worry about compile-time exception handling, which he can't do much other than rethrow or ignore them...
The "original caller" will be aware, and catch the RuntimeException or VisitorException, and extract the root cause, and act accordingly.
Now the other option is, that the visitor doesn't throw an exception at all. Instead he can catch the low-level exception, and switch a flag, or accumulate the errors into whatever accumulation variable. The original caller again, will not have to catch an exception, but he will have to query the visitor for the result of the overall operation.
As an optimizaton, the visitor coud include a fail-fast check to not execute all the business logic for the next visits, after one visit has turned up the flag.
So the first question is: should the visitee structure give up the next visits as soon as one visit has failed?
Maybe you can be a bit more specific in this thread about what your Visitor and Visitee classes do.
Throwing a RuntimeException is anything but transparent. Transparent means you can see what is going on. Throwing a RuntimeException just hides what is going on but does not protect you from it. Its a terrible idea unless you have designed your whole architecture around catching exceptions on the highest levels only.
My problem is that your are visiting a class that is obviously doing JDBC things with a visitor that does not understand JDBC exceptions. That not right. Im sure other methods on your class throw JDBC exceptions as well, not just the visitor. It appears to me that you are missing a layer of separation. This layer should sit between the layer that has the visitor and the JDBC/IO layer. It should thrown its own type of checked exception.
Runtime exceptions have a general expectation that when they are thrown the program will be shut down. If this is not the case then you would need any layers that create resources to catch runtime exceptions so they can clean up these resources. Else you leak.
I have a simple tree structure. A root with many ServiceAccounts and each has many ServiceElements. I have two tasks: print a report showing the tree's information in some format and insert a row for each leaf node in some table.
I don't think I understand _dnoyeB's point. I'm visiting a class that does not do any JDBC with a visitor that does JDBC operations, not the opposite way.
The tree knows nothing about those operations, it just accepts visitors that do either of those tasks. It is clear to me that any IOException or any SQLException will require aborting the whole traversal. I agree that the tree will not handle those exceptions, it will be the outermost caller that will probably be in a servlet (or struts action) and the probable response will be to forward to an error jsp page.
I want those fatal exceptions to be caught and handled there, otherwise I think I will get a ServletException with a default error page.
I'm leaning towards creating a checked VisitorException or something like that. In that way I'm sure they are handled. I want them to ba handled so the outermost caller closes the jdbc connection or the Writer used for the output. Otherwise I will leak connections and leave files in unknown states.
> I'm leaning towards creating a checked
> VisitorException or something like that. In that way
> I'm sure they are handled. I want them to ba handled
> so the outermost caller closes the jdbc connection or
> the Writer used for the output. Otherwise I will leak
> connections and leave files in unknown states.
The problem with using a checked exception is that everyone now has to deal with it even it if there isn't a good way to handle it at that point or if the exception never occurs.
A lot of languages don't even have checked exceptions. One option is to use a unchecked exception type and document it in the throws part of the JavaDoc.
I find that unless the exception is expected to occur in the normal operation of the application, you should just use an unchecked exception. The argument that the this is somehow dangerous because the code using it may not catch runtime excptions is specious because all Java applications need to have handling at some level for runtime exceptions because most, if not all, method calls could throw an unchecked exception.
I agree partly Dubwai, but remember what I said about the unchecked exceptions. There is a general expectation that if its thrown the application will be shutting down. This is because by default 99% of unchecked exceptions can not be recovered from. (there are some rule benders like ConcurrentModificationException). If it is not the case then every creator of a resource needs to catch runtimeException so the particular layer does not leak on runtimeexception.
Another difference is that for checked exceptions, regardless if you can handle it or not, you need to deal with it in code. With unchecked, you only need to write code if you can handle it on a certain level. Nice up front, but when refactor time comes along its tougher to be sure you are catching them all appropriately.
I think the OPs fundamental problem is one of layer separation. If the visitor can encounter a recoverable error, then the visitor should throw an exception checked or otherwise.This should not be a JDBC or IO exception. It should be an exception based on the layer of the program.
Why is your tree ignoring such exceptions? It should handle it by displaying something appropriate (like emptying the tree), then passing up an exception or otherwise to instruct the rest of the program that the tree is not proper. I have a tree and I also have a visitor that does just this.
The tree catches the exception, empties the tree, sets it to unuseable, and opens an error dialog using a static method, that displays the error.
> I agree partly Dubwai, but remember what I said about
> the unchecked exceptions. There is a general
> expectation that if its thrown the application will
> be shutting down.
I don't disagree with this. What it really comes down to is whether there is anything the handler can do about the exception. The problem with an issue like the OPs is that there's no one answer. A JDBC error may be something to handle, sometimes JDBC issues are temporary. On the other hand you might work with some other Visitor where only fatal exceptions are thrown. It's something the developer must decide.
I have done with Builders which might work with visitors. You can put a handleException method in the Visitor interface. This can be a little tricky to write but it creates a structured way to handle exceptions without adding lots of catch blocks and without checked exceptions everywhere.
The thing I have to do when I encounter an SQLException is mainly a rollback. Since the visitor is executed inside an ongoing transaction it is the caller's resposibility to call commit or rollback in addition to displaying an error message or a success message. I now understand better the forces I have to analize when designing exceptions.
I chose declaring the visitor methods to declare to throw VisitException which isa checked exceptions. In this way I'm forcing the caller of tree.accept(visitor) to handle that exception in some way. I prefer an extra catch block over a default error page and a leaked connection.
