-2

In this code result = myDef.getRank() - inDef.getRank();
I am getting java.lang.IllegalArgumentException: Comparison method violates its general contract!

Below is the code snippet . Please help me to resolve this issue.

public int compareTo(Object inObj) {
    int result = 0;
    if (inObj instanceof OmsFeatureFacade) {
        OmsFeatureFacade inFeature = (OmsFeatureFacade) inObj;

        FeatureDefinition inDef = inFeature.getFeatureDefinition();
        FeatureDefinition myDef = getFeatureDefinition();

        if ((inDef == null) || (myDef == null) ) {
            return result;
        }

        // Handling of primary feature.
        if (myDef.isPrimary() != inDef.isPrimary()) {
            if (myDef.isPrimary()) {
                return -1;
            }
            if (inDef.isPrimary()) {
                return 1;
            }
        }

        // Place all bolt on features under primary
        if (myDef.isPrimaryPricePlan() != inDef.isPrimaryPricePlan()) {
            if (myDef.isPrimaryPricePlan()) {
                return -1;
            }
            if (inDef.isPrimaryPricePlan()) {
                return 1;
            }
        }

        // Handling of SC feature.
        if (this.isSysGeneratedFeature() != inFeature.isSysGeneratedFeature()) {
            if (this.isSysGeneratedFeature()) {
                return -1;
            }
            if (inFeature.isSysGeneratedFeature()) {
                return 1;
            }
        }

        // First Sort by rank asc, if no rank, then getRank() returns very large number.
        result = myDef.getRank() - inDef.getRank();
        if (result != 0) {
            return result;
        }



        // Group by price plan, and sort alpha asc.
        result = myDef.getPricePlanCode().compareTo(inDef.getPricePlanCode());
        if (result != 0) {
            return result;
        }

        // Handling of main feature for price plan.
        if (this.isMainFeature() != inFeature.isMainFeature()) {
            if (this.isMainFeature()) {
                return -1;
            }
            if (inFeature.isMainFeature()) {
                return 1;
            }
        }
        // Sort alpha asc.
        result = myDef.getDescription().compareTo(inDef.getDescription());
        if (result != 0) {
            return result;
        }
    }
    return result;
}

2 Answers2

1

Using substraction myDef.getRank() - inDef.getRank()in compareTo methods is generally a bad practice because it can result in integer overflow problems. See more on that here.

There are two problematic parts with this:

  1. result = myDef.getRank() - inDef.getRank(); - it has integer overflow problems. You can fix it like this: result = new Integer(myDef.getRank()).compareTo(inDef.getRank());
  2. if (inObj instanceof OmsFeatureFacade) { - if you implement Comparable<OmsFeatureFacade> (watch out for generics) then you can skip this part.

  3. 3.
Community
  • 1
  • 1
Tamas Rev
  • 7,008
  • 5
  • 32
  • 49
  • //this is the implementation of public int getRank() { if (rank == null) { return Integer.MAX_VALUE; } return rank.intValue(); } – Shubham Agrawal May 10 '16 at 11:42
  • 1
    @ShubhamAgrawal an Integer.MAX_VALUE combined with the substraction is a typical int overflow situation. Apply the fix I suggested and this part will be fine. – Tamas Rev May 10 '16 at 12:14
  • 1
    @tamasrev or just use [`Integer.compare()`](https://docs.oracle.com/javase/8/docs/api/java/lang/Integer.html#compare-int-int-) which handles possible overflows as well. – Alex Salauyou May 10 '16 at 13:37
  • @SashaSalauyou that's fine too. Added it to the edited answer. – Tamas Rev May 10 '16 at 14:23
0

it's hard to say what's wrong with your code. The reason can be in the rest of compareTo method or in the getRank() implementation.

I can only recommend you to check this rules of compareTo implementation:

The implementor must ensure sgn(x.compareTo(y)) == -sgn(y.compareTo(x)) for all x and y. (This implies that x.compareTo(y) must throw an exception iff y.compareTo(x) throws an exception.)

The implementor must also ensure that the relation is transitive: (x.compareTo(y)>0 && y.compareTo(z)>0) implies x.compareTo(z)>0.

Finally, the implementor must ensure that x.compareTo(y)==0 implies that sgn(x.compareTo(z)) == sgn(y.compareTo(z)), for all z.

In the foregoing description, the notation sgn(expression) designates the mathematical signum function, which is defined to return one of -1, 0, or 1 according to whether the value of expression is negative, zero or positive.

Akirus
  • 108
  • 2
  • 11