deletion from Set iterator

Question

Could someone explain to me why the following doesn't work (runways is a Hibernate PersistentSet) ?

System.out.println("size before " + runways.size());
Iterator<Runway> deleteIterator = runways.iterator();
while (deleteIterator.hasNext()) {
    Runway rwy = deleteIterator.next();
    if (rwy == rwy3) {
        System.out.println("remove !");
        deleteIterator.remove();
    }
}
System.out.println("size after " + runways.size());

I get the system.Out logs:

INFO: size before 3
INFO: remove !
INFO: size after 3

I thought that deletion through the iterator was safe and possible. You see the log "remove"! which indicates that the remove() method is called.

EDIT : PersistentSet has problem with the remove method from the Iterator interface.

Can you show this in a short but *complete* example? (A similar version works for me...) — Jon Skeet, May 01 '13 at 16:07
@Nicholas: The iterator would throw an `UnsupportedOperationException` if it didn't implement `remove` — Zim-Zam O'Pootertoot, May 01 '13 at 16:11
Your code works on my machine, although I do not know how your `Runway` class is implemented. Did you create some strange `equals` and/or `hashCode` methods in `Runway`? — home, May 01 '13 at 16:17
Is that the full code ? *If the hashCode() value of an object has changed since it was added to the HashSet, it seems to render the object unremovable.* — AllTooSir, May 01 '13 at 16:18
Please show us the Runway code, in particular your `equals` and `hashCode` methods if you've overridden them. — Rob Watts, May 01 '13 at 16:20
I voted to close as the OP did not provide required information (`equals` and `hashCode`)... — home, May 01 '13 at 17:36

score 3 · Answer 1 · answered May 01 '13 at 16:11

3

Check to make sure your hashCode() and equals() methods are overridden and correct on your Runway object.

answered May 01 '13 at 16:11

JustinKSU

4,875
2
29
51

1

He's seeing the `System.out` be called, which would indicate that he is already in the `if` block. – Nicholas May 01 '13 at 16:12
1

Since it enters the `if` block, this is not a problem. – Konstantin Yovkov May 01 '13 at 16:13
1

No, this is a very relevant issue – Rob Watts May 01 '13 at 16:14
@RobWatt I think I see what you are saying. The `HashMap$HashIterator` class has the `remove()` method for the iterator, and that performs a remove based on the `hashCode()` and `equals()`, is that a valid assumption? – Nicholas May 01 '13 at 16:21
He enters the if block because he is using == which checks to see if it's the same instance of the class. remove() uses .equals() which is different. – JustinKSU May 01 '13 at 20:07
To further explain, there is no guarantee the List implementation uses == although it should. Seems the problem IS related to the implementation of the List (see @user1446127 answer) – JustinKSU May 01 '13 at 20:15

Nicholas · Answer 2 · 2013-05-01T16:49:27.513

//EDIT: FYI this code is an example of failing code. Override your equals() and hashCode() properly.

To follow up on JustinKSU's point, if you don't implement your hashCode() and equals() well, what you are doing won't work. I can replicate your situation if I muddy the equals() and the hashCode().

public static void main(String[] args) {
    Set<Runway> runways = new HashSet<Runway>();
    Runway rwy1 = new Runway();
    Runway rwy2 = new Runway();
    Runway rwy3 = new Runway();
    runways.add(rwy1);
    runways.add(rwy2);
    runways.add(rwy3);
    System.out.println("size before " + runways.size());
    Iterator<Runway> deleteIterator = runways.iterator();
    while (deleteIterator.hasNext()) {
        Runway rwy = deleteIterator.next();
        if (rwy == rwy3) {
            System.out.println("remove !");
            deleteIterator.remove();
        }
    }
    System.out.println("size after " + runways.size());
}

private static class Runway {
    @Override
    public boolean equals(Object obj) {
        return false;
    }

    @Override
    public int hashCode() {
        return (new Random()).nextInt();
    }
}

+1; nice example! maybe you should add a **this code is wrong** disclaimer — home, May 01 '13 at 16:37

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

From looking at the JDK source code (I found it here), the iterator returned by a HashSet uses one of the HashSet's internal remove methods for performing the actual removal.

For a HashSet to work correctly, you need to either leave both equals and hashCode in your Runway class alone, or override them both. This SO question talks about this issue: What issues should be considered when overriding equals and hashCode in Java?

score 0 · Answer 4 · answered May 01 '13 at 16:56

My equals and hashCode methodes were created by Eclipse itself. So it doesn't come from them.

But I was too quick at the beginning of my question. I forgot that Runways is a PersistentSet from Hibernate (and not a java.util.HashSet)

And I found this bug which explaines why it is not possible to remove a object from a PersistenSet iterator.

The bug seems old and complicated to solve. My only option is then to recreate a set from the first one, and without the element I don't want.

deletion from Set iterator

4 Answers4