Java How to add to an array list while looping

Question

package biz.boulter.state;

import java.awt.Color;
import java.awt.Graphics2D;
import java.util.ArrayList;

import biz.boulter.sword.Game;
import biz.boulter.sword.Particle;

public class Menu implements State{
private ArrayList<Particle> particles = new ArrayList<Particle>();
boolean inLoop = false;

public Menu(){

}

@Override
public void render(Graphics2D g) {
    if(!inLoop){
        for(Particle p: particles){
            p.render(g);
        }
    }
}

@Override
public void tick() {
    if(!inLoop){
        for(Particle p: particles){
            p.tick();
        }
    }
}

@Override
public void keyPressed(int kc) {

}

@Override
public void keyReleased(int kc) {

}

@Override
public void mousePressed(int button, int x, int y) {
    for(int i = 0; i<500; i++){
        int rand;
        if(Game.rand.nextBoolean()){
            rand = Game.rand.nextInt(10)-11;
        }else{
            rand = Game.rand.nextInt(10)+1;
        }
        particles.add(new Particle(x, y, rand, Game.rand.nextInt(10)-11, new Color(Game.rand.nextInt(1000000000))));
    }
}

@Override
public void mouseReleased(int button, int x, int y) {

}
}

Hey guys this is my code and i keep getting ConcurrentModificationException. And i know this means that two things are changing the particles variable at the same time. But how else am i supposed to add to the array list. I saw some forums that said use Iterator but that is for removing not adding.

Thanks in advance!

EDIT:

stacktrace:

Exception in thread "Thread-2" java.util.ConcurrentModificationException
    at java.util.ArrayList$Itr.checkForComodification(Unknown Source)
    at java.util.ArrayList$Itr.next(Unknown Source)
    at biz.boulter.state.Menu.render(Menu.java:21)
    at biz.boulter.sword.Game.render(Game.java:43)
    at biz.boulter.sword.Game.run(Game.java:136)
at java.lang.Thread.run(Unknown Source)

particle class:

package biz.boulter.sword;

import java.awt.Color;
import java.awt.Graphics2D;
import java.awt.Rectangle;

public class Particle {
private double x = 0;
private double y = 0;
private double xa = 0;
private double ya = 0;
private Color particleColour;
private Rectangle img;

public Particle(int x, int y, int xa, int ya, Color colour){
    this.x = x;
    this.y = y;
    this.xa = xa;
    this.ya = ya;
    this.particleColour = colour;
    img = new Rectangle(x, y, 5, 5);
}

public void render(Graphics2D g){
    img.setBounds((int)Math.round(x), (int)Math.round(y), 5, 5);
    g.setColor(particleColour);
    g.fill(img);
}

public void tick(){
    ya+=0.5;

    if(xa < 0){
        xa+=1;
    }

    if(xa > 0){
        xa-=1;
    }

    x+=xa;
    y+=ya;
}
}

Answer 1

Just to avoid concurent problems and ConcurentModificationException in iterations over collections, you have to use concurrent collection classes.

For ArrayList, for example, you have can try with CopyOnWriteArrayList which is threadsafe variant of ArrayList.

More details about this read here: http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/CopyOnWriteArrayList.html

Answer 2

You have 2 options. If you could, mark the methods tick, render and mousePressed as synchronized. This will solve the issue but though this might not be the best solution if there are insertions at a faster rate.

The other solution would be to change

private ArrayList<Particle> particles = new ArrayList<Particle>(); 

to

private LinkedBlockingQueue<Particle> particles = new LinkedBlockingQueue<Particle>();

This works because, as per documentation of iterator method:

The returned iterator is a "weakly consistent" iterator that will never throw ConcurrentModificationException, and guarantees to traverse elements as they existed upon construction of the iterator, and may (but is not guaranteed to) reflect any modifications subsequent to construction.

Answer 3

The simplest thing to do is to lock the collection when you are using it.

synchronized(particles) {
    for(Particle p: particles){
        p.tick();
    }
}

and

Particle p = new Particle(x, y, rand, Game.rand.nextInt(10)-11, new Color(Game.rand.nextInt(1000000000)))
synchronized(particles) {
    particles.add(p);
}

This way you ensure access is not concurrent.

The problem with CopyOnWriteArrayList is that writes are expensive. As the name suggests, it takes a copy of the entire list on every update.

The problem with BlockingQueue is it is not designed for iteration. It will work but not as efficiently.

Answer 4

you are trying to change the internal state of the Array, while looping, this is the expected behavior.

One quick (but comes to some perfomance cost) is to use then CopyOnWriteArrayList, see linked entry, In what situations is the CopyOnWriteArrayList suitable?