Why I get "Comparison method violates its general contract"

if I call this line of code

Arrays.sort(allNew, 0, sizeNew, mCatBPosComparator);

with that comparator

 private class BPosComparator implements Comparator<CatBPos> {

    private static final int SWAP_BPosS = 1;

    @Override
    public int compare(CatBPos p1, CatBPos p2) {
      int lCompare;

      if (TimeUtils.isNotVoid(p1.getHdopBPos().getTime()) && TimeUtils.isNotVoid(p2.getHdopBPos().getTime())) {
        lCompare = p1.getHdopBPos().getTime().compareTo(p2.getHdopBPos().getTime());
        if (lCompare != 0) {
          return lCompare;
        }
      }

      lCompare = p1.getBPos().getTime().compareTo(p2.getBPos().getTime());
      if (lCompare != 0) {
        return lCompare;
      }

      if (p1.getBPos().isDeparture() && p2.getBPos().isVacation()) {
        return SWAP_BPOS;
      } else if (p1.getBPos().isVacation() && p2.getBPos().isArrival()) {
        return SWAP_BPOS;
      }

      // Ankunft und Abfahrt für denselben Bahnhof sollen in der richtigen Reihenfolge sein
      if (p1.getBPos().isDeparture() && p2.getBPos().isArrival() && p1.getBPos().getStat().equals(p2.getBPos().getStat())) {
        return SWAP_BPOS;
      }

      return 0;
    }
  }

I runs into that Exception:

Comparison method violates its general contract!

I see a lot of topics regarding this. But I dont understand it. Could you help me?

Jon Skeet
people
quotationmark

I suspect this is at least one immediate problem:

if (p1.getBPos().isDeparture() && p2.getBPos().isVacation()) {
    return SWAP_BPOS;
} else if (p1.getBPos().isVacation() && p2.getBPos().isDeparture()) {
    return SWAP_BPOS;
}

If you call compare(p1, p2) and then compare(p2, p1) you'll get the same non-zero result... which breaks the comparison rules.

Basically, your comparison needs to obey the documented rules. The above is one problem - it's entirely possible there are more. You should read the rules very carefully and think about how your comparison needs to work.

You can start off by negating SWAP_BPOS for the reverse situation:

if (p1.getBPos().isDeparture() && p2.getBPos().isVacation()) {
    return SWAP_BPOS;
} else if (p1.getBPos().isVacation() && p2.getBPos().isDeparture()) {
    return -SWAP_BPOS;
}

That may not be everything required, but it's a start.

people

See more on this question at Stackoverflow