1

I have comparison method for users list. I am moving users up and down in this list based on 3 values: their type, if they are checked as favourite and based on their names and surnames. My method looks like this: 

public int compare(ContactEntity lhs, ContactEntity rhs) {
    Integer lType = ContactUtils.getPresenceStatusType(lhs.getPresenceStatus(), presenceEntities).getType();
    Integer rType = ContactUtils.getPresenceStatusType(rhs.getPresenceStatus(), presenceEntities).getType();

    Boolean lhsFavourite = ContactsHelper.isContactFavourite(lhs.getContactId(), favourites);
    Boolean rhsFavourite = ContactsHelper.isContactFavourite(rhs.getContactId(), favourites);

    int c;
    c = lType.compareTo(rType);
    if (c == 0)
        c = rhsFavourite.compareTo(lhsFavourite);
    if (c == 0)
        c = lhs.compareTo(rhs);
    return c;
}

In very rare cases, when for some users there is very huge friends list, i see in crashlytics error Comparison method violates its general contract. What is wrong with my code?

ContactEntity comparison method:

@Override
public int compareTo(@NonNull ContactEntity another) {
    int compared = mFirstName.compareTo(another.getFirstName());
    if (compared == 0) {
        compared = mLastName.compareTo(another.getLastName());
    }
    return compared;
}
Phantômaxx
  • 37,901
  • 21
  • 84
  • 115
Andris
  • 3,895
  • 2
  • 24
  • 27
  • Does this answer your question? [Java error: Comparison method violates its general contract](https://stackoverflow.com/questions/11441666/java-error-comparison-method-violates-its-general-contract) – deHaar Feb 19 '20 at 07:57
  • @deHaar I looked on that question. But as far as i see, my code is even more readable and simpler than answers in that question. None of those answers are good enough to fix this issue. – Andris Feb 19 '20 at 08:00
  • OK then... retracted the close vote. – deHaar Feb 19 '20 at 08:12
  • 2
    If transitivity is the problem, then most probably the problem is here: `c = lhs.compareTo(rhs);`. Because the comparison of integer and booleans done before that will most probably be ok when it comes to transitivity. Could you add to the question the comparison method for class `ContactEntity`? – gil.fernandes Feb 19 '20 at 08:15
  • @gil.fernandes Tnx. I have updated my question. – Andris Feb 19 '20 at 08:21
  • Are you trying to implement `Comparator`? Can't you just use of its static methods to create an instance? – Sweeper Feb 19 '20 at 08:31
  • @Sweeper ′Are you trying to implement Comparator′: Yes. ′Can't you just use of its static methods to create an instance?′: What benefits i will have, if i do this way? – Andris Feb 19 '20 at 08:36
  • 1
    I noticed that the check for favorites had rhs.favorite.compareTo(lhs.favorite). The other two comparisons involved lhs.comlareTo(rhs) – NomadMaker Feb 19 '20 at 08:39
  • @Andris So you don't have to think about writing your code in a way that conforms to the contract. Just give it the three things you want to compare by calling `comparing` and `thenComparing`. If it _still_ violates the contract, then something's wrong with `getPresenceStatusType` and `isContactFavourite`. – Sweeper Feb 19 '20 at 08:39
  • @NomadMaker Oh! Good spot! Andris, this is exactly the kind of mistakes that `comparing` and `thenComparing` prevents you from making. – Sweeper Feb 19 '20 at 08:41
  • @NomadMaker and Sweeper tnx. I will take a look – Andris Feb 19 '20 at 08:42
  • Actually as far as i check. this is the order i want users to be in list. @Sweeper i will take a look to implement your provided solution. – Andris Feb 19 '20 at 08:47
  • Speculation: what happens if the favourites or status change during a lengthy comparison? Could this lead to the case that before the change `user1 > user2` and suddenly after the change `user2 > user1`. Are you operating the comparison on a copy of the list or on data which can change? – gil.fernandes Feb 19 '20 at 08:52
  • @gil.fernandes well this looks like going to right direction. Yes, comparison is on list, in which data can change. Tnx. I will take a look. But this is promising! – Andris Feb 19 '20 at 08:56

0 Answers0