0

My code is not working like I thought it should. The list listColumn0 contains the X and Y positions for 8 sprite objects on the screen. When I touch on one of them I check witch sprite object that match that X and Y position and then remove it from the list. But the strange thing is that this only works if I first touch on the last sprite object with index 7 and if I then continue with sprite object with index 6 and so on.

If I click on, let's say, sprite object with index 3 or some of the other besides the last, then the app is closing down! Why this? Can someone see what I have done wrong or can I do this in a better way? Is there a better way to detect/match which sprite object I have touched?

        String size = Integer.toString(listColumn0.size());
    // Check all lists
    for(ColorObject colorObject: listColumn0) {
        if(x > (colorObject.xPosition - colorObject.radius) && x < (colorObject.xPosition + colorObject.radius) && y > (colorObject.yPosition - colorObject.radius) && y < (colorObject.yPosition + colorObject.radius)) {

            String colorCode = Integer.toString(colorObject.color);
            String index = Integer.toString(listColumn0.indexOf(colorObject));
            Log.i("Test","Match!! " + size + " Color: " + colorCode + "ID: " + index);

            listColumn0.remove(listColumn0.indexOf(colorObject));
        }
    }

EDIT:

Eror message from LogCat:

05-22 07:08:55.482: W/dalvikvm(1444): threadid=12: thread exiting with uncaught exception (group=0x40a13300)
05-22 07:08:55.482: E/AndroidRuntime(1444): FATAL EXCEPTION: Thread-124
05-22 07:08:55.482: E/AndroidRuntime(1444): java.util.ConcurrentModificationException
05-22 07:08:55.482: E/AndroidRuntime(1444):     at java.util.ArrayList$ArrayListIterator.next(ArrayList.java:569)
05-22 07:08:55.482: E/AndroidRuntime(1444):     at com.test.game.ColorObjectManager.checkPosition(ColorObjectManager.java:164)
05-22 07:08:55.482: E/AndroidRuntime(1444):     at com.test.game.GameLoop.run(GameLoop.java:190)
05-22 07:08:55.482: E/AndroidRuntime(1444):     at java.lang.Thread.run(Thread.java:856)
05-22 07:13:55.753: I/Process(1444): Sending signal. PID: 1444 SIG: 9
Shruti
  • 1
  • 13
  • 55
  • 95
3D-kreativ
  • 9,053
  • 37
  • 102
  • 159

2 Answers2

1

You cannot modify listColumn0 while iterating it with a foreach loop. Doing this results in the ConcurrentModificationException which you can see in the LogCat.

You can modify a Collection while iterating it if you use a old-fashioned Iterator:

Iterator<ColorObject> it = listColumn0.iterator();
while(it.hasNext()) {
   ColorObject colorObject = it.next();
   ...
   it.remove(); // this removes the current object
}

And to reduce the scope of it it is a best practice to use a for loop here:

for (Iterator<ColorObject> it = listColumn0.iterator(); it.hasNext();) {
   ColorObject colorObject = it.next();
   ...
   it.remove(); // this removes the current object
}
Kai
  • 38,985
  • 14
  • 88
  • 103
  • Thanks! When you wrote old-fashioned Iterator I thought of this: for(int i = 0; i < listColumn0.size(); i++) {.... And this works, but this is perhaps a little bit slower? – 3D-kreativ May 22 '13 at 07:32
  • @3D-kreativ what makes you think this is slower? – Kai May 22 '13 at 07:36
  • Can't remember where, but I read somewhere about performance and that my first option should be faster. – 3D-kreativ May 22 '13 at 07:37
  • @3D-kreativ I've found a SO question about this: http://stackoverflow.com/questions/2113216/which-is-more-efficient-a-for-each-loop-or-an-iterator – Kai May 22 '13 at 07:38
  • OK, thanks for link and thanks for help! I guess my question is solved now. – 3D-kreativ May 22 '13 at 07:40
0

Easiest solution is

Integer colorObjectIndex = -1;
for(..)
.....
   colorObjectIndex = listColumn0.indexOf(colorObject)
.....
}
// check for colorObjectIndex > -1
listColumn0.indexOf(colorObjectIndex);

Or else you have to use Iterator of listColumn0.

rajdeep
  • 49
  • 1