The HashSet<T>.removeAll method is surprisingly slow (in some cases)
java - Unintuitive behavior of removeAll method in sets - Stack Overflow
java - ArrayList removeAll() not removing objects - Stack Overflow
java - AbstractSet.removeAll() method not working properly - Stack Overflow
How are 2 members determined to be equal? I'm guessing if they have the same ID, you deem them equal, however java wants them to be the exact same reference in memory which may not be the case. To correct for this you can override the equals function to have it return if the ids are equal:
public class Member {
//..
@Override
public boolean equals(Object anObject) {
if (!(anObject instanceof Member)) {
return false;
}
Member otherMember = (Member)anObject;
return otherMember.getUserUID().equals(getUserUID());
}
}
Also when you override .equals it is recommended to also override hashCode so that the objects also work correctly in hashing functions like Set or Map.
You have to understand the sequence of invocation underneath it
-> ArrayList#removeAll(Collection)
|
| calls
|
|--> ArrayList#contains(Object)
|
| calls
|
|--> ArrayList#indexOf(Object)
|
| calls
|
|--> Object#equals
So if equals is not correctly overridden (following the equals contract rules), you're not getting the correct behaviour.
You only read documentation partly. You forgot one important paragraph from TreeSet:
Note that the ordering maintained by a set (whether or not an explicit comparator is provided) must be consistent with equals if it is to correctly implement the
Setinterface. (SeeComparableorComparatorfor a precise definition of consistent with equals.) This is so because the Set interface is defined in terms of the equals operation, but aTreeSetinstance performs all element comparisons using its compareTo (or compare) method, so two elements that are deemed equal by this method are, from the standpoint of the set, equal. The behavior of a set is well-defined even if its ordering is inconsistent with equals; it just fails to obey the general contract of the Set interface.
Now removeAll implementation comes from AbstractSet and utilizes equals method. According to your code you will have that "a".equals("A") is not true so that elements are not considered equal even if you provided a comparator which manages them when used in the TreeSet itself. If you try with a wrapper then the problem goes away:
import java.util.*;
import java.lang.*;
class Test
{
static class StringWrapper implements Comparable<StringWrapper>
{
public final String string;
public StringWrapper(String string)
{
this.string = string;
}
@Override public boolean equals(Object o)
{
return o instanceof StringWrapper &&
((StringWrapper)o).string.compareToIgnoreCase(string) == 0;
}
@Override public int compareTo(StringWrapper other) {
return string.compareToIgnoreCase(other.string);
}
@Override public String toString() { return string; }
}
static void test(StringWrapper... abc)
{
Set<StringWrapper> s = new TreeSet<>();
s.addAll(Arrays.asList(new StringWrapper("a"), new StringWrapper("b")));
s.removeAll(Arrays.asList(abc));
System.out.println(s);
}
public static void main(String[] args)
{
test(new StringWrapper("A"));
test(new StringWrapper("A"), new StringWrapper("C"));
}
}
This because you are now providing a consistent implementation between equals and compareTo of your object so you never have incoherent behavior between how the objects are added inside the sorted set and how all the abstract behavior of the set uses them.
This is true in general, a sort of rule of three for Java code: if you implement compareTo or equals or hashCode you should always implement all of them to avoid problems with standard collections (even if hashCode is less crucial unless you are using these objects in any hashed collection). This is specified many times around java documentation.
This is an inconsistency in the implementation of TreeSet<E>, bordering on the bug. The code will ignore custom comparator when the number of items in the collection that you pass to removeAll is greater than or equal to the number of items in the set.
The inconsistency is caused by a small optimization: if you look at the implementation of removeAll, which is inherited from AbstractSet, the optimization goes as follows:
public boolean removeAll(Collection<?> c) {
boolean modified = false;
if (size() > c.size()) {
for (Iterator<?> i = c.iterator(); i.hasNext(); )
modified |= remove(i.next());
} else {
for (Iterator<?> i = iterator(); i.hasNext(); ) {
if (c.contains(i.next())) {
i.remove();
modified = true;
}
}
}
return modified;
}
you can see that the behavior is different when c has fewer items than this set (top branch) vs. when it has as many or more items (bottom branch).
Top branch uses the comparator associated with this set, while the bottom branch uses equals for comparison c.contains(i.next()) - all in the same method!
You can demonstrate this behavior by adding a few extra elements to the original tree set:
s.addAll(Arrays.asList("x", "z", "a", "b"));
Now the output for both test cases becomes identical, because remove(i.next()) utilizes the comparator of the set.
A few possibilities:
1) There is a problem with the implementation of id's equals or hashCode methods. In this case, you could have id1.equals(id2) but id1.hashCode() != id2.hashCode(). This would cause inconsistency between equals and hashCode() for the album objects and could cause the symptoms you're seeing.
2) The id for one or more albums changes at some point after the for loop that checks deleteAlbum.equals(entity) for each album in the Set. If an id changes for an album, the remove() method may not be able to find it. An id could change from null to some non null number if got saved to the database - EclipseLink might do this for you without you explicitly asking it to.
3) Because of EclipseLink's meddling, deleteGroup might not actually be a HashSet when you run your code. The docs for EclipseLink suggest it will give you an "indirection object" instead of the java.util.Set (or java.util.HashSet I presume) declared in your class, depending on how it is configured. In that case, the contains and remove methods might not do what you expect them to.
See Overriding equals and hashCode in Java for more details on these and other possible problems involving equals and hashCode, which can cause bizarre behavior with Sets.
Okay let's try a bit of testing:
1:
Iterator<AlbumEntity> it = deleteGroup.getAlbums().iterator();
while(it.hasNext()){
AlbumEntity entity = it.next();
Assert.assertTrue(deleteGroup.getAlbums().contains(entity))
}
Does this test run successfully?
Yes, it calls equals(). compareTo() could only be used if the Set knew that it contained Comparable objects (sorted sets, for instance, might possibly do this).
It depends on the implementation.
For instance, a HashSet will use hashCode and equals. A TreeSet will probably use compareTo. Ultimately, so long as your types behave appropriately it shouldn't matter.
