concurrentexception i don't understand where it's the error

Question

I receive this exception

Exception in thread "Thread-3" java.util.ConcurrentModificationException
at java.util.LinkedList$ListItr.checkForComodification(LinkedList.java:761)
at java.util.LinkedList$ListItr.next(LinkedList.java:696)
at ServerMultiThread.run(ServerMultiThread.java:89)
at java.lang.Thread.run(Thread.java:680)

from this code:

            synchronized(itemsList)
            {

                if(itemsList.isEmpty())
                {
                    item.initCounter();
                    itemsList.add(item);
                    pw.println("It's the first time you connect to server!");
                }
                else
                {

                    for(ItemClient itm : itemsList)
                    {

                    if(itm.equals(item))
                    {
                        int value = itm.getCounter();
                        value++;
                        itm.setCounter(value);
                        pw.println(itm.getCounter());

                    }
                    else
                    {
                        item.initCounter();
                        itemsList.add(item);
                        pw.println("It's the first time you connect to server!");   
                    }
                    }
                }
    }

the row 89 corresponds to this for(ItemClient itm : itemsList). Why I receive this error?

possible duplicate of [Java: adding elements to a collection during iteration](http://stackoverflow.com/questions/993025/java-adding-elements-to-a-collection-during-iteration) — dogbane, Jan 26 '12 at 09:28
I'd say your algorithm is flawed. You are adding a new item into the list for every item in the list that does not match the new one. And if you fix this bug (by adding some boolean field `isInList`) - that will fix this error too. — bezmax, Jan 26 '12 at 09:32
@Mazzy: Moreover, you can remove that loop and write `if (itemsList.contain(item)) incrementCounter() else initCounter()` instead. Read about `contain` method here: http://docs.oracle.com/javase/1.4.2/docs/api/java/util/LinkedList.html#contains(java.lang.Object) — bezmax, Jan 26 '12 at 09:39
@Max, not really, he's executing the else code for each item not matching the condition, so he still needs a loop. — Tudor, Jan 26 '12 at 09:51
@Tudor Yes, but author agreed that his code is flawed. Read my first comment. There shouldn't had been that else block inside a loop. He simply wanted to check if the item exists in a list. — bezmax, Jan 26 '12 at 09:53

score 3 · Answer 1 · answered Jan 26 '12 at 09:20

3

You are changing the LinkedList content inside the for-each loop. The implementation of the LinkedList iterator checks on the next call to next() if the list has changed and throws the exception (sad I know ...).

answered Jan 26 '12 at 09:20

Alex Calugarescu

838
5
9

Do not use the for-each loop and thus avoid the use of the Iterator, like @Gagandeep Bali sugested... :D (use a simple for loop) – Alex Calugarescu Jan 26 '12 at 09:25
@Mazzy can't you use an ArrayList instead? – Tudor Jan 26 '12 at 09:29
@Mazzy : to know the dimensions of the list you can use listObject.size() Regards – nIcE cOw Jan 26 '12 at 09:37

score 2 · Answer 2 · answered Jan 26 '12 at 09:21

2

The enhanced for loop is used to iterate over the given set of values, but during iteration you are modifying the contents of the same, that's why you getting that error, instead use a normal for loop to do your stuff for this thing.

Regards

answered Jan 26 '12 at 09:21

nIcE cOw

24,468
7
50
143

score 0 · Answer 3 · answered Jan 26 '12 at 09:38

Sadly, there is no easy way around it. As the others have said, you cannot modify a collection inside this kind of loop. Your other option is to use a normal for loop. However, accessing a LinkedList by index like:

for(int i = 0; i < list.size(); i++) {
    list.get(i);
}

takes O(N) time for each item, because the linked list needs to be traversed from the beginning each time.

If the linked list is not essential to your algorithm, I suggest you to use an ArrayList instead and change your code as follows:

for(int i = 0; i < itemsList.size(); i++) {

     itm = itemsList.get(i);
     if(itm.equals(item)) {
          int value = itm.getCounter();
          value++;
          itm.setCounter(value);
          pw.println(itm.getCounter());
     } else {
          item.initCounter();
          itemsList.add(item);
          pw.println("It's the first time you connect to server!");   
     }
}

This will not throw the exception, but it's still not a nice piece of code because you are adding to the list while iterating and that is never a good idea.

I hope you had patience to read so far!

My final suggestion for you is to hold a temporary list of elements that you need to add and append them to the initial list at the end of the loop. This way you can keep all your original code and the LinkedList:

LinkedList<ItemClient> tempList = new LinkedList<ItemClient>();
for(ItemClient itm: itemsList) {

     itm = itemsList.get(i);
     if(itm.equals(item)) {
          int value = itm.getCounter();
          value++;
          itm.setCounter(value);
          pw.println(itm.getCounter());
     } else {
          item.initCounter();
          tempList.add(item);
          pw.println("It's the first time you connect to server!");   
     }
} 

itemsList.addAll(tempList);

concurrentexception i don't understand where it's the error

3 Answers3