CODEWITHSUNDEEP
javaandroidexceptionconcurrency
MrHill
MrHill

Reputation: 1271

ConcurrentModificationException while iterating through Arraylist (not removing)

I currently have a problem with iterating through an ArrayList. I've read several posts here, but nothing seem to have resolved my problem. Here is my code:

//restaurants contains a list of all restaurants and i want to filter them
List<Restaurant> newList = new ArrayList<Restaurant>();
List<Restaurant> allRestaurants = new ArrayList<Restaurant>(restaurants);
if (query != null && query.length() > 0 && !query.equals("*")) {
            synchronized (allRestaurants) {
                for (Iterator<Restaurant> it = allRestaurants.iterator(); it
                        .hasNext();) {
                    Restaurant restaurant = it.next();
                    if (restaurant.getCity().contains(query)) {
                        synchronized (newList) {
                            newList.add(restaurant);
                        }
                    } else {
                        newList = allRestaurants;
                    }
                }
            }

This is code was modified by me with several ideas i've read here (synchronized, using iterator instead of for-each-loop). I even have synchronized the whole method and still get an exception.

The exception is happening in following line:

Restaurant restaurant = it.next();

which I don't understand. I am not manipulating the list in this line. Why is this happening and how can i fix it?

Upvotes: 3

Views: 2273

Answers (5)

Trinimon
Trinimon

Reputation: 13967

You can't change the ArrayList that is used for iteration inside a loop; that is what ConcurrentModificationException says (http://docs.oracle.com/javase/1.4.2/docs/api/java/util/ConcurrentModificationException.html) and newList = allRestaurants; plus newList.add(restaurant);does potentially change the list allRestaurants.

So what you could do is

  1. create another list
  2. put items to modify in that list
  3. add/remove the new list (addAll or removeAll) to your old one after the loop

Check out http://www.javacodegeeks.com/2011/05/avoid-concurrentmodificationexception.html for more.

Upvotes: 0

WeMakeSoftware
WeMakeSoftware

Reputation: 9162

Your problem is in the else clause.

         newList = allRestaurants;

That's why you get exceptions

Upvotes: 0

Chris Knight
Chris Knight

Reputation: 25084

The failure starts with:

newList = allRestaurants;

which points both references to the same list (i.e. the one you are iterating over). You then do the following:

newList.add(restaurant);

modifying the list. From the javadoc of ConcurrentModificationException:

Note that this exception does not always indicate that an object has been concurrently modified by a different thread. If a single thread issues a sequence of method invocations that violates the contract of an object, the object may throw this exception. For example, if a thread modifies a collection directly while it is iterating over the collection with a fail-fast iterator, the iterator will throw this exception.

Upvotes: 0

Cephalopod
Cephalopod

Reputation: 15145

In the else branch

else {
   newList = allRestaurants;
}

You set newList to be allRestaurants. The next modification newList.add(restaurant); changes the allRestaurants-list.

The exception is thrown when it.next() is called, because then the iterator checks if its source was changed.

Upvotes: 0

John Vint
John Vint

Reputation: 40276

else{
    newList = allRestaurants;
}

That is almost certainly your issue.

Assigning newList to allRestaurants then adding to newList is causing your comodification.

That is after newList = allRestaurants any add to newList will update the mod count in allRestaurants and thus your error.

Upvotes: 4

Related Questions