Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Avoiding instanceof when checking a message type

I have the following situation where a client class executes different behavior based on the type of message it receives. I'm wondering if there is a better way of doing this since I don't like the instanceof and the if statements.

One thing I thought of doing was pulling the methods out of the client class and putting them into the messages. I would put a method like process() in the IMessage interface and then put the message specific behavior in each of the concrete message types. This would make the client simple because it would just call message.process() rather than checking types. However, the only problem with this is that the behavior contained in the conditionals has to do with operations on data contained within the Client class. Thus, if I did implement a process method in the concrete message classes I would have to pass it the client and I don't know if this really makes sense either.

public class Client {
    messageReceived(IMessage message) {
        if(message instanceof concreteMessageA) {
            concreteMessageA msg = (concreteMessageA)message;
            //do concreteMessageA operations
        }
    }
        if (message instanceof concreteMessageB) {
            concreteMessageb msg = (concreteMessageB)message;
            //do concreteMessageB operations
        }
}
like image 738
volker238 Avatar asked Aug 10 '09 03:08

volker238


2 Answers

The simple way to avoid instanceof testing is to dispatch polymorphicly; e.g.

public class Client {
    void messageReceived(IMessage message) {
        message.doOperations(this);
    }
}

where each message class defines an appropriate doOperations(Client client) method.

EDIT: second solution which better matches the requirements.

An alternative that replaces a sequence of 'instanceof' tests with a switch statement is:

public class Client {
    void messageReceived(IMessage message) {
        switch (message.getMessageType()) {
        case TYPE_A:
           // process type A
           break;
        case TYPE_B:
           ...
        }
    }
}

Each IMessage class needs to define an int getMessageType() method to return the appropriate code. Enums work just as well ints, and are more more elegant, IMO.

like image 146
Stephen C Avatar answered Oct 06 '22 01:10

Stephen C


One option here is a handler chain. You have a chain of handlers, each of which can handle a message (if applicable) and then consume it, meaning it won't be passed further down the chain. First you define the Handler interface:

public interface Handler {
    void handle(IMessage msg);
}

And then the handler chain logic looks like:

List<Handler> handlers = //...
for (Handler h : handlers) {
    if (!e.isConsumed()) h.handle(e);
}

Then each handler can decide to handle / consume an event:

public class MessageAHandler implements Handler {
    public void handle(IMessage msg) {
        if (msg instanceof MessageA) {
            //process message
            //consume event 
            msg.consume();
        }
    }
}

Of course, this doesn't get rid of the instanceofs - but it does mean you don't have a huge if-elseif-else-if-instanceof block, which can be unreadable

like image 24
oxbow_lakes Avatar answered Oct 05 '22 23:10

oxbow_lakes