How to decouple data and behavior objects in java?

Question

I am building a multiplayer game using Java for server. Currently, I am using a single class file to store player data and process the data. I am a beginner, so I had no idea that this is a bad practice. http://howtodoinjava.com/best-practices/5-class-design-principles-solid-in-java/ This article helped me to understand I am breaking the rule of 'Single Responsibility principle'.

This is how my code looks now.

public class PlayerSession{

    String playerId;
    String playerName;

    // 20+ player data fields, which I am trying to reduce
    // and keep only the most used data

    public void messageProcessor(JSONObject clientRequest) throws JSONException{

        switch(clientRequest.getString("task")){            
        case "login": loginProcess(); break;
        case "logout": logoutProcess(); break;

        //50+ different actions 
        }
    }

    public void populateSessionData(String playerId){
        // populate player data from database
    }

    private void loginProcess(){
        //Process login
    }

    private void logoutProcess(){
        //Process logout
    }

    //20+ other methods which do entirely different tasks.
}

As we add more features, the class will become extremely difficult to maintain and modify. Now I am trying to decouple this class into two different class. One, just to store Player data and an another one to handle behavior as shown below.

public class PlayerSession {

    final TaskHandler taskHandler = new TaskHandler();

    public void messageProcessor(JSONObject clientRequest) throws JSONException {

        switch (clientRequest.getString("task")) {
        case "login":
            taskHandler.loginProcess();
            break;
        case "logout":
            taskHandler.logoutProcess();
            break;

        // 50+ different actions
        }
    }
}

public class PlayerData {

    String playerId;
    String playerName;

    // 20+ player data fields, which I am trying to reduce
    // and keep only the most used data

    public void populateSessionData(String playerId) {
        // populate player data from database
    }
}

public class TaskHandler {

    final PlayerData player = new PlayerData();

    private void loginProcess() {
        // Process login
    }

    private void logoutProcess() {
        // Process logout
    }

    // 20+ other methods which do entirely different tasks.
}

And this design results in 2 additional object creation for a single client i.e, PlayerData and TaskHandler. For a server of 10,000 concurrent client, will this become an issue? Is this the right way to do it? If not, what is the best approach for a scenario like this?

Somewhere I read that objects just to save data is not a good approach. Is that right?

Answer 1

You need to do a lot things here:

PlayerSession class:

Why do you parse JsonObject here? you need to create class that named ClientRequest and all work will be done inside it. Place all you need to want from client request to this class and only that methods that will be give ready to use something don't pull data from object and calculate manually in your code this is procedural way. Also rename PlayerSession to Session.

PlayerData class

First of all rename it to Player class it is represent Player don't Player data. Don't do any database relation actions in Player. Also after creation you need to have ready to use Player, here you create instance and after that populate it with data, better to do it in constructor.

TaskHandler class

Don't create class that do many thing, btw you almost in right way, simple create interface Task or Action that will be have only one method performTask() and create many implementations as LoginTask, LogoutTask etc. Maybe you need to have fabric that give you instance of particular action also that help to get rid of manually create concrete implementations and you will be do it in more polymorphic way.

public class Session {

    private final SessionActions actions;
    private final RequestFabric requests;
    private final Player player;

    public Session (SessionActionFabric actionsFabric,
                    RequestFabric requests, Player player) {
        this.actionsFabric = actionsFabric;
        this.requests = request;
        this.player = player;
    }

    void login() throws LoginException {
        Request request = request.createRequest();
        SessionAction login = actions.createActions("login", player, request);
        login.performAction();
        //something like that it's depends on whole picture of your project and maybe changed
    }

    //etc.
}

public interface Request {

    public performRequest(Player player, /*request data your api specs here*/) throws RequestException;
}

public class Player {

    private String id;
    private String name;

   public Player(String id, String name){
       this.id = id;
       this.name = name;
   }

   //getters, setters 
}

interface SessionAction {

    void performAction() throws SessionActionException;
}

class LoginAction implements SessionAction {
    private final Player player;
    private final Request request;


    LoginAction (Player player, Request request) {
        this.player = player;
        this.request = request;
    }

    void performAction() {
        // do you things here
    }

}

Q For a server of 10,000 concurrent client, will this become an issue? Is this the right way to do it? If not, what is the best approach for a scenario like this?

A Don't mind about performance best pay attention to good design, if you have issues with performance you almost always find the way to improve it with good design (caching, pooling, etc.)

QSomewhere I read that objects just to save data is not a good approach. Is that right?

A you are righ, this calls anemic model, (data and methods to process it are separate) but in present it very popular.

Answer 2

Based on your discussion with Joy. If you are worried that Action class like LoginAction will be created every 2 seconds. Use singleton pattern, have static final instance of action class and one static getter for that instance (LoginAction.getInstanceOfLoginAction()) and use it every time instead of creating new ones. But make sure your action classes are stateless not stateful!!

class LoginAction implements SessionAction {
  private static final LoginAction loginAction = new LoginAction();
  public static LoginAction getLoginAction() {
      return loginAction;
  }

  void performAction(Player player, Request request) {
    // do you things here
  }

}

One more thing use a factory(see factory pattern) for getting implementation of Action interface as per request.