Why are my getters returning null when I try to send to the server?

Question

I know this question has been asked a countless number of times on here, and I've searched SO and other sources for a solution but I just can't resolve the error. I have a getter and setter for my ClientPlayer class attribute, the setter is called in the GUI when a certain button is called, and I would like to use the getter after a client connects and send the object to the server. Calling the method "this.client.sendTCP(clientPlayer.getPlayerName());" in the ClientController returns a nullPointerException.

Error: "JavaFX Application Thread" java.lang.IllegalArgumentException: object cannot be null.

My guess would be that I create a new instance of ClientPlayer one time too many thus returning null as my string playerName is originally set to none. However, I am not sure on how to go about solving the problem. I would really appreciate any help whatsoever.

public class ClientPlayer implements Serializable {

public ClientPlayer() {

}

private String playerName;

public void setPlayerName(String playerName) {
    this.playerName = playerName;
   }

public String getPlayerName() {
    return this.playerName;
   }
}

Below the relevant part of my GUI-code where I set a button to connect the to the server:

    ClientPlayer clientPlayer = new ClientPlayer();  

    clientToGame.setOnAction((ActionEvent w) -> {
         clientPlayer.setPlayerName(clientNameText.getText());
         clientController.connect();
         window.setScene(lobbyScene);
    });

Here is the bit of my client-class where I try to get the name and send to server:

    public class ClientController() {
     ClientPlayer clientPlayer = new ClientPlayer();

    public void connect() {
    if (client.isConnected()) {
        Logger.getLogger(getClass().getName()).log(Level.INFO, "You are      already connected to :{0}", config.getHost());
        return;
    }
    this.client.start();
    try {
        this.client.connect(5000, config.getHost(), config.getTCPPort());
        System.out.println("Successfully connected to " + config.getHost());
        this.client.sendTCP(clientPlayer.getPlayerName());

    } catch (IOException ex) {
        Logger.getLogger(getClass().getName()).log(Level.SEVERE, "Server connection failed: {0}", ex.getMessage());
        throw new RuntimeException(ex);
    }

    MessageRegistry.registerMessages(client.getKryo());
    this.client.addListener(listener);
  }
 }

Is `clientToGame.setOnAction...` inside the `ClientController` class? — Tobias G, Feb 01 '17 at 14:54
The getter here return a value only if the setter initialize your `String playerName` and as your variable `playerName` is not initialized so it returns a null ! — Bo Halim, Feb 01 '17 at 14:55
no it's not- It's the GUI class where i get the name form a textField. — cndolo, Feb 01 '17 at 14:56
At no point do you ever call `setPlayerName()` on the `ClientPlayer` you create in `ClientController`. — James_D, Feb 01 '17 at 14:58
Sorry, could you please explain that again? I didn't quite get that, I don't use setPlayerName() anywhere in the ClientController. I use the getter. And I know that the string is initialised to null as I do not do it myself, but wouldn't calling setPlayerName(String name) in the GUI overwrite that? — cndolo, Feb 01 '17 at 15:01
In the GUI you are calling `setPlayerName()` on a completely different object. — James_D, Feb 01 '17 at 15:02
I know, but I don't know how else to call methods of the ClientPlayer-class without creating a new instance of the class each time. — cndolo, Feb 01 '17 at 15:07

score 2 · Accepted Answer · answered Feb 01 '17 at 15:09

In your GUI, you create a ClientPlayer instance and call setPlayerName() on it. Then in ClientController, you create a new ClientPlayer instance (on which you never call setPlayerName()), and call getPlayerName() on it. Since you never set the player name for that instance, getPlayerName() of course returns null.

You need to decide whose responsibility it is to "own" the ClientPlayer instance. If it is the responsibility of the ClientController, then either add a getClientPlayer() method to ClientController, and do

clientToGame.setOnAction((ActionEvent w) -> {
     clientController.getClientPlayer().setPlayerName(clientNameText.getText());
     clientController.connect();
     window.setScene(lobbyScene);
});

