ConcurrentModificationExecption

Question

I have a normal Database call that gathers information from my database. i use these informations to create my objects (CallQueue) these objects are then added to a list and the list is then returned.

Suddenly i discovered that my originally code did not work as intended because i created dublicates and so now i am trying to void that any dublicates are being created! But there is a problem!

I am unable to loop through my list and check wether or not the object is already created!

Here is my code:

while (query.next()) {
    if (!queues.isEmpty()) {
        /*This gives the Execption->*/
        for (CallQueue callQueue : queues) {
            if (callQueue.getType().equals(query.getString("KØ"))) {
                double decimalTime = query.getDouble("TID");
                int hourOfDay = (int)Math.round(24 * decimalTime);
                int callAmount = query.getInteger("ANTAL_KALD");
                if (hourOfDay > 19) {
                    hourOfDay = 19;
                }
                callQueue.addCallsByTime(hourOfDay, callAmount);
            } else {
                String queueName = query.getString("Kø");
                if (!queueName.equalsIgnoreCase("PrivatOverflow")) {
                    CallQueue cq = new CallQueue(query.getString("KØ"));
                    double decimalTime = query.getDouble("TID");
                    int hourOfDay = (int)Math.round(24 * decimalTime); 
                    int callAmount = query.getInteger("ANTAL_KALD");
                    if (hourOfDay > 19) {
                        hourOfDay = 19;
                    }
                    cq.addCallsByTime(hourOfDay, callAmount);
                    queues.add(cq);
                }
            }
        }
    } else {
        String queueName = query.getString("Kø");
        if (!queueName.equalsIgnoreCase("PrivatOverflow")) {
            CallQueue cq = new CallQueue(query.getString("KØ"));
            double decimalTime = query.getDouble("TID");
            int hourOfDay = (int)Math.round(24 * decimalTime); 
            int callAmount = query.getInteger("ANTAL_KALD");
            if (hourOfDay > 19) {
                hourOfDay = 19;
            }
            cq.addCallsByTime(hourOfDay, callAmount);
            queues.add(cq);
        }
    }
}

for (CallQueue callQueue : queues) {
    System.out.println(callQueue.getType());
}
query.Close();
return queues;

The execption i get from this is:

Caused by: java.util.ConcurrentModificationException

ive tried to look up the execption at ConcurrentModificationException

Can anyone help me fix this problem?

My advice: Loop through it by index instead of using the for-each construct if you want to add items to it while iterating. — Cat, Nov 28 '12 at 16:07

Joeri Hendrickx · Accepted Answer · 2012-11-28T16:16:29.837

5

You're doing an add inside of your iteration. As per the spec, you're not allowed to modify the collection you're iterating over.

The classic solution is to make a copy of the collection first and iterate over that instead. Another solution is to not use the iterator (the short foreach notation is using it implicitly) but to manually iterate using an index.

for (int i=0; i<queues.size(); i++) {
    CallQueue callQueue = queues.get(i);

    ... code goes here

}

A better solution would be to use a Set instead of a list (unless the order is important for you). That does mean you would have to implement equals and hashcode properly.

Btw: I believe your code is flawed. You're iterating over your list, and if the item you encounter does not match, you add one at the end. Meaning that, if the item you're looking for is the xth one in the list, you will have added a new item x times. I seriously doubt that is what you need. If you do some refactoring, this will become immediately clear.

edited Nov 28 '12 at 16:16

answered Nov 28 '12 at 16:10

Joeri Hendrickx

16,947
4
41
53

@Marc you can use CopyOnWriteArrayList as well, it incorporates this write copy functionality. – Jakub Zaverka Nov 28 '12 at 16:16
@Joeri first of all thank you for your response 2ndly your comment about my code being flawed could you elaburate abit? how would i else go around not making dublicates? – Marc Rasmussen Nov 28 '12 at 16:52
@MarcRasmussen basically, the check for whether or not you found your element needs to be _outside_ of your iteration. The best would be to refactor the find logic into its own method. That way you can also get rid of the duplcation of code you have right now for checking the empty list. If findQueue returns null, you need to create a new one and add it, regardless of whether the list was empty or not – Joeri Hendrickx Nov 28 '12 at 23:25

score 0 · Answer 2 · answered Nov 28 '12 at 16:13

0

Use set instead of List to avoid this problem. You can iterate on the same list that you want to modify.

http://docs.oracle.com/javase/1.5.0/docs/api/java/util/ConcurrentModificationException.html

answered Nov 28 '12 at 16:13

CodeDreamer

444
2
8

ConcurrentModificationExecption

2 Answers2

Linked