I agree that it's weird that the behavior of the removeAll depends on the size of the other collection.
On Thu, Mar 5, 2009 at 11:11 AM, Pointbreak <[email protected]> wrote: > Dave, you are missing the point completely. The issue raised by Johan is > that if you call TreeSet.removeAll(otherSet), then in some cases the > comperator/equals contract of otherSet is used, in other cases the > comperator/equals contract of the treeset. Which is a bit strange, and > even by Sun considered a bug (although low priority). You are just > making both comperators the same, which of course will not demonstrate > this problem. Make the comperator of the TreeSet use str.compareTo > instead of compareToIgnoreCase, while keeping your implementation of > MyObject the same, and you will have the same problem that Johan > illustrated. > > On Thu, 05 Mar 2009 16:51 +0100, "Dave Schoorl" <[email protected]> > wrote: >> I do understand your point. However you are dealing with multiple >> contracts and not all of them are satisfied. And sometimes contracts are >> conflicting and it is impossible to meet all contracts, but I believe >> this is not the case in your scenario. >> >> A TreeSet is an implementation of a Set and must obey the Set contract. >> Adding order to the Set should not break the Set contract, because if I >> have a Set, I should not be concerned with the implementation details >> (is it a TreeSet or a HashSet or whatever). I believe the Set contract >> takes precedence over the Comparator contract, but in your case, by >> providing an equals method to MyObject that is in line with the >> Comparator, there still is no problem. See the code below, where >> MyObject implements the equals method with by comparing case insensitive: >> >> import java.util.ArrayList; >> import java.util.Collection; >> import java.util.Comparator; >> import java.util.HashSet; >> import java.util.Iterator; >> import java.util.TreeSet; >> >> public class CorrectedMyObjectTest { >> >> public static class MyObject { >> private final String aString; >> >> MyObject(String str) { >> aString = str; >> } >> >> @Override >> public String toString() { >> return aString; >> } >> >> @Override >> public int hashCode() { >> final int prime = 31; >> int result = 1; >> result = prime * result + ((aString == null) ? 0 : >> aString.toLowerCase().hashCode()); >> return result; >> } >> >> @Override >> public boolean equals(Object obj) { >> if (this == obj) >> return true; >> if (obj == null) >> return false; >> if (!(obj instanceof MyObject)) >> return false; >> MyObject other = (MyObject) obj; >> if (aString == null) { >> if (other.aString != null) { >> return false; >> } >> } else if (other.aString == null) { >> return false; >> } >> return aString.compareToIgnoreCase(other.aString) == 0; >> } >> >> } >> >> public static void main(String[] args) { >> TreeSet<MyObject> set1 = getCleanSet(); >> >> HashSet<MyObject> set2 = new HashSet<MyObject>(); >> set2.add(new MyObject("johan")); >> >> set1.removeAll(set2); >> >> System.err.println("this works: " + set1.size() + " == 1, and >> remaining object is " + set1.iterator().next() + " == rob"); >> >> // add removed back in >> set1 = getCleanSet(); >> >> // increase the size of set2 with some other random others >> set2.add(new MyObject("random1")); >> set2.add(new MyObject("random2")); >> >> // now size is bigger then set1, call removeall again: >> set1.removeAll(set2); >> >> System.err.println("this doesnt work: " + set1.size() + " != 1, >> so now both objects stil remain! This is because " >> + "removeAll isnt overwritten by TreeSet and AbstractSet >> walks over the smallest set but then compare fails"); >> >> // same for retainAll that also compares wrong. >> set1 = getCleanSet(); >> set1.retainAll(set2); >> >> System.err.println("set1 is now completely empty, but it should >> have 1 left: " + set1); >> >> // so both methods should always iterator through the colleciton >> they get and do the compare on its self >> >> set1 = getCleanFixedSet(); >> set1.removeAll(set2); >> >> System.err >> .println("now this works: " + set1.size() + " == 1, and >> remainng object is " + set1.iterator().next() + " == rob"); >> >> // add removed back in >> set1 = getCleanFixedSet(); >> >> set1.retainAll(set2); >> >> System.err.println("set1 is now correct, it has 1 left: " + >> set1); >> >> } >> >> public static TreeSet<MyObject> getCleanSet() { >> TreeSet<MyObject> set1 = new TreeSet<MyObject>(new >> Comparator<MyObject>() { >> >> public int compare(MyObject o1, MyObject o2) { >> return o1.aString.compareToIgnoreCase(o2.aString); >> } >> }); >> >> set1.add(new MyObject("johan")); >> set1.add(new MyObject("rob")); >> return set1; >> } >> >> public static TreeSet<MyObject> getCleanFixedSet() { >> TreeSet<MyObject> set1 = new MyFixedTreeSet<MyObject>(new >> Comparator<MyObject>() { >> >> public int compare(MyObject o1, MyObject o2) { >> return o1.aString.compareToIgnoreCase(o2.aString); >> } >> }); >> >> set1.add(new MyObject("johan")); >> set1.add(new MyObject("rob")); >> return set1; >> } >> >> public static class MyFixedTreeSet<E> extends TreeSet<E> { >> public MyFixedTreeSet(Comparator<? super E> comparator) { >> super(comparator); >> } >> >> @Override >> public boolean retainAll(Collection<?> c) { >> ArrayList<E> list = new ArrayList<E>(); >> Iterator<?> e = c.iterator(); >> while (e.hasNext()) { >> Object next = e.next(); >> if (contains(next)) { >> list.add((E) next); >> } >> } >> boolean modified = list.size() < size(); >> if (modified) { >> clear(); >> for (E item : list) { >> add(item); >> } >> } >> return modified; >> } >> >> @Override >> public boolean removeAll(Collection<?> c) { >> boolean modified = false; >> for (Iterator<?> i = c.iterator(); i.hasNext();) >> modified |= remove(i.next()); >> return modified; >> } >> } >> } >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [email protected] >> For additional commands, e-mail: [email protected] >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
