How do I iterate through a list that is modified outside of the current loop?

Newbie & Debugging Questions
#1

So I’m implementing Separating Axis Theorem right now and currently it works perfectly when just looping through two objects, however if I were to add any more objects to my List then the code would break without errors:

public boolean separatingAxisTheorem(){
      //this method is being called constantly in my update methods

    for(DevEntity e : handler.getDevWorld().getDevM().getDevEntities()){
     //here I am looping through a list of the entities in my world


        obtainEdges();

        if(!e.equals(this)){

            Vector2[] axes1 = axis;
            Vector2[] axes2 = e.axis;

            for(int i = 0; i < axes1.length; i++){
                Vector2 axis = axes1[i];

                p1 = new Vector2(projectAxis(axis));
                p2 = new Vector2(e.projectAxis(axis));

                if(!p1.overlap(p2)){
                    return false;
                }
            }

            for(int i = 0; i < axes2.length; i++){
                Vector2 axis = axes2[i];

                p3 = new Vector2(projectAxis(axis));
                p4 = new Vector2(e.projectAxis(axis));
                if(!p3.overlap(p4)){
                    return false;
                }
            }
        }   
    }
    return true;
}

This is how I add to the list in my World class:


    devM = new DevEntityManager(handler, new DevPlayer(handler, (1270/2) - 
    32, (720/2) - 32));
    devM.addDevEntity(new DevSquareEnemy(handler, 200, 400));
    //devM.addDevEntity(new DevTriEnemy(handler, 200, 100));

With the second addition commented out my code works fine and my code returns true/false depending on if my player is intersecting my DevSquareEnemy object, however if I remove the comment then my code will only return false, and I think this is because of the way I am adding my objects to the list, I have tried replacing my for loop with an Iterator and while loop but then I started receiving ConcurrentModificationException’s

#2

If you actually need to iterate a list that is modified outside of its current loop, you need a CopyOnWriteArrayList, or if it’s certainly going to be modified every time, just iterate through a copy of the list instead.

If you actually need to iterate through the newly added elements, you can’t use an iterator. It’s also probably not the design you’re looking for though there are sometimes good reasons to do that.

Cas :slight_smile:

#3

Use a standard for loop like so:


    List<DevEntity> devEntities = handler.getDevWorld().getDevM().getDevEntities();

    for(int i = 0; i < devEntities.size(); i++ ){
        DevEntity e = devEntities.get(i);
        //....
#4

Like this?

CopyOnWriteArrayList<DevEntity> des = new CopyOnWriteArrayList<DevEntity>(handler.getDevWorld().getDevM().getDevEntities());
		
		for(DevEntity e : des){
#5

I believe the field in DevWorld that contains the entities would be a CopyOnWriteArrayList, and then you would use a standard list reference in the SAT method. The for-each structure may not work depending on how it’s implemented, and you’d need to reinitialize the iterator every time the SAT method is called to make sure you are iterating over a current copy of the list.

You could also consider setting up a second data structure to hold new objects while the current objects are being acted upon. Then merge the two structures when neither is being iterated/modified.

something like this.


List<DevEntity> currentEntities
List<DevEntity> entitiesToAdd

update (){
    SAT(currentEntities)

    merge(currentEntities, entitiesToAdd)
}
#6

CopyOnWriteArray is used very much like an ArrayList. So yes, the declaration you wrote looks good.

CopyOnWriteArray is concurrency safe, so you can add to it or delete from it from another thread, even during an iteration. What happens is basically this: the new item (or lack of item) becomes part of the next iteration when a new “snapshot” of the list is taken.

CopyOnWriteArray works best in situations where iterations greatly outnumber adds and deletes to the list, assuming nothing will break when the “snapshot” list runs to completion. If that doesn’t describe your situation, maybe we can help come up with some alternate plans.

#7

Hi, so that declaration didn’t work, nothing changed and my SAT code did not recognise my DevWorld objects, and correct I will be rarely adding/removing to the ArrayList.

#8

[quote=kingAree] … that declaration didn’t work …
[/quote]
Which declaration of CopyOnWriteArray did’t work; the one you proposed, the one I proposed, both?

#9

The one I wrote

#10

It’s perfectly fine.

You can even cut out some cruft:

CopyOnWriteArrayList des = new [b]CopyOnWriteArrayList<>/b;

No need to redeclare the generic type.

In any case you’ve got completely the wrong end of the stick. If you’re just going to take a copy of your DevEntities list anyway, you have already done all the work the CopyOnWriteArrayList was going to do for you anyway. The idea is that the list returned by getDevEntities is already a CopyOnWriteArrayList, which means you can iterate through it and modify it to your heart’s content.

Cas :slight_smile: