Design advice for mess if/then logic

Question

I am willing to have a design pattern choice for this common scenario:

I have a module which receives messages(MessageListener). Eeach message it receives is actually an object (MyMessage1,MyMessage2,..)

My_Message1,My_Message2 exteneds My_Message_Abstract.

Now when a message being retrieved by the the MessageListener Object(onMessage(..)) I want to do a diffident "command" depends on the message instance.. something like this:

onMessage(My_Message_Abstract msg)
{
   if (msg instance of My_Message1)
   {
      doSomething()..
   }
   else if(msg instance of My_Message2)
   {
     doSomethingElse()..
    }
}

I want to get rid of this boiler if/then code and having it better future- maintenance/dynamic/plug-ability/neat way.

So I took the Command design pattern. and I found out that I could have something like that:

in the MessageListener to have a map:

Map<Integer, MessageCommand> messageCommandsMap = new HashMap<Integer, MessageCommand>();

..

sessionTargetMap.put(MSG_1_TYPE, new Message1Command());
sessionTargetMap.put(MSG_2_TYPE, new Message2Command());

(Message1Command,Message2Command implements from Command interface)

onMessage(My_Message_Abstract msg)
{
messageCommandsMap.get(msg.getType).executeCommand(msg);
}

I didnt like the hashmap idea in the MessageListeenr in this way i am coupling all commands to that object(MessageListener).

As the solution provided at this thread: Long list of if statements in Java

Any idea how could I improve this? Mybe I should use other pattern for this idea?

thanks,

Answer 1

The approach I take is to decorate these classes with a common interface that enables me to utilize the virtual nature of Java functions. For example I would do this:

public interface CoolInterface  
{
      void doSomething();  
}  

