Behavior of the ArrayList hasNext when removing elements

Question

I am removing elements from an ArrayList, consider the code below :

private static List<Person> persons = new ArrayList<>();
     //initialize the persons arraylist.

     for (Person p : persons) {
        if (p.getAge() < 18) {
            persons.remove(p);
        }
    }

The initialize method:

private static void initialize() {

    persons.add(new Person("Rahul", 25, Gender.MALE, Citizenship.INDIA));
    persons.add(new Person("Sally", 21, Gender.FEMALE, Citizenship.USA));
    persons.add(new Person("Ben", 35, Gender.MALE, Citizenship.CANADA));
    persons.add(new Person("Wayne", 30, Gender.MALE, Citizenship.UK));
    persons.add(new Person("ShaneYoung", 18, Gender.MALE, Citizenship.AUSTRALIA));
    persons.add(new Person("Kimmo", 17, Gender.MALE, Citizenship.FINLAND));
    persons.add(new Person("Simo", 17, Gender.MALE, Citizenship.FINLAND));

}

Output: [Rahul, Sally, Ben, Wayne, ShaneYoung, Simo]

Notice that the 2nd last and last items should not be returned ideally. I understand that if we try to modify the structure of the ArrayList while iterating through it , we could get a ConcurrentModificationException. I am referring to the remove(Object o) in the ArrayList. This is of course not the recommended way of removing the elements while iterating.The exception is thrown in most cases. However if I were to have a list where the 2nd last item satisfies my removal condition and the last one also satisfies the removal condition, a ConcurrentModificationException is not thrown because hasNext method returns false. The code for the hasNext() is:

return cursor != size

In this scenario, cursor == size. But note that that last element is also skipped or not processed.So no exception but the last element is skipped but also returned in the output.

I have gone through this bug and also looked at duplicate bugs it is linked to. They seem to highlight a slightly different scenario.

When we call the Iterator.remove, it seems to adjust the cursor position to lastRet. So using an Iterator.remove even with a 2nd last and last item which satisfies the removal condition works fine but the remove(Object o) which in turn calls the fastRemove does not seem to set this and hence skips the last element altogether.

I know the Iterator.remove javadoc mentions about unspecified behavior, the comments mentioned in the bugs raised above also indicate that the ConcurrentModificationException is thrown as a best effort. But considering the code above, the part where the last element is not filtered out, isn't this a bug or a scenario which should be handled in a better way ?

I have noticed another question here but the answers only explain about how the output is achieved given the current implementation which I have mentioned above.My question is more about whether the hasNext implementation should be changed to handle this scenario ?

"It's not a bug, it's a feature" :p HasNext uses iterators, and the behaviour is undefined when you remove elements on their collection while iterating. — Turtle, May 05 '17 at 11:54

score 2 · Answer 1 · answered May 05 '17 at 11:47

According to the JLS #14.14.2, the enhanced for loop is equivalent to using the iterator.

And as you mentioned, the javadoc of Iterator::remove is quite clear:

The behavior of an iterator is unspecified if the underlying collection is modified while the iteration is in progress in any way other than by calling this method.

So no, it's not a bug.

rilent · Answer 2 · 2017-05-05T11:50:55.463

0

It gets bugged when you try to remove elements of a list during looping over it. What I usually do is declaring another temp List, so that you can it with the removeAll method

List<Person> temp = new List<Person>();
     for (Person p : persons) {
        if (p.getAge() < 18) {
            temp.add(p);
        }
      }

persons.removeAll(temp);

edited May 05 '17 at 11:50

answered May 05 '17 at 11:47

rilent

659
1
6
22

2

An even better way in that case would be: `persons.removeIf(p -> p.getAge() < 18);`. – assylias May 05 '17 at 11:48
If you are on java8, I would recommend doing what @assylias say. – rilent May 05 '17 at 11:50
I agree with the suggestion of using the removeIf in Java 8. The remove in the Iterator manages to adjust the cursor to lastRet during the remove. I was wondering if it is not possible at all to do this in the remove(Object o) and handle this scenario mentioned above or may be there are potentially more problems if we try to change that hasNext implementation ? – Ajay Iyengar May 05 '17 at 11:58
1

@assylias, the removeIf may be better performing too but using iterator.remove() also seems to handle above scenario correctly. It removes the last 2 elements in the above scenario by correctly maintaining the cursor,lastRet fields. – Ajay Iyengar May 05 '17 at 12:33
@AjayIyengar Prior to Java 8, using the remove method of the Iterator is the right way to handle this, yes. – assylias May 05 '17 at 12:34

score 0 · Answer 3 · answered May 05 '17 at 12:01

If you loop from the end to start, you should be able to remove the values correctly:

for (int i = persons.size()-1; i > 0; i--){
    Person p = persons.get(i);
    if (p.getAge() < 18) {
        persons.remove(p);
    }
}

I know it's a little big off-topic, since your question focuses on iterator, but for me this has been always safe and the effort to write a little bit more code is worth it.

score 0 · Answer 4 · answered May 05 '17 at 12:14

0

You could use a ListIterator to remove the items since it has a remove method for that:

ListIterator<Person> iter = persons.listIterator();
while (iter.hasNext()) {
    Person p = iter.next();
    if (p.getAge() < 18) {
        iter.remove();
    }
}

but I would recommend the Java 8 solution as suggested by assylias:

persons.removeIf(p -> p.getAge() < 18);

answered May 05 '17 at 12:14

user85421

28,957
10
64
87

@assylias just because I prefer to use it if I have a list and am 'forced' to use an iterator (The iterator#remove javadoc is kind of scary [:-) ) – user85421 May 05 '17 at 13:30

score 0 · Answer 5 · edited May 23 '17 at 12:26

I think you already know one or more good ways to obtain what you want. So I will take your questions rather literally.

But considering the code above, the part where the last element is not filtered out, isn't this a bug or a scenario which should be handled in a better way ?

Define ‘bug’. When I taught testing many years ago, my textbook defined a bug as any behaviour not conforming with what a user can reasonably expect. I should say that this is a bug by this definition. However, it also seems to me to be a version of the bugs you are linking to, not a new and yet undiscovered one. The existence of those bug reports also confirms that this is a bug.

I did see the quote in assylias’ answer. Taken at face value and out of context it allows your iterator, and hence your for loop, to behave in any way at all, including tacitly skipping the last element of the list. But that quote is in the documentation of Iterator.remove(), a method you are not using, so I consider it very reasonable that you expect your loop either to proces all elements from the list or throw a ConcurrentModificationException (or some other exception for some other reason). I would have expected the same.

My question is more about whether the hasNext implementation should be changed to handle this scenario ?

It seems it would be quite easy to add a call to checkForComodification() in hasNext() just as there is in next() and in remove(). This would have given you the expected behaviour. Still, it’s worth thinking twice about, if the bug you are referring to would be fixed, it may be that we would prefer not to add this call. I have not thought through all the consequences.

Also even though I agree this is a bug, changing the behaviour of a class that is used in millions of programs running around the world could easily cause programs that are working as expected to show unwanted behaviour with a new Java version. Oracle is doing right in being very reluctant to making such changes.

The least they could do IMHO would be to document the bug more clearly in the documentation of ArrayList.

PS The same behaviour (bug) is readily produced with `persons.iterator()` and a `while` loop. — Ole V.V., May 06 '17 at 08:40

Behavior of the ArrayList hasNext when removing elements

5 Answers5