2

I am developing a client-server application in Java using Websocket. Currently, all the client messages are processed using switch-case as shown below.

@OnMessage
public String onMessage(String unscrambledWord, Session session) {
    switch (unscrambledWord) {
    case "start":
        logger.info("Starting the game by sending first word");
        String scrambledWord = WordRepository.getInstance().getRandomWord().getScrambledWord();
        session.getUserProperties().put("scrambledWord", scrambledWord);
        return scrambledWord;
    case "quit":
        logger.info("Quitting the game");
        try {
            session.close(new CloseReason(CloseCodes.NORMAL_CLOSURE, "Game finished"));
        } catch (IOException e) {
            throw new RuntimeException(e);
        }
    }
    String scrambledWord = (String) session.getUserProperties().get("scrambledWord");
    return checkLastWordAndSendANewWord(scrambledWord, unscrambledWord, session);
}

The server has to process more than 50 different requests from client and that results in more than 50 case statements. And in future, I expect it to grow. Is there any better way to process Websocket messages from client? Or, is this how it is usually done?

I read somewhere about the use of hashtable to avoid long switch-case scenario by mapping to function pointers. Is this possible in Java? Or, is there any better solutions?

Thanks.

Kalesh Kaladharan
  • 898
  • 1
  • 7
  • 22
  • Regarding `@endpoint...`, are you talking about javax.xml.ws.Endpoint? This rather looks web services and not websocket. AFAIK, websocket is only standard in Java EE environment (JSR 356). Other point, referring to method can be done via [Reflection](https://stackoverflow.com/a/161005/4906586) but IMHO, a long switch is easier to handle – Al-un Jun 05 '17 at 17:08
  • Its websocket only. If I am not wrong, framework like Spring uses @endpoint annotation to direct api requests to specific methods/classes. I thought, if I could get the underlying implementation of this annotation, I could build something similar. For eg, when client sends the request to login, I could just forward the request to specific method to perform the task without using any condition statements. – Kalesh Kaladharan Jun 05 '17 at 18:03
  • This is the annotation I was talking about. How @ServerEndpoint works?http://docs.oracle.com/javaee/7/api/javax/websocket/server/ServerEndpoint.html – Kalesh Kaladharan Jun 05 '17 at 18:10
  • You have plenty of good tutorials over the web such as [this](https://blog.openshift.com/how-to-build-java-websocket-applications-using-the-jsr-356-api/). You'll need to be more specific if you want a more accurate – Al-un Jun 06 '17 at 14:41
  • I have edited my code snippet. Please take a look at it. I hope its more clear to you now. The code is taken from the tutorial you suggested. Thanks for your efforts. – Kalesh Kaladharan Jun 07 '17 at 13:38

2 Answers2

0

As mentioned in the comments, one of websockets drawback is that you'll to specify the communication protocol yourself. AFAIK, the huge switch is the best option. To improve code readability and maintenance, I'll suggest to use encoders and decoders. Then, your problem becomes: how should I design my messages?

Your game looks like Scrabble. I don't know how to play Scrabble so let's take the example of card game with money. Let's assume you have three types of actions:

  1. Global action (join table, leave table ...)
  2. Money action (place bet, split bet, ...)
  3. Card action (draw card, etc)

Then your messages can look like

public class AbstractAction{
    // not relevant for global action but let's put that aside for the example
    public abstract void endTurn();
}

public class GlobalAction{
    // ...
}

public class MoneyAction{

    enum Action{
        PLACE_BET, PLACE_MAX_BET, SPLIT_BET, ...;
    }

    private MoneyAction.Action action;
    // ...
}

public class CardAction{
    // ...
}

Once your decoder and encoders are properly defined, your switch would be easier to read and easier to maintain. In my project, the code would look like this:

@ServerEndPoint(value = ..., encoders = {...}, decoders = {...})
public class ServerEndPoint{

    @OnOpen
    public void onOpen(Session session){
        // ...
    }

    @OnClose
    public void onClose(Session session){
        // ...
    }

    @OnMessage
    public void onMessage(Session session, AbstractAction action){

        // I'm checking the class here but you
        // can use different check such as a 
        // specific attribute 

        if(action instanceof GlobalAction){
            // do some stuff
        }

        else if (action instanceof CardAction){
            // do some stuff
        }

        else if (action instance of MoneyAction){
            MoneyAction moneyAction = (MoneyAction) action;
            switch(moneyAction.getAction()){
                case PLACE_BET:
                    double betValue = moneyAction.getValue();
                    // do some stuff here
                    break;
                case SPLIT_BET:
                    doSomeVeryComplexStuff(moneyAction);
                    break;
            }
        }

    }


    private void doSomeVeryComplexStuff(MoneyAction moneyAction){
        // ... do something very complex ...
    }

}

I prefer this approach because:

  1. The messages design can leverage your entities design (if you are using JPA behind)
  2. As messages are not plain text anymore but objects, enumerations can be used and enumerations are very powerful in this kind of switch-case situation. With the same logic but in a lesser extend, class abstraction can be useful as well
  3. The ServerEndPoint class only handles communication. The business logic is handled out of this class, either directly in Messages classes or in some EJB. Because of this split, code maintenance is much easier
  4. Bonus: @OnMessage method can be read as a summary of the protocol but details should not be displayed here. Each case must contain few lines only.
  5. I prefer avoid using Reflection: it'll ruin your code readability, in the specific scenario of websocket

To go further beyond code readability, maintenance and efficiency, you can use a SessionHandler to intercept some CDI event if this can improve your code. I gave an example in this answer. If you need a more advanced example, Oracle provides a great tutorial about it. It might help you to improve your code.

Graham
  • 6,577
  • 17
  • 55
  • 76
Al-un
  • 2,304
  • 1
  • 19
  • 37
  • I don't think this approach will help in my scenario. Even if I do something like this, there will be switch statements for each actions. I am looking for some methods to completely eliminate the switch and if statements. I'll try to get some information about hash tables and see if there are any approaches to avoid the long switch scenario.Thanks. – Kalesh Kaladharan Jun 10 '17 at 18:48
  • Thanks for your feedback. If you find a way to get rid of the switch, feel free to share it here. It may help other users who look for the same thing – Al-un Jun 10 '17 at 20:06
  • I've posted an answer. What is your opinion? Any improvements or suggestions? – Kalesh Kaladharan Jun 21 '17 at 07:11
  • I haven't thought about a `Map` but I agree with your conclusion (which was my first statement): when your options sounds aesthetics, it takes a toll on performance and above all code readability. Moreover, the approaches you mention sounds very naming convention dependent to me which is pretty. Globally speaking, not only for your question, I try to avoid using Reflection as much as possible – Al-un Jun 21 '17 at 16:51
0

After a bit of testing and study, I found two alternatives to avoid long switch case scenario.

  1. Anonymous class method (Strategy pattern)
  2. Reflection with Annotations

Using Anonymous Class

Anonymous class method is the norm and following code shows how to implement it. I used Runnable in this example. If more control is required, create a custom interface.

public class ClientMessageHandler {

    private final HashMap<String, Runnable> taskList = new HashMap<>();

    ClientMessageHandler() {

        this.populateTaskList();
    }

    private void populateTaskList() {

        // Populate the map with client request as key
       // and the task performing objects as value

        taskList.put("action1", new Runnable() {
            @Override
            public void run() {
                // define the action to perform.
            }
        });

       //Populate map with all the tasks
    }

    public void onMessageReceived(JSONObject clientRequest) throws JSONException {

        Runnable taskToExecute = taskList.get(clientRequest.getString("task"));

        if (taskToExecute == null)
            return;

        taskToExecute.run();
    }
}

Major drawback of this method is object creation. Say, we have 100 different tasks to perform. This Anonymous class approach will result in creating 100 objects for a single client. Too much object creation is not affordable for my application, where there will be more than 5,000 active concurrent connections. Have a look at this article http://blogs.microsoft.co.il/gilf/2009/11/22/applying-strategy-pattern-instead-of-using-switch-statements/

Reflection with Annotation

I really like this approach. I created a custom annotation to represent the tasks performed by methods. There is no overhead of object creation, like in Strategy pattern method, as tasks are performed by a single class.

Annotation

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)

public @interface TaskAnnotation {
    public String value();
}

The code given below maps the client request keys to the methods which process the task. Here, map is instantiated and populated only once.

public static final HashMap<String, Method> taskList = new HashMap<>();

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

    // Retrieves declared methods from ClientMessageHandler class 

    Method[] classMethods = ClientMessageHandler.class.getDeclaredMethods();

    for (Method method : classMethods) {            
        // We will iterate through the declared methods and look for
        // the methods annotated with our TaskAnnotation

        TaskAnnotation annot = method.getAnnotation(TaskAnnotation.class);

        if (annot != null) {                
            // if a method with TaskAnnotation is found, its annotation
            // value is mapped to that method.

            taskList.put(annot.value(), method);
        }
    }

    // Start server
}

