Question

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
        }
}
Was it helpful?

Solution

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.

OTHER TIPS

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

What type of message system are you using?

Many have options to add a filter to the handlers based on message header or content. If this is supported, you simply create a handler with a filter based on message type, then your code is nice and clean without the need for instanceof or checking type (since the messaging system already checked it for you).

I know you can do this in JMS or the OSGi event service.

Since you are using JMS, you can basically do the following to register your listeners. This will create a listener for each unique message type.

  String filterMsg1 = "JMSType='messageType1'";
  String filterMsg2 = "JMSType='messageType2'";

  // Create a receiver using this filter
  Receiver receiverType1 = session.createReceiver(queue, filterMsg1);
  Receiver receiverType2 = session.createReceiver(queue, filterMsg2);

  receiverType1.setMessageHandler(messageType1Handler);
  receiverType2.setMessageHandler(messageType2Handler);

Now each handler will receive the specific message type only (no instanceof or if-then), assuming of course that the sender sets the type via calls to setJMSType() on the outgoing message.

This method is built into message, but you can of course create your own header property and filter on that instead as well.

//Message.java

abstract class Message{
     public abstract void doOperations();
 }

//MessageA.java

 class MessageA extends Message{
    public void doOperations(){ 
         //do concreteMessageA operations ;
    }
 }

   //MessageB.java

class MessageB extends Message {
     public void doOperations(){ 
         //do concreteMessageB operations 
     }
}

//MessageExample.java

class MessageExample{
   public static void main(String[] args) {
        doSmth(new MessageA());
    }

    public static void doSmth(Message message) {
       message.doOperations() ;     

     }
}   

A Java 8 solution that uses double dispatch. Doesn't get rid of instanceof completely but does only require one check per message instead of an if-elseif chain.

public interface Message extends Consumer<Consumer<Message>> {};

public interface MessageA extends Message {

   @Override
   default void accept(Consumer<Message> consumer) {
      if(consumer instanceof MessageAReceiver){
        ((MessageAReceiver)consumer).accept(this);
      } else {
        Message.super.accept(this);
      }
   }
}

public interface MessageAReceiver extends Consumer<Message>{
   void accept(MessageA message);
}

With JMS 2.0 you can use:

consumer.receiveBody(String.class)

For more information you can refer here:

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top