and remove the ClientPlayer entirely from your GUI class. If it is the responsibility of the GUI class to own it, then pass a reference to it to the connect() method, and remove the ClientPlayer field from the controller:

public class ClientController() {
     // ClientPlayer clientPlayer = new ClientPlayer();

    public void connect(ClientPlayer clientPlayer) {
    if (client.isConnected()) {
        Logger.getLogger(getClass().getName()).log(Level.INFO, "You are      already connected to :{0}", config.getHost());
        return;
    }
    this.client.start();
    try {
        this.client.connect(5000, config.getHost(), config.getTCPPort());
        System.out.println("Successfully connected to " + config.getHost());
        this.client.sendTCP(clientPlayer.getPlayerName());

    } catch (IOException ex) {
        Logger.getLogger(getClass().getName()).log(Level.SEVERE, "Server connection failed: {0}", ex.getMessage());
        throw new RuntimeException(ex);
    }

    MessageRegistry.registerMessages(client.getKryo());
    this.client.addListener(listener);
  }
 }

and of course

clientToGame.setOnAction((ActionEvent w) -> {
     clientPlayer.setPlayerName(clientNameText.getText());
     clientController.connect(clientPlayer);
     window.setScene(lobbyScene);
});

Tobias G · Answer 2 · 2017-02-01T15:09:30.067

Ugly but should work:

Make the ClientPlayer access static. In your gui class:

public static ClientPlayer clientPlayer = new ClientPlayer();

then you can access that very same object in your ClientController via:

...
this.client.sendTCP(MyGuiClass.clientPlayer.getPlayerName());
...

Edit: The thing is, you do not reference the "same" object via clientPlayer in your ClientController as you do in GUI-Class. You should somehow hand that object over or use a static reference. You could also hand it over in the constructor of ClientController.

Edit2:

How you should do it

ClientPlayer clientPlayer = new ClientPlayer();  

    clientToGame.setOnAction((ActionEvent w) -> {
         clientPlayer.setPlayerName(clientNameText.getText());
         clientController.connect(clientPlayer);
         window.setScene(lobbyScene);
    });

In your ClientController:

public void connect(ClientPlayer player) {
    this.clientPlayer = player;
    if (client.isConnected()) {
        Logger.getLogger(getClass().getName()).log(Level.INFO, "You are already connected to :{0}", config.getHost());
        return;
    }
    this.client.start();
    try {
        this.client.connect(5000, config.getHost(), config.getTCPPort());
        System.out.println("Successfully connected to " + config.getHost());
        this.client.sendTCP(player.getPlayerName());

    } catch (IOException ex) {
        Logger.getLogger(getClass().getName()).log(Level.SEVERE, "Server connection failed: {0}", ex.getMessage());
        throw new RuntimeException(ex);
    }

    MessageRegistry.registerMessages(client.getKryo());
    this.client.addListener(listener);
}

My am I referring to the gui-class now? I want the GUI to pass the name to the ClientPlayer and set it then get it from there. I know I'm not referencing the same object, but I don't know how else to call the class methods without instantiating the classes. — cndolo, Feb 01 '17 at 15:06
This is horrible. Don't make fields public, or static, and certainly not both, as a hack because your OO design is not right. — James_D, Feb 01 '17 at 15:06
@James_D if `clientPlayer` only holds the name of the local player, this would be a reason to make it static. The local player only exists once and it doesn't hurt to access the name from anywhere. Or am I wrong? — Tobias G, Feb 01 '17 at 15:15
What if you decide later you want two instances of your GUI class (say in two different windows, or two tabs, or whatever). If you made fields static in there simply for your convenience, you are then completely screwed.The `static` keyword is about *scope*, it has absolutely nothing to do with getting access to something. — James_D, Feb 01 '17 at 15:29
@James_D yeah you're right, but for me personally, it would be clear that "my game" only has one gui, so it would choose the convenient static approach. Anyways, thank you for your tips! — Tobias G, Feb 02 '17 at 07:23

Why are my getters returning null when I try to send to the server?

2 Answers2

How you should do it