0

I am trying to write code for the simulation WaTor in which sharks and fish eat each other and simulate population control etc. Anyways, the problem I am having is that i keep getting a concurrect modification exception even when using an iterator.

Here is my code:

private void updateSharks() {   
    for(Iterator<Shark> sharkit = sharks.iterator(); sharkit.hasNext();) {
        Shark sharky = sharkit.next();
        if(sharky.hasStarved()) {
            sharkit.remove();
        }
        else if(sharky.canBreed()) {
            addNewShark(sharky.getX(),sharky.getY());
        }
        moveShark(sharky);
    }
}

and here is the exception:

Exception in thread "main" java.util.ConcurrentModificationException
at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:901)
at java.util.ArrayList$Itr.next(ArrayList.java:851)
at WaTor.Ocean.updateSharks(Ocean.java:281)
at WaTor.Ocean.update(Ocean.java:307)
at WaTor.Main.main(Main.java:13)

Line 281 in ocean is "Shark sharky = sharkit.next();" Thank you for any help!

Jared
  • 578
  • 7
  • 21
  • You can't modify the collection while traversing it with iterator. Check for example this answer for a way to achieve what you want: http://stackoverflow.com/q/122105/4250114 – Dawid Wysakowicz Jun 17 '15 at 18:51
  • 1
    I'm guessing either `addNewShark` or `moveShark` is to blame here; they probably access the collection in an unsafe way. – Chris Hayes Jun 17 '15 at 18:52
  • 1
    Also, regarding your stack trace: the reason it throws right there is because `Iterator.next` is when it checks for commodification (as you can guess from the methods in the trace). It doesn't happen at the point you actually comodify the collection. – Chris Hayes Jun 17 '15 at 18:54

4 Answers4

2

You cannot modify a List while traversing it. If you are removing elements from a list while traversing then - One option to fix it would be to put these elements to be removed in another list and then looping over that list to remove elements. - Other option would be to use Iterator.remove() You are using Iterator.remove(). So that is fine.

If you are adding elements while traversing that can also be a problem. I haven't tested your code. But make sure you are not adding elements to list sharks in methods addNewSharks() and moveSharks()

Balkrishna Rawool
  • 1,865
  • 3
  • 21
  • 35
1

As others mentioned, you can't modify collection while iterating (remove is optional). Well, you can gather all the sharks that should die in another collection, and remove them after the iterator loop terminates.

Try the following:

private void updateSharks() { 
    ArrayList<Shark> toRemove = new ArrayList<Shark>();
    ArrayList<Shark> toAdd = new ArrayList<Shark>();

    for(Iterator<Shark> sharkit = sharks.iterator(); sharkit.hasNext();) {
        Shark sharky = sharkit.next();
        if(sharky.hasStarved()) {
            toRemove.add(sharky);
        }
        else if(sharky.canBreed()) {
            toAdd.add(/*create new shark object here*/)
        }
        moveShark(sharky);
    }

    for (Shark shark : toRemove) {
        sharks.remove(shark);
    }
    for (Shark shark: toAdd) {
        sharks.add(shark);
    }
}

EDIT: How it works

You are iterating through the collection. At this step, you can do whatever you wish within the objects contained in the collection, but you cannot alter the collection itself, because iterator does not allow you to.

So you create a second, temporary collection. Every time you encounter an object which you want to remove from the main collection, you add this object to the temporary collection. Because in Java only references are passed and there is no deep copying here, and it is the same object contained in two collections at the same time.

Now, when the iteration is over, you have all the objects (or their references, if you prefer to think about it that way) contained within your temporary collection. Because the iterator is gone, nothing is restraining you from removing all these objects from the primary collection, and it is easily done!

