The exception message is actually pretty descriptive. The contract it mentions is transitivity: if A > B
and B > C
then for any A
, B
and C
: A > C
. I checked it with paper and pencil and your code seems to have few holes:
if (card1.getRarity() < card2.getRarity()) { return 1;
you do not return -1
if card1.getRarity() > card2.getRarity()
.
if (card1.getId() == card2.getId()) { //... } return -1;
You return -1
if ids aren’t equal. You should return -1
or 1
depending on which id was bigger.
Take a look at this. Apart from being much more readable, I think it should actually work:
if (card1.getSet() > card2.getSet()) { return 1; } if (card1.getSet() < card2.getSet()) { return -1; }; if (card1.getRarity() < card2.getRarity()) { return 1; } if (card1.getRarity() > card2.getRarity()) { return -1; } if (card1.getId() > card2.getId()) { return 1; } if (card1.getId() < card2.getId()) { return -1; } return cardType - item.getCardType(); //watch out for overflow!