Bug 58535 - ReverseComparator unsafely negates result
ReverseComparator unsafely negates result
Product: Tomcat 8
Classification: Unclassified
Component: Catalina
PC Mac OS X 10.1
: P2 normal (vote)
: ----
Assigned To: Tomcat Developers Mailing List
Depends on:
  Show dependency tree
Reported: 2015-10-25 19:55 UTC by Anthony Whitford
Modified: 2015-10-28 13:41 UTC (History)
0 users

Patch to avoid negation risk (602 bytes, patch)
2015-10-25 19:55 UTC, Anthony Whitford
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anthony Whitford 2015-10-25 19:55:18 UTC
Created attachment 33210 [details]
Patch to avoid negation risk

Consider the code:

    public int compare(Session o1, Session o2) {
        int returnValue = comparator.compare(o1, o2);
        return (- returnValue);

This code negates the return value of the compare method. This is a questionable or bad programming practice, since if the return value is Integer.MIN_VALUE, negating the return value won't negate the sign of the result. You can achieve the same intended result by reversing the order of the operands rather than by negating the results.

See http://findbugs.sourceforge.net/bugDescriptions.html#RV_NEGATING_RESULT_OF_COMPARETO

Recommend the following:

    public int compare(Session o1, Session o2) {
        // Note that comparing o2 with o1 to get reverse result...
        return comparator.compare(o2, o1);
Comment 1 Violeta Georgieva 2015-10-28 13:41:25 UTC

Thanks for the report and the patch.
I decided to remove this class and to use the standard Collections.reverseOrder(Comparator) method.
The fix is available in trunk, 8.0.x (for 8.0.29 onwards) and 7.0.x (for 7.0.66