pnadczuk
  • 907
  • 1
  • 7
  • 12
  • will toRemove still recognize the shark I added to it even after I've moved it? moving the shark just changes its x and y coordinates, but does this mean the shark in sharks is different from the shark in toRemove? – Jared Jun 17 '15 at 19:26
  • Nope, it will be still the same shark. This is because it is a reference to the same object. – pnadczuk Jun 17 '15 at 19:31
  • this works! thank you. if you don't mind, i still don't quite get how it works. so i add sharky in toRemove, which refers to the starved shark. then i iterate through my sharks list and move the sharks. does this mean that the sharky in toRemove also got moved since its referring to the same shark? – Jared Jun 17 '15 at 19:46
  • cool thanks. now i just have to deal with teleporting sharks and fish haha – Jared Jun 17 '15 at 21:41
  • What exactly do you mean by teleporting ;-) ? – pnadczuk Jun 17 '15 at 21:50
  • well basically they move fine and then once they breed they start moving erratically and new fish are added all over the place haha – Jared Jun 17 '15 at 21:59
0

Please have a look at the JavaDoc for the Iterator. For the remove method you can find the following text:

Removes from the underlying collection the last element returned by this iterator (optional operation). This method can be called only once per call to next(). The behavior of an iterator is unspecified if the underlying collection is modified while the iteration is in progress in any way other than by calling ths method.

To me it looks like you are adding elements to the collection while iterating over it. Hence you may experience an unspecified behaviour.

Stefan Freitag
  • 3,578
  • 3
  • 26
  • 33
0

Don't use iterator. Iterate through the collection backwards like this:

import java.util.ArrayList;

public class RemoveFromCollectionExample {

    //
    // instance variables
    //

    private ArrayList<Shark> sharks;

    //
    // main
    //

    public static void main(String[] args) {
        RemoveFromCollectionExample example = new RemoveFromCollectionExample();
        example.sharks = new ArrayList<RemoveFromCollectionExample.Shark>();
        example.addShark(false, false, 0, 0);
        example.addShark(false, false, 0, 0);
        example.addShark(false, false, 0, 0);
        example.addShark(false, false, 0, 0);
        example.addShark(false, false, 0, 0);
        example.addShark(false, false, 0, 0);
        example.addShark(false, true, 0, 0);
        example.addShark(true, false, 0, 0);
        example.addShark(true, false, 0, 0);
        example.addShark(true, false, 0, 0);
        System.out.println("START WITH: " + example.sharks.size());
        example.updateSharks();
        System.out.println("Added 1, Removed 3");
        System.out.println("ENDED WITH: " + example.sharks.size());
    }

    //
    // update sharks
    //

    private void updateSharks() {
        for (int i = this.sharks.size() - 1; i > 0; i--) {
            Shark sharky = this.sharks.get(i);
            if (sharky.hasStarved()) {
                this.sharks.remove(i);
                System.out.println("REMOVED");
            } else if (sharky.canBreed()) {
                addNewShark(sharky.getX(), sharky.getY());
                System.out.println("ADDED");
            }
            moveShark(sharky);
        }
    }

    //
    // add new shark method
    //

    private void addNewShark(int x, int y) {
        this.sharks.add(new Shark(false, false, x, y));
    }

    private void addShark(boolean hasStarved, boolean canBreed, int x, int y) {
        this.sharks.add(new Shark(hasStarved, canBreed, x, y));
    }

    //
    // move method
    //

    private void moveShark(Shark sharky) {
    }

    //
    // Shark class
    //

    private class Shark {

        private boolean hasStarved;
        private boolean canBreed;
        private int x;
        private int y;

        public boolean hasStarved() {
            return hasStarved;
        }

        public void setHasStarved(boolean hasStarved) {
            this.hasStarved = hasStarved;
        }

        public boolean canBreed() {
            return canBreed;
        }

        public void setCanBreed(boolean canBreed) {
            this.canBreed = canBreed;
        }

        public int getX() {
            return x;
        }

        public void setX(int x) {
            this.x = x;
        }

        public int getY() {
            return y;
        }

        public void setY(int y) {
            this.y = y;
        }

        public Shark(boolean hasStarved, boolean canBreed, int x, int y) {
            super();
            this.hasStarved = hasStarved;
            this.canBreed = canBreed;
            this.x = x;
            this.y = y;
        }

        public void moveShark() {
        }

    }

}
John
  • 3,458
  • 4
  • 33
  • 54