Removing elements from an ArrayList

Question

In this program I keep a list of duelists that kill the best duelist when their random int = 0. I am having trouble with my loop near the bottom and am getting this error:

Exception in thread "main" java.util.ConcurrentModificationException
at java.util.AbstractList$Itr.checkForComodification(Unknown Source)
at java.util.AbstractList$Itr.next(Unknown Source)
at Duelist.main(Duelist.java:74)

Code

import java.util.Arrays;
import java.util.Random;
import java.util.ArrayList;
import java.util.Collections;

public class Duelist implements Comparable{

private String name;
private int chance;
private boolean alive;


public Duelist(String duelistName, int hitChance){
    alive = true;
    name = duelistName;
    chance = hitChance;

    }
public String getName(){
    return name;
}
public int getChance(){
    return chance;
}
public boolean getLife(){
    return alive;
}
public void kill(){
    alive = false;
}

 public int compareTo(Object anotherDuelist) throws ClassCastException {
        if (!(anotherDuelist instanceof Duelist))
          throw new ClassCastException("A Duelist object expected.");
        int anotherDuelistChance = ((Duelist) anotherDuelist).getChance();  
        return this.chance - anotherDuelistChance;    
      }

public static void main(String[] args){

ArrayList<Duelist> duelers = new ArrayList<Duelist>(5);
//ArrayList<Duelist> personToKill= new ArrayList<Duelist>(5);
ArrayList<Duelist> rank = new ArrayList<Duelist>(5);
Random generator = new Random();




Duelist Antoine = new Duelist("Antoine", 3);
Duelist Francis = new Duelist("Francis", 6);
Duelist Juliette = new Duelist("Juliettee", 1);

duelers.add(Antoine); duelers.add(Francis); duelers.add(Juliette);



//System.out.println(duelers);


//for(Duelist element : duelers){
//  System.out.println(element.getName());
//  System.out.println(element.getChance());
//}
Collections.sort(duelers);
Collections.reverse(duelers);
//for(Duelist element : duelers){
    //System.out.println(element.getName());
    //System.out.println(element.getChance());
//}

while(duelers.size() > 1){


    for(Duelist element : duelers){



            System.out.println(element.getName());
            System.out.println("Chance:" + element.getChance());
            int randomInt = generator.nextInt(element.getChance());
            System.out.println("Random int " + randomInt);



            //Destroy target if randomInt equals 0
            if (randomInt == 0){


                //Check to make sure the best duelist is not killing themselves
                if (element.equals(duelers.get(duelers.size()-1))){
                    System.out.println("LASTDUDE");
                    Duelist bestDueler = duelers.get(duelers.size()-2);
                    bestDueler.kill();
                    rank.add(element);
                    duelers.remove(bestDueler);
                }

                else {
                    System.out.println("Killshot");
                    Duelist bestDueler = duelers.get(duelers.size()-1);
                    bestDueler.kill();
                    rank.add(element);
                    duelers.remove(bestDueler);
                }

            }


    }
}
System.out.println(duelers);


}
}

Please post a *shorter* example, without all the commented-out lines, without lots of areas of multiple blank lines, with proper indentation. I'm sure you could come up with an example which takes so little space it doesn't even need to scroll when viewed in SO... — Jon Skeet, Mar 16 '12 at 21:06
possible duplicate of [ConcurrentModificationException for ArrayList](http://stackoverflow.com/questions/3184883/concurrentmodificationexception-for-arraylist) — CoolBeans, Mar 16 '12 at 21:08

Sean Patrick Floyd · Accepted Answer · 2012-03-16T22:16:14.757

for(Duelist element : duelers){

While you are inside this block, you are iterating over the duelers list, which means that you can't change the list without causing a ConcurrentModificationException unless you use the iterator's remove() method.

The problem here is, you don't have access to the iterator. So you'll have to change your foreach loop to a while loop with an iterator:

Iterator<Duelist> it = duelists.iterator();
while(it.hasNext()){
    Duelist element = it.next();
    // continue as before

(update after comments) or better:

for(Iterator<X> it=x.iterator();it.hasNext();){
    Duelist element = it.next();
    //continue as before

and instead of

duelers.remove(element);

write

// the problem here is: you can only remove the current element,
// so you'll have to re-think some of your code
it.remove();

Or: you could create a copy of your list for iterating. That's a waste of memory (and a code smell) but if your application is small it should not make much of a difference, and it would require the least amount of rewriting on your part.

for(Duelist element : new ArrayList<Duelist>(duelers)){
// continue as above

You should use a for loop with the Iterator to limit the scope of the Iterator. — Steve Kuo, Mar 16 '12 at 21:27
@Steve true, but I hate the looks of a `for(Iterator it=x.iterator();it.hasNext();)` loop where the last segment is empty. But it's better encapsulation, I agree — Sean Patrick Floyd, Mar 16 '12 at 21:30

score 1 · Answer 2 · answered Mar 16 '12 at 21:10

1

You cannot modify a collection within a foreach loop, instead use an Iterator and when you want to remove something, use Iterator.remove():

answered Mar 16 '12 at 21:10

AdrianS

1,980
7
33
51

score 0 · Answer 3 · answered Mar 16 '12 at 21:09

The problem is this line duelers.remove(bestDueler);

You are not supposed to modify the list while you iterate over it and not use the iterator
You are implicitely using an iterator by for(Duelist element : duelers)
Use an iterator explicitily i.e. duelers.listIterator() and use the remove of iterator instead

Removing elements from an ArrayList

3 Answers3