Integer won't increment?

Question

Okay, so I have a client/server test going on, and I am passing the Integer playerID to a thread where it gives the int value to a simple Player object, than increments playerID by 1.

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

        Vector<Player> player = new Vector<Player>();

        SlickServer ss = new SlickServer();
        ss.setVisible(true);

        ServerSocket serverSocket = new ServerSocket(4444);
        boolean listening = true;

        Integer playerID = new Integer(0);

        while(listening){
            ss.textArea.append("Waiting to connect with player: " + playerID.intValue()  + "\n");
            new ClientThread(serverSocket.accept(), player, playerID, ss.textArea).start();
            ss.textArea.append("Waiting to connect with player: " + playerID.intValue() + "\n");
        }

        serverSocket.close();
        System.exit(0);
    }

and here's where it increments it in the thread:

public ClientThread(Socket acceptedSocket, Vector<Player> players, Integer playerID, JTextArea textArea){
        super("ClientThread");
        this.acceptedSocket = acceptedSocket;
        this.players = players;
        players.add(new Player(50,50, playerID.intValue()));

        if(players != null)
            System.out.println("Not Null: " + players.size());

        boolean b = false;
        for(int i = 0; i < players.size(); i++){
            if(!b){
                if(players.get(i).id == playerID){
                    me = players.get(i);
                    b = true;
                }
            }
        }

        playerID = new Integer(playerID.intValue() + 1);
        this.textArea = textArea;
    }

Reading http://stackoverflow.com/questions/40480/is-java-pass-by-reference may give you some insight as to why this doesn’t do what you expect. — Josh Lee, Sep 28 '10 at 17:14
If you want to know a bit more about data sharing between threads I'd recommend Java Concurrency in Practice. In short, if you don't explicitly design variables for sharing you never know what you get. — extraneon, Sep 28 '10 at 17:51

Richard Cook · Accepted Answer · 2010-09-28T17:56:09.977

7

new Integer is creating a brand-new Integer instance inside the client thread method which is not available to the caller.

However, you need to consider synchronization between the main and client thread. This can be achieved using synchronized statements for nontrivial objects or classes such as java.util.concurrent.atomic.AtomicInteger for integers as follows:

AtomicInteger playerID = new AtomicInteger(0);
while (listening) {
  ss.textArea.append("Waiting to connect with player: " + playerID.get()  + "\n");
  new ClientThread(serverSocket.accept(), player, playerID, ss.textArea).start();
  ss.textArea.append("Waiting to connect with player: " + playerID.get() + "\n");
}

class ClientThread {
  public ClientThread(Socket acceptedSocket, Vector<Player> players, AtomicInteger playerID, JTextArea textArea) {
    // etc.
    playerID.incrementAndGet();
    // etc.
  }
}

You need to think about how to share data between concurrently executing threads. This applies also to the Vector<Player> and JTextArea arguments. You should wrap accesses to the players and textArea objects using synchronize statements as appropriate.

edited Sep 28 '10 at 17:56

answered Sep 28 '10 at 17:10

Richard Cook

32,523
5
46
71

`ClientThread` shouldn't be responsible for incrementing the ID, period. – ColinD Sep 28 '10 at 17:17
Because this isn't the solution to his problem that he should be using. Obviously he needs to learn about how Java works, but in this case the root of his problem is that he's approaching how to increment his player ID in the wrong way and putting that responsibility in the wrong place. – ColinD Sep 28 '10 at 17:23
1

I really don't understand why you're doing all this shared data synchronization stuff when this is a simple matter of incrementing in the place where the variable is defined. Plus, you don't need synchronization if it's only being incremented in the constructor, which is called on a single thread. Plus, the synchronization wouldn't help even if it was needed since you can't do an atomic increment using your `SharedData`... an `AtomicInteger` would be better. – ColinD Sep 28 '10 at 17:41
1

Don't use lock objects for integers. Just use AtomicInteger. That's what it's there for. – extraneon Sep 28 '10 at 17:49

ColinD · Answer 2 · 2010-09-28T17:19:02.673

2

Increment the player ID in main after creating the ClientThread.

The client thread should not be responsible for incrementing the player ID. This is the responsibility of main, which is the one creating client threads and giving them their IDs.

edited Sep 28 '10 at 17:19

answered Sep 28 '10 at 17:13

ColinD

108,630
30
201
202

score 0 · Answer 3 · edited Aug 20 '12 at 02:45

0

Try use IntHolder if you need it.

edited Aug 20 '12 at 02:45

Shashank Kadne

7,993
6
41
54

answered Sep 28 '10 at 17:13

Stan Kurilin

15,614
21
81
132

2

AtomicInteger is designed just for these cases. – extraneon Sep 28 '10 at 17:52

Kevin Williams · Answer 4 · 2010-09-28T17:46:34.257

0

If you want to manipulate the integer within the method you need to encapsulate it within an object.

Read this article for a better understanding http://www.javaworld.com/javaworld/javaqa/2000-05/03-qa-0526-pass.html

edited Sep 28 '10 at 17:46

answered Sep 28 '10 at 17:19

Kevin Williams

2,588
3
24
25

2

Java does not pass objects by reference. It passes references by value. There's a world of difference. – dty Sep 28 '10 at 17:25
Thanks @dty, I've removed the erroneous details. – Kevin Williams Sep 28 '10 at 17:47

Integer won't increment?

4 Answers4