should i use command pattern for multiple types of requests in TCP server code?

Question

Requirements: Implement a TCP server by socket API in java, the server could handle several clients at the same time. The server could add/remove ... items into/from a XML text file(as the data storage) The client could send command to server like "addItem"/"removeItem" .. If the client sent an "addItem", a new node should be added to the XML doc.

should command design pattern be used? If so, which should be command, receiver, invoker, client ?

My implementation is like below(also some questions in the comments):

interface Command{
    public void execute();
}

AddItemCommand implements Command{

    //The receiver
    XMLFileHelper xmlHelper;

    //The data need to added to XML file
    //This data should be here or not??
    ItemNode addedNode;

    public AddItemCommand(XMLFileHelper newHelper){
        xmlHelper = newHelper;
    }

    public void execute(){
        xmlHelper.addItemNode(addedNode);
    }

}

/*this class will handle all the xml doc operation: add, remove ...
i guess, it is should be the receiver
DataHelper in the parent interface, subclass could be XMLFileHelper,  DBHelper,         MessageQueueHelper ...
*/
public class XMLFileHelper implements DataHelper{

}

RemoveItemCommand implements Command(){
    //...............
}

/*This is the invoker, i am not sure what should it be, so i do not have a good name for it.*/
public class Invoker{
    Map<String, Command> map = new HashMap<String, Command>();

    public void addCommand(String cmdName, Command c){
        map.put(cmdName, c);
    }


    public void processRequest(String reqName){
        map.get(reqName).execute();
   }

}

TcpServerThread implements Runable{

    public void run(){
        DataHelper xmlPaser = new XMLFileHelper();
        Command cmd1 = new AddItemCommand();
        Command cmd2 = new RemoveItemCommand();

        Invoker invoker = new Involer();
        invoker.addCmd("add", cmd1);
        invoker.addCmd("remove", cmd2);

        String cmdName = getCommandName(socket.getInputStream());

        invoker.processRequest(cmdName);


    }
}

Answer 1

I am not sure what is the main question you are trying to ask but my intuition tells my that you are looking for some kind of a confirmation whether your approach is good or not.

So to answer this you should think what are requirements for this app. If you want TcpServer to be central point of your application which is the only side in this transaction that offers some services to clients plus you want centralized storage then this approach is perfectly fine. Your design allows you to easily test almoust all units of your code ( the only exception would be TcpServerThread which is doing too much in run method and it has hidden dependencies. I mean it initializes commands add them to Invoker and performs execution. I would split those to init and run logic and Invoker would be injected from outside. The job of invoker's setup would be performed somewhere else )

I think command pattern fits here very well. You could also consider changing your Command interface from:

interface Command{
  void execute();
}

to something like

interface Command{
  ResponseType execute( Parameters params );
}

ResonseType would give you ability to pass feedback to client. And execute with Parameters type injected would give you ability to manage request with parameters.

Now let's focus on Invoker class. You asked about if the name is good. What is the process I always follow when I try to figure out good descriptive name is that I am asking a question: "What is the main purpose of this class and what it is doing ?" In your case Invoker is storing commands ( a CommandRegister ?) in HashMap and also introduces another layer so TcpServer is not calling commands directly. So it feels like another responsibility for the class. Do you really need Invoker to be the one who processRequest ? Do you plan to perform some extra work before command is executed ? If no then maybe it would be good to replace Invoker with something like this:

class CommandRegister{
    Map<String, Command> map = new HashMap<String, Command>();

    public void addCommand(String cmdName, Command c){
        map.put(cmdName, c);
    }


    public Command getCommand(String reqName){
        return map.contains(reqName) ? map.get(reqName) : null;
   }
}

then your TcpServer would call execute directly. What is the benefit ? If you would like to refactor your execute and provide parameters to it you will have one spot less to modify. One spot less to test. And there will be no additional responsibility for the class. Also the name is now more descriptive. You can always have another layer in future that your TcpServer will use to execute commands. For Example:

class CommandInvoker{
  public CommandInvoker(CommandRegister cR){
    this.cR = cR;
  }

  public void execute(String cmdName){
     Command cmd = cR.get(cmdName);
     cmd.execute(); //if you decided like me to return null if there is no Command with specific name in register you should also check if cmd is not null before calling execute.
  }
}

But you can always stay with your Invoker if your app is a simple one and you don't plan to do extra work on the Command execution layer.

Let me know if something is not clear I'll try to clarify ;)

Answer 2

@Luke: Yes, i am looking for some confirmation. I am staring to learn command pattern in java, so i am not sure about if the command pattern should be used here. I am trying to google more real world example of command patters, but just found some very simple demos. So try to implements a simple TCP server. May be later i will try to watch source code of tomcat to check how they design and implement the server, if they use command pattern. So like you said, the command patter fits here. Thanks for that. But i am afraid that the construction of command and invoker objects should be put in the run method of the thread, because when the client try to connect, i plan to use one thread (with a socket obj) to handle all requests from that same client. So only the thread know which command the client sent. Adding return types for execute() could be good id.