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]
