I'm working on a game in Java, and in that game, when you fire a bullet and it goes off screen, it gets removed from the arraylist.

The problem is, when you fire a lot at once and they're all traveling off screen, I get a java.util.ConcurrentModificationException and the game crashes.

Note: I'm using the 'Rectangle' in java for my bullets so I could implement collision detection with ease.

I've tried everything so far.

Here's how I remove them:

for(Iterator<Bullet> iterator = bullet.iterator(); iterator.hasNext();) {

testBullet = iterator.next();

if(testBullet.rectangleBullet.x > (screen size is here) {

iterator.remove();

}

if(testBullet.rectangleBullet.x < 0) {

iterator.remove();

}

}

Quick note: This isn't copy and pasted, so the formatting may be a little weird.

Thanks!

Java just pisses me off with this kind of stuff. I know that the internet says calling remove should work, and yet I've had this same problem.

1. You are allowed to only call remove() once, so maybe it's being called twice for one iteration. That seems unlikely

2. Some other thread is adding/removing stuff to the same list while you are iterating.

3. Just cheat to get it working, and figure it out later (what I end up doing)

ArrayList<Bullet> toRemove = new ArrayList<>();
for( Bullet bullet : bullets ) {
   if( outOfBounds(bullet) )
      toRemove.add(bullet);
}
bullets.removeAll(toRemove);

Someone try this with a little main method and see if the original works or throws an exception. I don't have Java installed on my home machine now

From the documentation:

The iterators returned by this class's iterator and listIterator methods are fail-fast: if the list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException. Thus, in the face of concurrent modification, the iterator fails quickly and cleanly, rather than risking arbitrary, non-deterministic behavior at an undetermined time in the future.

So, it looks like it's actually possible to get this exception in a single-threaded app, if something is modifying the container from within the for loop. The code you posted doesn't appear to do that, but you also mention that you didn't copy and paste the exact code, and you didn't say that this was the whole code, so if there's more to your loop than what you've just posted, make sure it isn't adding more bullets from within that for loop!

If you're using Java 8, I believe you can just use an anonymous function and removeIf:

bullet.removeIf(b -> (b.rectangleBullet.x < 0 || b.rectangleBullet.x > screenSize));

An alternative to GlassKnife's solution is to add the bullets you don't want to delete to the second ArrayList and just zap the rest from the original, then swap them out.

ArrayList<Bullet> bullets= new ArrayList<>();
ArrayList<Bullet> backBuffer = new ArrayList<>();
for( Bullet bullet : bullets ) {
   if( !outOfBounds(bullet) )
      backBuffer.add(bullet);
}
bullets.clear();
ArrayList<Bullet> tmp = bullets;
bullets = backBuffer;
backBuffer = tmp;

This avoids all the shuffling of items that goes on under the hood when removing elements from an ArrayList.

Java just pisses me off with this kind of stuff. I know that the internet says calling remove should work, and yet I've had this same problem.

1. You are allowed to only call remove() once, so maybe it's being called twice for one iteration. That seems unlikely

2. Some other thread is adding/removing stuff to the same list while you are iterating.

3. Just cheat to get it working, and figure it out later (what I end up doing)

ArrayList<Bullet> toRemove = new ArrayList<>();
for( Bullet bullet : bullets ) {
   if( outOfBounds(bullet) )
      toRemove.add(bullet);
}
bullets.removeAll(toRemove);

Someone try this with a little main method and see if the original works or throws an exception. I don't have Java installed on my home machine now

I think I may have screwed up somewhere in my code and/or done something extremely stupid since the solution isn't working :P

I didn't think of that, though. It did seem to last a bit longer before throwing an exception, too.

EDIT: Wait, I may have figured it out. Will verify soon.

EDIT 2: Yup! Thanks for the little cheat, Glass_Knife! Works great.

I'd use Aldacron's method, it should be faster.

Actually, after looking at removeIf implementation, Oberon's method could be even faster.

1. GlassKnife's method does a first pass looking for elements, then a linear search through the array every time it needs to remove each element inside 'removeAll'.
2. Aldacron's method does a single pass, but it needs two "long lived" lists and copying the element's references around.
3. Oberon's does a single pass, and it only uses internally a short lived BitSet, which will probably be ruled out by escape analysis.

I'd go with 3 if Java 8 is available, otherwise with 2.

EDIT: Ohh dude, you're calling remove twice probably. Duh.

for(Iterator<Bullet> it = bullet.iterator(); it.hasNext(); bullet = it.next()) {
 if(bullet.rectangleBullet.x > screenSize || bullet.rectangleBullet.x < 0) {
   it.remove();
  }
}

Doing two ifs separately there makes it so you'll be calling 'remove' twice for bullets that meet both conditions. That throws an error obviously since you can't remove the same element twice. So you should remove once if it meets any of the two conditions.

EDIT: Ohh dude, you're calling remove twice probably. Duh.

for(Iterator<Bullet> it = bullet.iterator(); it.hasNext(); bullet = it.next()) {
 if(bullet.rectangleBullet.x > screenSize || bullet.rectangleBullet.x < 0) {
   it.remove();
  }
}

[background=#fafbfc]Doing two ifs separately there makes it so you'll be calling 'remove' twice for bullets that meet both conditions. That throws an error obviously since you can't remove the same element twice. So you should remove once if it meets any of the two conditions.[/background]

Indeed, but you'd hope that screenSize is positive, which implies you never remove it twice in the original code. Your solution of course makes sure it works even for non-positive screen sizes :)

My guess is that the problem is elsewhere, it's just detected here. @OP: Maybe you call a function in this loop that also loops over the list? (EDIT: Or the other way around, a function that iterates over the list calls this function? Not sure that could cause this exception)

The above solution is much nicer, so use that rather than mine, but if you don't care about the order in the bullets list, and you don't want to create temporary lists, another solution is to move the last entry to the deleted spot, like

int last = bullets.size() - 1;  // Do yourself a favor, make lists a multiple rather than a singular word.

int i = 0;
while (i <= last) {
    Bullet bullet = bullets.get(i); // So you can have singular bullets use the singular word.
    if(bullet.rectangleBullet.x > screenSize || bullet.rectangleBullet.x < 0) {
        if (i < last) { bullets.set(i, bullets.get(last)); }
        last = last - 1;
        // Do not increment i so the next iteration inspects the bullet we just moved here.
    } else {
        i = i + 1;
    }
}

// Minor problem, bullets[last] is now the last bullet, but bullets.size() may be larger, and Java has no truncate-list operation.
// Instead, remove the last element repeatedly
int new_size = last + 1;
while (bullets.size() > new_size) { bullets.remove(bullets.size() - 1); }

Untested, so it may contain some typos.

As you can see, it's much longer and somewhat convoluted, but it's a possibility.