ConcurrentModificationException while trying to remove an element while iterating

Question

I am writing a code that determines if an item can be eaten. If it is edible, I return a give message.

Here is my code:

public void eat(String item){
    //update the game's message with one of the following options:
    //1:"you are not holding an item"
    //2:"item is not edible"
    //3:"Yum, that was a tasty item!"
    if(items == null){
        message = "You are not holidng anything.";
    }
    else{
        for(Item i: items){
            if(i.isEdible()){
                message = "Yum, that was tasty!";
                items.remove(i);
            }
            else{
                message = "That is not edible.";
            }
        }
    }
}

When I run the above, I get:

java.util.ConcurrentModificationException:
null(in java.util.ArrayList$itr)

What does this error mean? What can I do to fix this?

You can't remove an item while you are looping cause it messes up the memory. You can add logic to remove it afterwards — Nick, Jul 31 '15 at 15:19
Please Google errors before asking questions. Searching "java ConcurrentModificationException" returns dozens of relevant results. — DavidS, Jul 31 '15 at 15:36
`String item` is never used. Also, it looks like your character is going to eat every single item in his entire backpack every time you call `eat()`. Why are you looping at all if your intent is just to eat a single item? I'm asking because if you remove the loop, the answer to the question that you asked becomes irrelevant. — Rainbolt, Jul 31 '15 at 15:40

steven35 · Answer 1 · 2015-07-31T15:57:26.250

0

You need to use an Iterator to remove items from an Array

So in your case

Iterator<Item> i = items.iterator();
while (i.hasNext()) {
   Item s = i.next();
   if(s.isEdible()){
            message = "Yum, that was tasty!";
            i.remove();
        }
        else{
            message = "That is not edible.";
        }
    }
}

edited Jul 31 '15 at 15:57

answered Jul 31 '15 at 15:25

steven35

3,747
3
34
48

`remove()` is an optional method within the Iterator interface. It is not always supported. – JNYRanger Jul 31 '15 at 15:29
You can see in the error that he is using an `ArrayList` which supports `Iterator` – steven35 Jul 31 '15 at 16:01
Agreed, steven35. @JNYRanger, I think you'll be hard-pressed to find a standard Java iterator that does not support `remove()`. – DavidS Jul 31 '15 at 16:04

score 0 · Answer 2 · answered Jul 31 '15 at 15:26

0

You can make a temp ArrayList in eat(), add any items you're going to be removing in that temp list, and after you are done looping through items, just do an items.removeAll(temp);

answered Jul 31 '15 at 15:26

Austin

4,801
6
34
54

score 0 · Answer 3 · answered Jul 31 '15 at 15:26

When using an iterator to loop over a collection you cannot make changes to the collection itself (ie. remove, replace, add). This causes the data within the iterator to 'go bad,' and is the reason why you're receiving that exception. This happens even in non-concurrent iteration and in most languages that you can use iterators.

Instead you can do one of two things.

The first is that you can loop over indexes if it's an indexable collection and remove data that way. This is legal.
The other option is to add some logic to create a 'deletion queue'. You can then iterate over the deletion queue and remove the objects from the original collection (or use a removeAll() method if available on the collection)

score 0 · Answer 4 · answered Jul 31 '15 at 15:54

You cannot modify a collection while iterating over it. There are ways to get around this (e.g., using iterators), and there are also other approaches to solving the problem that don't require iterators. Here's one:

Collect the bad items
Remove the bad items

Pseudocode:

// Collect the bad items
ArrayList<Item> blacklist = new ArrayList<>();
for (Item item : items) {
    if (!isEdible(item)) {
        blackList.add(item)
    }
}

// Remove the bad items
for (Item item : blacklist) {
    myList.remove(item);
}

Your code has other problems.

String item is taken as a parameter but never used.
Your character is going to eat every single item in his entire backpack every time you call eat(), because you loop over the entire collection and remove all of the edible items.
item is a String, but items is a collection of Item. Why do the types mismatch?

It's pretty obvious from your description in the question that you meant to eat only one item (probably the item that was passed in). Just check if the item is in your backpack, and if it is, check it it is edible, and if it is, eat it.

public void eat(Item item) {
    if (items.contains(item)) {
        if (item.isEdible()) {
            items.remove(item);
            message = "Yum";
        } else {
            message = "Can't eat it";
        }
    } else {
        message = "Can't find it";
    }
}

ConcurrentModificationException while trying to remove an element while iterating

4 Answers4