Sam Adamsh
Sam Adamsh

Reputation: 3391

java arraylist iteration

Is this going to cause unpredictable behavior?

 ArrayList<X> x = new ArrayList<>();
 //x.add(new X())...
f:
for(int i = 0; i < x.size() -1;)
{
      X y = x.get(i);
      for(int j = i + 1; j < x.size();)
       {
          if(a) {
           x.remove(j);
           continue;
          }
          if(b) {
           x.remove(i);
           continue f;
          }
          j++;
        }
       i++;
}

Upvotes: 0

Views: 131

Answers (3)

Pavel Ryzhov
Pavel Ryzhov

Reputation: 5142

It would be better to create an array which contains indexes to remove. And fill it with indexes in the main for cycle. Than you can do something like this:

Collections.sort(indexesToRemoveArr);
Collections.reverse(indexesToRemoveArr);
for (int indexToRemove : indexesToRemoveArr) {
     arr.remove((int) indexToRemove );
}

In that code I remove indexes from end to start. That's why it won't do any problems.

Upvotes: 1

Sameer
Sameer

Reputation: 4389

Yes. Compiler will optimize and call x.size() only once. So your terminatin condition becomes incorrect as soon as you remove element.

Upvotes: 0

James Black
James Black

Reputation: 41858

I don't think it will be unpredictable, but your style seems wrong to me, so your approach is suspect.

Using a label is a bad idea, and deciding to use it shows that your approach is flawed.

You may want to look at this discussion about deleting on ArrayList, but basically, LinkedList would be fsster:

http://www.velocityreviews.com/forums/t587893-best-way-to-loop-through-arraylist-and-remove-elements-on-the-way.html

But, removing this way will work.

UPDATE:

Oops, just saw a couple of bugs:

      if(a) {
       x.remove(j);
       continue;
      }

OK, in this one, you will go back through the loop for j, but you didn't increment j.

      if(b) {
       x.remove(i);
       continue f;
      }

This is the same for i.

So, you need a similar change to fix it:

for(int i = 0; i < x.size() -1; i++)

This way, when you hit continue then it will still go to the next element.

Upvotes: 1

Related Questions