Tautvydas Jalinskas
Tautvydas Jalinskas

Reputation: 129

Add something to arraylist while checking it

I need to check through an arraylist of "tiles" and if it finds a tile that its not used yet add 4 tiles around that tile but when i add something new while checking the list it crashes. How to add objects to arraylist without crashing it while checking it?

I add one tile and then check it:

private static List<Tile> tiles = new ArrayList<Tile>();



tiles.add(new Tile(mapsize/2*tilesize, mapsize/2*tilesize));

    for(Tile tile: tiles){
        if(!tile.spread){
            tile.spread=true;
            tiles.add(new Tile(tile.position.x-tilesize, tile.position.y)); //this line crashes it
        }
    }

Upvotes: 2

Views: 176

Answers (5)

wassgren
wassgren

Reputation: 19211

If you want another approach to the problem you can use Java 8 streams and implement it along the lines of:

List<String> strings = Arrays.asList("10", "20", "30");
final List<String> result = strings.stream().collect(ArrayList::new, (c, s) -> {
    if (s.equals("20")) {
        c.add("15"); // Here the result collection is modified
        c.add(s);
        c.add("25");
    } else {
        c.add(s);
    }
}, ArrayList::addAll);
System.out.println(result); // -> [10, 15, 20, 25, 30]

Upvotes: 1

Paul Gregoire
Paul Gregoire

Reputation: 9793

Use a concurrent collection or a Lock. Might I suggest CopyOnWriteArrayList or even better CopyOnWriteArraySet.

http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/CopyOnWriteArrayList.html

Edit: Here is an example using a reentrant lock

    private ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();

    private final Lock r = rwl.readLock();

    private final Lock w = rwl.writeLock();

    // estimated number of entries needed should be set on creation, 25 in this ex
    private static ArrayList<Tile> tiles = new ArrayList<Tile>(25);

    public boolean add(Tile tile) {
        boolean added = false;
        // get read lock
        r.lock();
        // do the tiles list checks and set a flag to add or not
        for (Tile t: tiles) {
            if (t.spread) {
                continue;
            }
            // flag the new tile
            tile.spread = true;
            break;
        }
        // release read lock
        r.unlock();
        // get write lock
        w.lock();
        try { 
            // add the tile
            added = tiles.add(tile);
            // add the four "surrounding" tiles here?

        } finally { 
            // release write lock
            w.unlock(); 
        }
        return added;
    }

    public void clear() {
        w.lock();
        try { 
            tiles.clear(); 
        } finally { 
            w.unlock(); 
        }
    }

Upvotes: 0

tom
tom

Reputation: 354

A ConcurrentModificationException is the error, some collections are designed to stop you from doing that since if it was allowed it would likely cause unexpected output and could potentially be unsafe and the item you just added to would also get iterated.

Like other people have mentioned it would be wise to add new items to a separate list and combine the lists at the end using built in Collections methods.

It would be allowed(Although ill advised) to iterate through a list with a for loop given the size of the collection. for instance:

ArrayList<String> arrlist = new ArrayList<String>();
    
    arrlist.add("foo");
    arrlist.add("bar");
    arrlist.add("foo");
    arrlist.add("bar");
    arrlist.add("foo");
    
    for(int i = 0; i < arrlist.size(); i++){
        if(arrlist.get(i).equals("foo")){
            arrlist.add("bar");
        }
        System.out.println(arrlist.get(i));
    }

The output would also print 3 bars after the last foo since the items that get added when the element is foo also get iterated in the instance above this would be fine but bad practice.

But ConcurrentModificationExceptions do exist for a reason and to use code such as above would be bad practice.

Upvotes: 0

fge
fge

Reputation: 121710

when i add something new while checking the list it crashes

With a ConcurrentModificationException. It is expected. Apart from using an Iterator's .remove() you cannot modify a list like you do (not a non concurrent one like ArrayList anyway).

You should create a new list in which you add your new tiles, then when you're done iterating, addAll this new list to the existing list:

final List<Tile> newTiles = new ArrayList<>();

for (final Tile tile: tiles) {
    if (tile.spread)
        continue;
    tile.spread = true;
    newTiles.add(new Tile(tile.position.x - tilesize, tile.position.y));
}

tiles.addAll(newTiles);

Of course, the code above supposed that your code is not called from several threads simultaneously! Or that this code is in a method protected by a lock...

Upvotes: 0

Nir Alfasi
Nir Alfasi

Reputation: 53525

Iterating a list and modifying it simultaneously causes a ConcurrentModificationException. In order to avoid that, you may add the new items into a new List and when you're done traversing the original list - combine both lists into the original. For example:

private static List<Tile> tiles = new ArrayList<Tile>();
private static List<Tile> tmpTiles = new ArrayList<Tile>();

tiles.add(new Tile(mapsize/2*tilesize, mapsize/2*tilesize));

for(Tile tile: tiles){
    if(!tile.spread){
        tile.spread=true;
        tmpTiles.add(new Tile(tile.position.x-tilesize, tile.position.y));
    }
}
tiles.addAll(tmpTiles);

Upvotes: 3

Related Questions