Java ConcurrentModificationException with an ArrayList

I'm creating an online videogame using Java. If have a client and a server application. In the server, to handle the player database, I create an ArrayList named inGamePlayers, that contains Players object (with InetAdress ipAdress and String username).

When a Player Connects, it first check if the connecting player username == to one of the Players in the ArrayList username. If not, it adds him in the list. Else, the connecting player is considered 'reconnected'...

When It runs on Eclipse, it worked weird. So I decided to put a part of my code as a test in "Java Tutor" (It's a website that reads your code and shows you the variables, really handy when you start programming).

At the first

for (Player p : inGamePlayers) {

line, it stops and says java.util.ConcurrentModificationException, not the first but the second time it passes-by.

Here's my entire test code

import java.util.ArrayList;

public class YourClassNameHere {

    public static void main(String[] args) {

        ArrayList<Player> inGamePlayers = new ArrayList<Player>();
        inGamePlayers.add(new Player("Griff"));

        String newUsername = "Polak";

        for (Player p : inGamePlayers) {

            if (newUsername == p.username) {
                System.out.println(p.username+" reconnected!");
                break;
            }

            inGamePlayers.add(new Player(newUsername));
        }

        for (Player p : inGamePlayers) {
            System.out.println(p.username);
        }
    }
}

class Player {

    public String username;

    public Player(String username) {
        this.username = username;
    }
}

Solution

This code has multiple flaws. In this form it is going to add new Player instances until an entry with matching name is found. Starting an iteration, modifying a list, and continuing the iteration will throw a ConcurrentModificationException as you observed. Furthermore you compare strings with == instead of equals.

  for (Player p : inGamePlayers) {

    if (newUsername == p.username) { // This needs to be .equals
      System.out.println(p.username+" reconnected!");
      break;
    }
    // this line runs many times
    inGamePlayers.add(new Player(newUsername));
  }

Instead I suggest you to extract that code to a new function, and change the control flow:

private static void handleConnected(ArrayList<Player> inGamePlayers, String newUsername) {
  for (Player p : inGamePlayers) {
    if (newUsername.equals(p.username)) {
      System.out.println(p.username+" reconnected!");
      return; // return instead of break
    }
  }
  // we did not return, so this user is new
  inGamePlayers.add(new Player(newUsername));
}

// ...

public static void main(String[] args) {
  ArrayList<Player> inGamePlayers = new ArrayList<Player>();
  inGamePlayers.add(new Player("Griff"));

  String newUsername = "Polak";

  // Call this function in place of the old loop
  handleConnected(inGamePlayers, newUsername);

  for (Player p : inGamePlayers) {
    System.out.println(p.username);
  }
}