I just noticed that ReverseComparator has a no-arg constructor which
essentially returns "-1" for all comparisons. This is the same as if a
null comparator is passed to the single-arg constructor. Doing so breaks
the contract for a comparator [sgn(compare(x, y)) == -sgn(compare(y, x))].
I think this should be changed to use the object's natural ordering if no
comparator has been specified. More specifically, see my diff below.
Aside: Is there any compelling reason to use -1*comparator.compare(o1,o2)
instead of comparator.compare(o2,o1)? It seems this way eliminates the
multiplication step and should have the same result.
Additionally, I think that "InverseComparator" is a more appropriate name,
as "inverse" has a more direct meaning (to me at least). Inverse has the
mathematical meaning of the opposite sign which is exactly what this
comparator does. Reverse, on the other hand, implies switching the order
of something, the comparator isn't really switching the order (although a
collection using the comparator may be "reversed" if it uses the "inverse"
comparator).
Thoughts?
regards,
Michael
Index: src/java/org/apache/commons/collections/comparators/ReverseComparator.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/collections/src/java/org/apache/commons/collections/comparators/ReverseComparator.java,v
retrieving revision 1.5
diff -u -r1.5 ReverseComparator.java
--- src/java/org/apache/commons/collections/comparators/ReverseComparator.java 1 Mar
2002 19:18:49 -0000 1.5
+++ src/java/org/apache/commons/collections/comparators/ReverseComparator.java 19 Mar
+2002 05:09:43 -0000
@@ -75,6 +75,7 @@
* the reverse(List) method of java.util.Collection.
*/
public ReverseComparator() {
+ this(null);
}
/**
@@ -82,15 +83,15 @@
* of the passed in comparator.
*/
public ReverseComparator(Comparator comparator) {
- this.comparator = comparator;
+ if(comparator != null) {
+ this.comparator = comparator;
+ } else {
+ this.comparator = ComparableComparator.getInstance();
+ }
}
public int compare(Object o1, Object o2) {
- if(comparator == null) {
- return -1;
- } else {
- return -1*comparator.compare(o1,o2);
- }
+ return -1*comparator.compare(o1,o2);
}
}
--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
