CODEWITHSUNDEEP
javapuzzle
user133638
user133638

Reputation:

Why won't it remove from the set?

This bug took me a while to find...

Consider this method:

public void foo(Set<Object> set)
{
    Object obj=set.iterator().next();
    set.remove(obj)
}

I invoke the method with a non-empty hash set, but no element will be removed!

Why would that be?

Upvotes: 6

Views: 8974

Answers (7)

Barrrettt
Barrrettt

Reputation: 819

My similar case:

enter image description here

This not work for me. I need @Override equals and hashCode on my class Group, like this:

@Override
public int hashCode() {
    final int prime = 31;
    int result = 1;
    result = prime * result + ((id == null) ? 0 : id.hashCode());
    return result;
}

@Override
public boolean equals(Object obj) {
    if (this == obj) return true;
    if (obj == null) return false;
    if (getClass() != obj.getClass()) return false;
    Group other = (Group) obj;
    if (id == null) {
        if (other.id != null) return false;
    } else if (!id.equals(other.id)) return false;
    return true;
}

This forces to compare POJOs by id field, without the phase of the moon.

Upvotes: 0

Adrian Pronk
Adrian Pronk

Reputation: 13906

For a HashSet, this can occur if the object's hashCode changes after it has been added to the set. The HashSet.remove() method may then look in the wrong Hash bucket and fail to find it.

This probably wouldn't happen if you did iterator.remove(), but in any case, storing objects in a HashSet whose hashCode can change is an accident waiting to happen (as you've discovered).

Upvotes: 11

Tom Hawtin - tackline
Tom Hawtin - tackline

Reputation: 147154

Puzzle? If Object.hashCode, Object.equals or the "hash set" were implemented incorrectly (see for instance, java.net.URL - use URI).

Also if the set (directly or indirectly) contains itself, something odd is likely to happen (exactly what is implementation and phase of the moon dependent).

Upvotes: 3

Matthew Iselin
Matthew Iselin

Reputation: 10670

I can't help but feel that (part of) the problem is that the set is passed by value, not reference. I don't have much experience in Java though, so I could be totally wrong.

Upvotes: -3

nd.
nd.

Reputation: 8932

What is the implementation type of the set and what objects are inside the set?

  • If it is a HashSet, make sure that the value of the object's hashCode() method remains constant between set.put(...) and set.remove(...).
  • If it is a TreeSet, make sure that not modifications were made to the object that affect the set's comparator or the object's compareTo method.

In both cases, the code between set.put(...) and set.remove(...) violates the contract defined by the respective class implementation. As a rule of thumb, it is a good idea to use immutable objects as set content (and as Map keys). By their very nature such objects cannot be changed while they are stored inside a set.

If you are using some other set implementation, check out its JavaDoc for its contract; but usually either equals or hashCode must remain the same while the object is contained in the set.

Upvotes: 2

Blorgbeard
Blorgbeard

Reputation: 103467

Should it be:

public void foo(Set<Object> set)
{
    Iterator i = set.iterator();
    i.next();
    i.remove();
}

?

The bug could be something to do with:

public void remove()

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.

(Reference)

Upvotes: 1

freitass
freitass

Reputation: 6694

Beyond the missing ';' after set.remove(obj), It can happen in three situations (quoted from javadoc).

ClassCastException - if the type of the specified element is incompatible with this set (optional).
NullPointerException - if the specified element is null and this set does not support null elements (optional). 
UnsupportedOperationException - if the remove method is not supported by this set.

You can also try:

public void foo(Set<Object> set)
{
    Object obj=set.iterator().next();
    iterator.remove();
}

Upvotes: 1

Related Questions