5
\$\begingroup\$

I am having difficulty deciding how to implement an exception handling strategy.

I am using an observer pattern to allow "plugin" programmers to subscribe to Messages. These subscribers generally log a unique error (and do some other stuff) under an exceptional circumstance during handling the message. The code snippet below is an example of what a common implementation (and its interface) looks like:

// API interface
interface IMessageListener {
   void onMessage(Message m);
}

class StuffMessageListener implements IMessageListener {
    ...

    @Override void onMessage(Message m) {

        try {
            Stuff s = stuffDAO.getStuff(..); // throws SQLException

            ...

            m.reply(true);
        } catch (SQLException e) {
            // if an exception happens, log it, and reply to the message
            log.error(A_UNIQUE_ERROR_CODE, e);
            m.reply(false);
        }
    }
}

What irks me is that the catch block has become very redundant code, copied throughout 95% of the ~50 existing listeners. Can we come up with something better?

A good solution should:

  • be easy to use/understand for an amateur programmer
  • keep the programmer aware of the existence/potential of exceptions
  • minimize the chance of a programmer accidentally "swallowing" an exception
  • reduce the amount of "boilerplate" code

There is a subtle requirement needed for the solution. The UNIQUE_ERROR_CODE should be unique across all declarations of the listeners. That way, I could gain statistics based upon where the error was caught across all listeners.

If the UNIQUE_ERROR_CODE is buried underneath an abstract class, or is used outside onMessage(..), I will lose the needed "uniqueness."

\$\endgroup\$
4
  • \$\begingroup\$ What does m.reply(boolean) do? \$\endgroup\$ – Nick ODell Aug 1 '12 at 17:40
  • \$\begingroup\$ It signals to the sender of the message that the handling was (un)successful. \$\endgroup\$ – Beefyhalo Aug 1 '12 at 17:42
  • \$\begingroup\$ Why not make it assume by default that it was unsuccessful? \$\endgroup\$ – Nick ODell Aug 1 '12 at 17:44
  • \$\begingroup\$ Thx for the suggestion @NickODell. I could use that strategy and add a suppressReply() method, but I'm not sure how much I like that.. The reason for this new method is because in some cases, the message should not be replied to. \$\endgroup\$ – Beefyhalo Aug 1 '12 at 17:47
9
\$\begingroup\$

Allow throwing exceptions

Do you really think your user is capable of handling all the exceptions?

interface IMessageListener {
   void onMessage(Message m) throws Exception;
}

Of course if some plugin wants to handle the exception, nothing prevents he/she of using try/catch. But no one is forced to do so and if, according to your knowledge, the catch block is almost always the same, put it outside in the code calling onMessage().

Give your user a simplified adapter

abstract class ThrowingMessageListenerAdapter implements IMessageListener {

    @Override
    public void onMessage(Message m) {
        try {
            doOnMessage(m);
        } catch (SQLException e) {
            // if an exception happens, log it, and reply to the message
            log.error(A_UNIQUE_ERROR_CODE, e);
            m.reply(false);
        }
    }

    protected abstract void doOnMessage(Message m) throws Exception;

}
\$\endgroup\$
1
  • \$\begingroup\$ I really like this answer. However, there is a fixed requirement that the user must not throw a exception in the onMessage() (like JMS). Please check my update to know why the adapter solution will not preserve unique error codes. \$\endgroup\$ – beefyhalo Aug 1 '12 at 20:50
6
\$\begingroup\$

This looks like a good place to use an abstract class instead of an interface:

abstract class MessageListener{
    void onMessage(Message m){
        try{ onMessageLogic(m); }
        catch(Exception e){ ... }
    }
    abstract void onMessageLogic(Message m) throws Exception;
}

class StuffMessageListener extends MessageListener{
    onMessageLogic(Message m) throws Exception{  
        /* possibly exception-raising code here */  
    }
}
\$\endgroup\$
0
\$\begingroup\$

best thing would be to

implement getStuff method with Message m as a parameter to it. This way you can catch the exception inside your method which set

m.reply(true) or m.reply(false)

inside your code itself making it easier for others to use it.

\$\endgroup\$
1
  • \$\begingroup\$ Side effects should be avoided, not encouraged. \$\endgroup\$ – Wug Aug 1 '12 at 17:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.