Now finally, our ClientMessageHandler class looks like the following

public class ClientMessageHandler {

    public void onMessageReceived(JSONObject clientRequest) throws JSONException {

        // Retrieve the Method corresponding to the task from map
        Method method = taskList.get(clientRequest.getString("task"));

        if (method == null)
            return;

        try {
            // Invoke the Method for this object, if Method corresponding
            // to client request is found 

            method.invoke(this);
        } catch (IllegalAccessException | IllegalArgumentException
                | InvocationTargetException e) {
            logger.error(e);
        }
    }

    @TaskAnnotation("task1")
    public void processTaskOne() {

    }

    @TaskAnnotation("task2")
    public void processTaskTwo() {

    }

    // Methods for different tasks, annotated with the corresponding
    // clientRequest code
}

Major drawback of this approach is the performance hit. This approach is slow compared to Direct Method calling approach. Moreover, many articles are suggesting to stay away from Reflection, unless we are dealing with dynamic programming.

Read these answers to know more about reflection What is reflection and why is it useful?

Reflection performance related articles

Faster alternatives to Java's reflection

https://dzone.com/articles/the-performance-cost-of-reflection

FINAL RESULT

I continue to use switch statements in my application to avoid any performance hit.

Kalesh Kaladharan
  • 898
  • 1
  • 7
  • 22