views:

57

answers:

3

Here's some code from an FTP application I'm working on. The first method comes from an interface which is being triggered by a class which is monitoring server output.

@Override
public void responseReceived(FTPServerResponse event) {
    if (event.getFtpResponseCode() == 227) {
        try {
            setupPassiveConnection(event.getFullResponseString());
        } catch (UnknownHostException e) {
            e.printStackTrace();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
}

It invokes the second setupPassiveConnection() which throws some Exceptions.

public void setupPassiveConnection(String serverReplyString) throws UnknownHostException, IOException {
    String passiveInfo[] = serverReplyString.substring(
            serverReplyString.indexOf("(") + 1,
            serverReplyString.indexOf(")")).split(",");

    int port = (Integer.parseInt(passiveInfo[4]) * 256)
            + (Integer.parseInt(passiveInfo[5]));

    String ip = passiveInfo[0] + "." + passiveInfo[1] + "."
            + passiveInfo[2] + "." + passiveInfo[3];

    passiveModeSocket = new Socket(ip, port);

    if( passiveModeSocket != null )
        isPassiveMode();
}

As the Exceptions can't be rethrown through the first method what would be a proper way to rewrite this?

+1  A: 

If you can't change the interface (or don't want to), your exception should be a RuntimeException (or some custom subclass of that). You could wrap the UnknownHostException in your RuntimeException as the cause (using the constructor that takes a Throwable).

Jeff Storey
Thanks for the reply. The interface should probably stay neutral so your solution seems a good one. Where should the `RuntimeException` be thrown? In `setupPassiveConnection()` or the Override of the interface method?
James P.
@James. Wherever you would want to throw UnknownHostException is where you would throw the runtime exception. So in this case, it would be from setupPassiveConnection. You don't have to catch it though in the responseReceived method, and if you don't, it will just continue to get thrown up the stack.
Jeff Storey
+1  A: 

If you can't handle them immediately, like displaying a sensible error message in the UI notifying that the enduser has to take another approach, or taking an alternative path in the code, then you need to wrap and rethrow them as a RuntimeException when it signifies a recoverable developer error, or as an Error when it signifies an unrecoverable error, for example IOError. Either way, this needs to be documented properly and clearly.

BalusC
Thanks BalusC. This seems to follow what is explained in this topic:http://forums.sun.com/thread.jspa?threadID=5375261
James P.
+3  A: 

Do you mean that the first code block is inside a class that implements an interface that specifies responseReceived with no throws clause, so you can't rethrow?

In that case, your class must store the result and provide an API via which clients can retrieve the response, i.e. a getResponseCode() method.

Take a look at java.util.concurrent classes ExecutorService and Future.

Jim Garrison
Yes. It's an attempt to apply the callback pattern (see http://stackoverflow.com/questions/2892743/handling-asynchronous-responses). As it's not possible to get an immediate reply after a command is sent, I'm using a thread to detect when an ftp server is sending a reply. I'll see if I can find a practical way to store the result.
James P.