Array out of bounds error

ok In the game thingy I made, its spawning new dudes every 5 seconds. It adds them to the array list of the corresponding team. I figured that, since it takes up a bit of processing power to constantly resize an arraylist, I should give them all a boolean value (dead), that the redering method checks for before painting them. However the new problem is that if the game goes on, said arrays will become filled with dead, and therefore useless, players. So every 30 seconds I made it so that it goes throught the array, and deletes the dead players. Now here is my problem, at seemeingly random checks, like it will successfully delete the dead once or maby ten times or maby more, it throws an arrray out of bounds error that refuses to be caught. I have no idea why it is doing this, because I ensure the capacity to like 50 for both arraylists too. Any one have an idea of what could cause this? The entire code is really long, but here is what causes the problem (bluep and redp are the arraylists, i.e. teams):

if((cc+600)%600==0)
        {
            System.out.println("Deleting the dead");
            //System.out.println(bluep.size()+"  "+redp.size());
            try
            {
                int x,y;
                for(x=0;x<bluep.size();++x)
                {
                    BlueStick subb = (BlueStick)bluep.get(x);
                    if(subb.isDead()==true)
                    {
                        bluep.remove(x);
                    }
                }
                for(y=0;y<redp.size();++y)
                {
                    RedStick subr = (RedStick)redp.get(y);
                    if(subr.isDead()==true)
                    {
                        redp.remove(y);
                    }
                }
            }
            catch(IndexOutOfBoundsException e)
            {
                //System.out.println("x: "+x+" y: "+y);
                System.out.println(bluep.size()+"  "+redp.size());
            }
        }

You might have better luck with a for-each loop when iterating through an ArrayList.

When you make your index, in the existing code, it uses the size of ArrayList at that time as the upper bounds. After a single delete, the true upper bounds will be lower than the bounds of your for loop, hence the exception.

ah that makes perfect sense (never heard of a for each loop though…), ty so much for the help! However two things still bug me, one why would it work at all (because somethimes it wouldnt throw an exception), because there were dead plenty of times it checked that should have been deleted. Second why would the exception not be caught by the try catch loop? because I watched the terminal, and it didn’t print redp.size().

I say just use a Vector.

When you remove something from an ArrayList it shifts all following elements to fill the gap. When you then increase the index again you miss the object that was shifted into the gap, and it will not be removedis b if it’s dead. You can fix this by simply doing x–; when calling remove(). That could simply be it.

Because synchronization solves logic problems? :stuck_out_tongue:

seriously, do NOT put so much thought in premature optimization.

Just take any Collection and add and remove Items. If it plays out that this single collection does in any way affect your overall performance in a bad way. You can still think about using some better type of collection.

[quote]ah that makes perfect sense (never heard of a for each loop though…), ty so much for the help! However two things still bug me, one why would it work at all (because somethimes it wouldnt throw an exception), because there were dead plenty of times it checked that should have been deleted. Second why would the exception not be caught by the try catch loop? because I watched the terminal, and it didn’t print redp.size().
[/quote]
I honestly don’t know why your try/catch block did not catch the IndexOutOfBoundsException. Seems like it should have. On what line of code does the error occur? What is an example of an error message? It should state the line, the array and the offending value.

Following is a plausible for:each loop using your variable names –


    for (Bluestick subb : bluep)
    {
        if( subb.isDead() )
        {
            bluep.remove(subb);
        }
    }


The nice thing with the above is that you are dealing directly with the object itself, not an integer index.

Funny things can happen when iterating through an array while deleting its elements. I can’t recall if you are allowed to do it in an ArrayList or not.

The Javadoc for ArrayList says: "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. "

So maybe it will work? (You aren’t accessing these arrays from another code location at the same time, are you?)

But it might be better to try and design something where the issue is doesn’t even come up, like maybe first copying the array into a new array, then emptying the original array and copying only the live ones back in. There are some nice methods available for array copying.

Then again, as another poster says, keep it simple and don’t worry too much about optimizing until you have proof of a need for it.

Yup that does throw a ConcurrentModificationException :wink:

One where it doesn’t:


Iterator<Bluestick> iter = bluep.iterator();
while(iter.hasNext()) {
    Bluestick subb = iter.next();
    
    if(subb.isDead())
        iter.remove();
}

