1

In an android application, I have a list of locations. I need to sort them based on their distance with the user location. for that I implemented a custom comparator:

Collections.sort(houseList, new Comparator<HouseEntity>()
  {
     @Override
     public int compare(HouseEntity house1, HouseEntity house2)
     {
        if(userLocation == null) return 0;
        return (int) (userLocation.distanceTo(house1.location) - userLocation.distanceTo(house2.location));
     }
  });

It is working well on all the testings I made. However some users had a crash with the following error:

java.lang.IllegalArgumentException: Comparison method violates its general contract!

After reading all the other same issues on SO I concluded that this error appears when there might be an error in the logic (for example we might get a>b and b>a in the same time). But I couldn't find any scenario to replicate that logic error.

What scenario could be causing this error? And how can I solve it?

Thanks a lot for any help

Y2theZ
  • 10,162
  • 38
  • 131
  • 200
  • 2
    Are you sure the cast to `(int)` never overflows/underflows? Can you try `Double.compare(x, y)` instead? – Peter Lawrey Jan 18 '13 at 11:43
  • Have you tried logging the results of the comparisons? What's the return type of the `distanceTo` method? – Jon Skeet Jan 18 '13 at 11:44
  • It seems that in those cases, some bad values are passed in? – shuangwhywhy Jan 18 '13 at 11:49
  • @harism THe comparor use substruction to know the order of the distances. a_dist - b_dist will give a negative so a comes before b and so on. finally I will get a list sorted in the coorect order from the smallest distance to the biggest – Y2theZ Jan 18 '13 at 11:56
  • @Youssef removed my comment as it made no sense at all. Sorry :) – harism Jan 18 '13 at 11:58
  • You would be better not to bother sorting the list if userLocation is null. – artbristol Jan 18 '13 at 13:38

2 Answers2

1

As Peter Lawrey said, you should use Double.compare(x, y) or Float.compare(x, y) instead of casting to int. Here the explanation:

Comparator MUST be transitive, i.e. whenever A == B and B == C, then also A == C. Lets imagine we have three points A, B and C with the distances to the user location 0.2, 0.4 and 1.3.

  1. (int) (0.2 - 0.4) = (int) (-0.2) = 0 => A == B
  2. (int) (0.4 - 1.3) = (int) (-0.9) = 0 => B == C
  3. (int) (0.2 - 1.3) = (int) (-1.1) = -1 => A < C

As you can see the comparator is not transitive.

Vladimir Mironov
  • 30,514
  • 3
  • 65
  • 62
0

If userLocation == null then house1.equals(house2) should give true.

András Kerekes
  • 1,001
  • 7
  • 13
  • This doesn't really answer the question. – Tom van Enckevort Jan 18 '13 at 12:05
  • 1
    The `IllegalArgumentException` thrown with the given error message indicates that the `compare` method is not consistent with the `equals` method. I pointed out such a case, when this might happen, however until we see the implementation of `HouseEntity.equals()` we indeed cannot be sure whether this is the cause. – András Kerekes Jan 18 '13 at 12:14