CODEWITHSUNDEEP
javaiteration
user658168
user658168

Reputation: 1253

Iteration over a list (ConcurrentModificationException)

The following code throws a ConcurrentModificationException:

for (String word : choices) {
         List<String> choicesCopy = choices;
         chosen.add(word);
         choicesCopy.remove(word);
         subsets(choicesCopy, chosen, alreadyPrinted);
}

What's going on? The original list (choices) isn't modified at all.

Upvotes: 1

Views: 3288

Answers (6)

klawx3
klawx3

Reputation: 11

I think the universal solution is:

List<E> obj = Collections.synchronizedList(new ArrayList<E>());

Upvotes: 1

Manoj
Manoj

Reputation: 5612

You made a reference copy not object copy in here

List<String> choicesCopy = choices;

So obviously you are modifying the same list and you are bound to get the ConcurrentModificationException

Use Collections.copy() to properly make a copy of your list.

EDIT: As suggested below you can also use constructor for copying.

Upvotes: 8

Peter Lawrey
Peter Lawrey

Reputation: 533492

I suspect chosen should be a copy as well. Otherwise chosen will accumulates all the words by the time the loop has finished. i.e. I suspect the chosen and choices shouldn't have any words in common.

I also suspect the collections should be sets (unordered collections without duplicates) instead of lists.

Perhaps the code should be.

Set<String> choices =
Set<String> chosen =
for (String word : choices) {
     Set<String> choicesCopy = new LinkedHashSet<String>(choices);
     choicesCopy.remove(word);
     Set<String> chosenCopy = new LinkedHashSet<String>(chosen);
     chosenCopy.add(word);

     subsets(choicesCopy, chosenCopy, alreadyPrinted);
}

Upvotes: 0

Sean Patrick Floyd
Sean Patrick Floyd

Reputation: 298838

Change the code like this:

for (Iterator<String> it = choices.iterator(); it.hasnext();) {
     String word = it.next();
     chosen.add(word);
     it.remove();
     subsets(choicesCopy, chosen, alreadyPrinted);
}

Explanation: foreach loops use an iterator internally, but don't expose it to the user. So if you want to remove items you have to simulate the foreach loop and keep a reference to the iterator yourself.

While iterating, any other means of removing data from a collection will result in a ConcurrentModificationException.

Upvotes: 1

RMT
RMT

Reputation: 7070

The reason is because you cannot modify anything inside a foreach loop. Try using a for loop. Or you have take all the contents of list and add them 1 at a time to the other list. because its done by reference

Edit: You need to make a deep copy of the list and remove from that copy. Then you can assign the reference of the original list to point to the new one that has the modifications. You cannot remove from the list you're currently iterating through even if it's being referenced by another variable.

Upvotes: 1

planetjones
planetjones

Reputation: 12633

You'll need to copy the list properly e.g. Collections.copy and then remove from the copy, or use Iterator.remove, which will remove the Object from the underlying collection. Iterators are fail fast, so you can't change the underlying Collection without using the API of the Iterator.

Upvotes: 0

Related Questions