My_Message_Abstract implements CoolInterface  
{  
      public abstract void doSomething();
}  
Message1Command extends My_Message_Abstract
{  
       public void doSomething(){ System.out.println("First");
}    
Message2Command extends My_Message_Abstract
{  
       public void doSomething(){ System.out.println("");
}   

Your code now becomes:

onMessage(My_Message_Abstract msg)
{
  msg.doSomething();
}

IF you wanted to delegate just do this:

 My_Message_Abstract implements CoolInterface  
    {  
          public void doSomething()
          {
             System.out.println("Default");
          }  
    }    

   Message1Command extends My_Message_Abstract
{  
       public void doSomething(){ System.out.println("First");
}    
Message2Command extends My_Message_Abstract
{  
       // no need to override the method just invoke doSomething as normal
}    

Answer 2

I always like to use an enum when you want a list of possible things to do:

public class Test {
  // A type of message.
  class MyMessage1 {
  };
  // A whole set of message types.

  interface MyMessage2 {
  };

  // The Dos - To demonstrate we just print something.
  enum Do {
    Something(MyMessage1.class) {
      @Override
      void doIt() {
        System.out.println("Something");
      }
    },
    SomethngElse(MyMessage2.class) {
      @Override
      void doIt() {
        System.out.println("Something else");
      }
    },
    Everything(Object.class) {
      @Override
      void doIt() {
        System.out.println("Everything");
      }
    };
    // Which classes this one applies to - could use an array of Class here just as easily.
    final Set<Class> applies = new HashSet<Class>();
    // You can add multiples on construction.
    Do(Class... applies) {
      this.applies.addAll(Arrays.asList(applies));
    }

    // Perform all that are relevant to this message type.
    static void doIt(Class messageClass) {
      for (Do d : Do.values()) {
        // If it is assignable
        boolean doIt = false;
        for (Class c : d.applies) {
          if (c.isAssignableFrom(messageClass)) {
            doIt = true;
          }
        }
        if (doIt) {
          // Execute the function.
          d.doIt();
        }
      }
    }

    // What to do.
    abstract void doIt();
  }

  public void test() {
    System.out.println("Hello");
    // Test with a concrete message.
    onMessage(new MyMessage1());
    // And an implementation of an interface.
    onMessage(new MyMessage2() {
    });
  }

  private void onMessage(Object message) {
    // Do something depending on the class of the message.
    Do.doIt(message.getClass());
  }

  public static void main(String args[]) {
    new Test().test();
  }
}

With this pattern you can even have an Everything which is triggered for all messages.

It would be easy to extend this to use an array of classes this Something is applicable to.

If you want just the first match to apply, break out of the loop at the appropriate place.

My point is - with this mechanism - you can implement almost any strategy you like without having to hack the code around aggressively.

Answer 3

You can have a MessageCommand Factory which basically knows which MessageCommand to be utilized for the given message type. In factory, you can use a map or if/else just to identify the command class.

Now your message listener asks factory to give the appropriate Message Command based on type and then each Message Command holds the logic to handle the command (doSomethingElse) method effectively.

Abstract idea of how code looks like:

   class MessageCommandFactory {
       Command get(MessageType messageType) {
         if(messageType == MSG_1_TYPE) {
           return new Message1Command();
         } else ...
       } 

    class MessageListener {
    MessageCommandFactory messageCommandFactor;
        onMessage(My_Absctract_Message message) {
          Command command =  messageCommandFactory.get(message.getType());
          command.execute();
      }
   }

Answer 4

So actually your MessageListener does two steps:

1. Figure out what message was received
2. Run the appropiate command

You can either delegate the "running" to the message itself like @Woot4Moo has, or subclass MessageListener to do specific things for the message like this:

public class Message1Listener implements MessageListener {
  onMessage(SpecificMessage1 msg) {
    /* do something with msg */
  }
}

public class MessageBus {
  Map<Class<? extends Message>, MessageListener> listeners = new HashMap<>();
  void register(MessageListener listener, Class<? extends Message> msgType) {
    listeners.put(msgType, listener);
  }

  void onMessage(Message msg) {
    listeners.get(msg.getClass()).onMessage(msg);
  }
}

Your MessageListener has become the MessageBus here, proxying to the registered handler. This is a very basic setup you can extend as necessary (ie have a list of MessageListener for each message type) or even may already exist in your context.

Answer 5

In a similar situation, I've started with what you already have, but have added a method:

addMessageListener(int messageType, MessageCommand messageCommand)

In addition, other classes need a way to "locate" your class. The simplest would be to kake the above method public static, if that works for your environment... but other ways of discovery may be used, as appropriate.

The method simply adds to the map you already have.

If two commands wish to listen to the same message, it would easy enough to tweak for that by making the "value" side of your map into a List

Also, you can always start the map out with some pre-determined Commands already "pre-registered", or read the pre-registration info from some type of configuration...and so on.

Example 1: Suppose you have a new class called NewMessageHandler. This class needs to handle a completely new message type. At some appropriate point, when NewMessageHandler initializes itself, if can also register itself as a listener. It has to "find" the dispatcher class (hopefully that's a singleton), and then it can call addMessageListener().

Example 2: The Dispatcher class could read a configuration file that defines pairs of message-type and class that will handle the message.

public class Dispatcher 
{
  public static Dispatcher getInstance()
  {
     //return the instance. Could accept parms for more complex needs
  }

  public void addMessageListener(int messageType, MessageCommand messageCommand)
  {
    //Add to the internal map
  }

  private void init()
  {
     //Optionally, read config file or System properties, and call addMessageListener()
  }

  private void dispatchMessage(Message msg)
  {
    //Look up map and dispatch to the registered instance
    //Call the handleMessage() method on the appropriate listener      
  }

}

the interface

public interface MessageCommand 
{
   public void handleMessage(Message msg);
}

and the other one...

public class NewMessageHandler implements MessageCommand 
{

   private void init()
   {
      Dispatcher.addMessageListener(666, this)
   }

  public void handleMessage(Message msg)
  {

  }
}

Answer 6

The basic problem here is that a message is data, and you need to dispatch based on attributes of that data. So either you need to change the data object to dispatch to different handlers or you need to use a dispatch table in the listener. Either way, you need code to handle dispatch.

IMO, the message is the wrong place to implement dispatch. It is data, and behavior is the responsibility of the listener. And if you ever have two listeners, you'll want to use different dispatch tables.

For my own implementation, I'd probably use an if-else chain for up to five or six possible distinations. Above that, a functor map. These are just easier, and it's obvious what's happening.

However, if you really don't like the easy approaches, here's one that uses reflection:

public class Dispatcher {

    public static class Foo {}
    public static class Bar {}


    public void dispatch(Object obj) throws Exception {
        try {
            Method handler = this.getClass().getDeclaredMethod("handler", obj.getClass());
            handler.invoke(this, obj);
        }
        catch (Exception e) {
            System.out.println("couldn't determine handler for " + obj.getClass().getName());
        }
    }

    private void handler(Foo foo) {
        System.out.println("handler(Foo)");
    }

    private void handler(Bar bar) {
        System.out.println("handler(Bar)");
    }


    public static void main(String[] argv) throws Exception {

        Dispatcher dispatcher = new Dispatcher();

        dispatcher.dispatch(new Foo());
        dispatcher.dispatch(new Bar());
        dispatcher.dispatch("something else");

    }
}