CODEWITHSUNDEEP
javaandroidmultithreadinglistarraylist
David T.
David T.

Reputation: 23429

ArrayList.remove(int) vs ArrayList.remove(Object) in different thread

I have an ArrayList of Minion objects, and when a shield collides with a minion, I want to remove that minion from the ArrayList. However, I can only get it to work in 1 way, but not the other way. can anyone plz explain why?

In all 3 cases, I'm using Android's Renderer's onDrawFrame() method... so I have no control over when it gets called. but here's the code for all 3 ways:

Method 1: (does not work)

public void onDrawFrame(GL10 gl) {
    List<Integer> indexesToRemove = new ArrayList<Integer>();
    int len = minions.size();
    for(int i=0; i<len; i++){
        if( OverlapTester.overlapCircleRectangle( (Circle)shield1.bounds,  (Rectangle)minions.get(i).bounds) ){ //this tests out to work just fine
            indexesToRemove.add(i);
        }
    }
    for(int i=indexesToRemove.size()-1; i>=0; i--){
        minions.remove(indexesToRemove.get(i)); //<------ why doesn't this work?
    }
}

the problem is that that last line minions.remove(indexesToRemove.get(i)); doesn't ACTUALLY remove the minions. it DOES get called, with the proper index. i've stepped through the debugger, ran it straight up, and the arraylist isn't modified at all. why is this? actually, in the debugger, that line "minions.remove(indexesToRemove.get(i));" gets called a bijillion times.

Method 2: (still does not work)

public void onDrawFrame(GL10 gl) {
    synchronized(minions){
        List<Integer> indexesToRemove = new ArrayList<Integer>();
        int len = minions.size();
        for(int i=0; i<len; i++){
            if( OverlapTester.overlapCircleRectangle( (Circle)shield1.bounds,  (Rectangle)minions.get(i).bounds) ){ //this tests out to work just fine
                indexesToRemove.add(i);
            }
        }
        for(int i=indexesToRemove.size()-1; i>=0; i--){
            minions.remove(indexesToRemove.get(i)); //<------ why doesn't this work?
        }
    }
}

In here, I thought... "oh maybe since it's not quite synchronized, the drawFrame sometimes gets called too many times and is accessing the arraylist at the wrong time and i need to lock it. but it still doesn't work. again, that line minions.remove(indexesToRemove.get(i)); gets called properly with the right index, but does NOT actually remove the object. i'm watching my shield on the screen slam right into the minion and nothing happens to the minion (it doesn't get removed from the arraylist)

Method #3 (this actually works)

public void onDrawFrame(GL10 gl) {
    ArrayList<Minion> colliders = new ArrayList<Minion>(minions);
    int len = colliders.size();
    for(int i=0; i<len; i++){
        GameObject collider = colliders.get(i);
        if(OverlapTester.overlapCircleRectangle((Circle)shield1.bounds, (Rectangle)collider.bounds)){
            minions.remove(collider); // <---- why does THIS work instead?
        }
    }
}

this code works perfectly. the shield smacks the minion and the minion drops dead. as you can see here, the ONLY difference is that i'm using the overloaded ArrayList.remove(object) method instead of removing by index. as in the line minions.remove(collider);. why does THIS work ?

can anyone please explain?

on a side note, aside from storing another instance variable copy of the arraylist, is there a better way to manage ArrayList<Minion> colliders = new ArrayList<Minion>(minions); ?

Note: both Shield and Minion are regular Java objects that have a rectangular shape as boundary. all that math checks out just fine. i've tested it in the debugger and the collision detection is accurate. I'm also updating the bounds/positions accurate in the onDrawFrame() method.

Upvotes: 1

Views: 10864

Answers (3)

Gray
Gray

Reputation: 116918

@Jack's answer is correct. For posterity you should be using an Iterator here that you can remove with inside your loop:

// synchronization wrapper here
Iterator<Minion> iterator = minions.iterator();
while (iterator.hasNext()) {
    Minion minion = iterator.next();
    if( OverlapTester.overlapCircleRectangle(..., minion.bounds)) {
        iterator.remove();
    }
}

Upvotes: 7

Jack
Jack

Reputation: 133619

Because ArrayList provides two methods that are:

public E remove(int index)
public boolean remove(Object o)

When you invoke minions.remove(indexesToRemove.get(i)), since indexesToRemove is a List<Integer>, the invocation is bound to the second signature in which you remove an element by directly specifying the object, auto-unboxing doesn't turn your Integer into an int so the element is not found and nothing happens.

Try with: minions.remove((int)indexesToRemove.get(i)) so that static binding of the method is correctly applied.

Upvotes: 10

Ryan
Ryan

Reputation: 2260

It's treating Integer as an object ref in the 1st two example, cast it to an int

Upvotes: 3

Related Questions