You are here

Chapter 29: Lesson learned #5 - Do's and Don'ts when fiddling with ArrayLists

My original plans for this chapter was to "play" the game for a few minutes on a real device, to get some sort of feedback regarding the speed and size of the objects in the game, the range of the missiles, the accuracy of the ship navigation and so on.

I never got to that.

I ended up with a new crash, similar to the previous but of another type - "ConcurrentModificationException":

I had an eerie feeling that the quick-and-dirty way of setting member variables without using synchronized() would bite back sooner or later, but I can not really understand how it happened in this case. The pending events are set into member variables in GameEngine. I can't figure out how it causes issues in MissileHandler.

A few well spent minutes (reading the JavaDocs for java.util.ArrayList) showed however, that this error has nothing to do with my pending event construction:

"...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...."

(Ref: http://docs.oracle.com/javase/6/docs/api/java/util/ArrayList.html)

As there is a call to "java.util.ArrayList$ArrayListIterator.next" in the stack trace above, it apparently means that "for (Missile missile : missiles)" implicitly creates an iterator for me, and I'm thereby not allowed to use "missiles.remove(i)" as I did...

But in order to be able to use the iterator's own remove method instead (as stated in the JavaDoc excerpt above), I will have to declare it explicitly (of course).

This gives the following changes in MissileHandler.update():

public void update(float screenWidth, float screenHeight, int pendingEvent,
   float shipX, float shipY, int shipAngle, List<Asteroid> asteroids) {
  float dX;
  float dY;
  float distance;

  if (pendingEvent == GameEngine.EVENT_FIRE) {
   Missile newMissile = new Missile(bitmap, shipX, shipY, 3, shipAngle);
   missiles.add(newMissile);
  }
  // int i = 0;
  // for (Missile missile : missiles) {
  Iterator<Missile> missilesIterator = missiles.iterator();
  Missile missile;
  while (missilesIterator.hasNext()) {
   missile = missilesIterator.next();
   missile.age++;
   if (missile.age > 25 * 3 || missile.collided == true) {
    // TODO: Hardcoded max age. Works better with quadratic game
    // area
    // missiles.remove(i);
    missilesIterator.remove();
   } else {
    missile.move(screenWidth, screenHeight);
    for (Asteroid asteroid : asteroids) {
     asteroid.collided = false;
     dX = Math.abs(missile.x - asteroid.x);
     dY = Math.abs(missile.y - asteroid.y);
     distance = (float) Math.sqrt(dX * dX + dY * dY);
     if (distance <= (missile.bitmap.getWidth() / 2 + asteroid.bitmap
       .getWidth() / 2)) {
      asteroid.collided = true;
      missile.collided = true;
     }
    }
   }
   // i++;
  }
  missiles.trimToSize();
}

This solved the issue, and there are no more crashes when I fire off some missiles!

There is one thing that is a bit fishy here: MissileHandler is built as a copy of AsteroidHandler, which means that the same, incorrect, remove(i) method is used there also. But it doesn't fail...

Anyway - I'll fix MissileHandler.update also:

public void update(float screenWidth, float screenHeight) {
  Iterator<Asteroid> asteroidsIterator = asteroids.iterator();
  Asteroid asteroid;
  while (asteroidsIterator.hasNext()) {
   asteroid = asteroidsIterator.next();
   asteroid.move(screenWidth, screenHeight);
   if (asteroid.collided) {
    asteroidsIterator.remove();
    if (asteroid.size > Asteroid.QUARTER) {
     int fragmentSize = Asteroid.HALF;
     if (asteroid.size == Asteroid.HALF)
      fragmentSize = Asteroid.QUARTER;
     asteroids.add(new Asteroid(asteroidBitmaps[fragmentSize],
       asteroidBitmapsHighlighted[fragmentSize],
       fragmentSize, asteroid.x, asteroid.y, asteroid
         .getVelocity(),
       asteroid.getDirection() - 45, asteroid.rotation));
     asteroids.add(new Asteroid(asteroidBitmaps[fragmentSize],
       asteroidBitmapsHighlighted[fragmentSize],
       fragmentSize, asteroid.x, asteroid.y, asteroid
         .getVelocity(),
       asteroid.getDirection() + 45, asteroid.rotation));
    }
    break;
   }
  }
  asteroids.trimToSize();
}

I guess that the playability part will be the topic for next chapter...

Theme by Danetsoft and Danang Probo Sayekti inspired by Maksimer