Thanks for the correction ra4king!

For some odd reason it is still throwing an exception.

i had to change the code slightly:

Iterator<BlueStick> iterb = bluep.iterator();
                while(iterb.hasNext()) 
                {    
                    BlueStick subb = iterb.next();        
                    if(subb.isDead())
                    {
                        iterb.remove();
                    }
                }
                Iterator<RedStick> iterr = redp.iterator();
                while(iterr.hasNext()) 
                {    
                    RedStick subr = iterr.next();        
                    if(subr.isDead())
                    {
                        iterr.remove();
                    }
                }

I have no idea why its still doing that, I’m also curious why this code also throws the exception:

for(int x=0;x<bluep.size();++x)
                {
                    //System.out.println("x: "+x);
                    //System.out.println("blue size: "+bluep.size());
                    
                    if(x==(bluep.size()-1))
                        break;
                    BlueStick subb = (BlueStick)bluep.get(x);
                    if(subb.isDead()==true)
                    {
                        bluep.remove(x);
                    }
                }

if it allways breaks out of the for loop on the very last object in the arraylist (before it has a chance to delete it), how is it still accessing past the bounds of the arraylist???

also here is the error code when I tested the iterator (just in case it matters):
Exception in thread “Thread-4” java.lang.IndexOutOfBoundsException: Index: 4, Size: 4
at java.util.ArrayList.rangeCheck(ArrayList.java:604)
at java.util.ArrayList.get(ArrayList.java:382)
at GamePanel.updateGame(GamePanel.java:425)
at GamePanel.gameLoop(GamePanel.java:145)
at GameFrame$1.run(GameFrame.java:127)

If isDead() returns true, this is effectively what happens:

there is no way this can cause an IndexOutOfBoundsException, unless:
[x] when isDead() is called, it also changes ‘bluep’
[x] you have multiple threads running concurrently, which are all modifying ‘bluep’

I only have 1 thread, and the method isDead() is:

public boolean isDead()
    {
        return dead;
    }

I don’t have the foggiest idea of why it isn’t working. I commented out the code and the error stopped, so I know it is coming from there.

I’m not sure I understand your post (Reply #9). Are you saying the rewrite, using Iterator, is throwing an Exception? What is the exception, and on what line?

For the second code excerpt, there is an obvious way to throw an out of bounds exception:

  • delete of the second to last element.

Let’s say your array starts with 10 elements.
On iteration 8, you delete an element.
Next for loop, x == 9, but bluep.size() also equals 9, and bluep.size() - 1 equals 8. So you blow right past your break command.

Do you have a step debugger?

Putting in <= instead of = would perhaps make your code work better, but it would still be less than crystal clear coding.

I would think you’d want to remove ALL the dead ones, and not leave 1 left, so why not do this:


for(int x=0; x < bluep.size(); x++)
{
	BlueStick subb = (BlueStick)bluep.get(x);
	if(subb.isDead())
	{
		bluep.remove(x);
		x--;
	}
}

I use that method whenever I need to remove things from ArrayLists and Vectors, while iterating through them. Works fine for me. As far as I can see, you can easily use this same method to fix your other problems.

I know this isn’t the most optimal way, and ra4king will probably roll over laughing, but it’ll work :stuck_out_tongue_winking_eye:

If you go backward through the list instead of forward, you don’t need to readjust x every time you delete.

Those casts also look dubious, but last time I suggested using generics properly, I got no end of grief from it. ::slight_smile:

Yeah. I’d do some inheritance here. Make a Stick-class, which holds all the fields and methods that are the same for BlueStick and RedStick, and add an int type to distinguish between the two. Then he would only have lists of the Stick-class, and there’d be no need to cast anything.

ok ultroman, I tried something almost exactly like the code in post 14, it still threw the exception… but it may have been the way I did it, I was on stupid pills that day. Also, those casts are very… unnecessary, but the whole code was just so I could learn active rendering, and see how well i could do with AI. I learned enough to improve it quite a bit (I hope), I really missused inheritance when I made the Blue/RedStick classes. Ty all for the help!!

Well, I hope you figure it out. If you want, I could check your implementation. That method really shouldn’t EVER go out of bounds.