android-storage icon indicating copy to clipboard operation
android-storage copied to clipboard

BUG: OrderType's comparators are wrong

Open drakeet opened this issue 6 years ago • 4 comments

OrderTypes are used for Collections.sort(list, comparator), but they are wrong, will lead to sorting error:

https://github.com/snatik/android-storage/blob/422a2b9cda205f8aa630d1c2604e1dafbc4897d3/storage/src/main/java/com/snatik/storage/helpers/OrderType.java#L27-L33

https://github.com/snatik/android-storage/blob/422a2b9cda205f8aa630d1c2604e1dafbc4897d3/storage/src/main/java/com/snatik/storage/helpers/OrderType.java#L34-L40

Try the test code:

TextView textView = findViewById(R.id.text);

ArrayList<Long> list1 = new ArrayList<>();
list1.add(1507755342589L);
list1.add(1507531570030L);
list1.add(1507403960822L);
list1.add(1507321017313L);
list1.add(1507235255461L);
list1.add(1507019735509L);
list1.add(1506985739800L);
list1.add(1510980593823L);
list1.add(1510884152730L);
list1.add(1510800393165L);
list1.add(1510372364493L);
list1.add(1510303509229L);
list1.add(1510293661701L);
list1.add(1510252260159L);
list1.add(1510179005397L);
list1.add(1510175799845L);
list1.add(1510095195735L);
list1.add(1510033172644L);
list1.add(1509974318427L);
list1.add(1509943757032L);
list1.add(1509906830316L);
list1.add(1509831147380L);
list1.add(1509812300328L);
list1.add(1509727408576L);
list1.add(1509659137459L);
list1.add(1509643466555L);
list1.add(1509597096744L);
list1.add(1509589493932L);
list1.add(1509501326282L);
list1.add(1509388926800L);
list1.add(1509360302279L);
list1.add(1509356734439L);
list1.add(1509249505561L);
list1.add(1509231210646L);
list1.add(1509228906474L);
list1.add(1509178702873L);
list1.add(1509119878492L);
list1.add(1509082817356L);
list1.add(1509042775375L);
list1.add(1508993607081L);
list1.add(1508990994790L);
list1.add(1508828403615L);
list1.add(1508762233971L);
list1.add(1508760776821L);
list1.add(1508727292580L);
list1.add(1508652546740L);
list1.add(1508633168391L);
list1.add(1508630744611L);
list1.add(1508500074390L);
list1.add(1508450769498L);
list1.add(1508444644300L);
list1.add(1508437127306L);
list1.add(1508419676999L);
list1.add(1508285637664L);
list1.add(1508273536784L);
list1.add(1508252239396L);
list1.add(1508241963462L);
list1.add(1508168648223L);
list1.add(1508160093931L);
list1.add(1508157246670L);
list1.add(1508135118936L);
list1.add(1507993768247L);
list1.add(1507958838417L);
list1.add(1507921437341L);
list1.add(1507899425055L);
list1.add(1507893347971L);
list1.add(1507855048760L);
list1.add(1507842285948L);
list1.add(1507834395876L);
list1.add(1507781709865L);
list1.add(1507764701974L);
list1.add(1507722142629L);
list1.add(1507648765778L);
list1.add(1507639201021L);
list1.add(1507607547432L);
list1.add(1507582839677L);
list1.add(1507539374452L);
list1.add(1507419694994L);
list1.add(1507398327485L);
list1.add(1507339481930L);
list1.add(1507337813397L);
list1.add(1507317123522L);
list1.add(1507291990718L);
list1.add(1507178591427L);
list1.add(1507162863060L);
list1.add(1507143200015L);
list1.add(1507093413099L);
list1.add(1507031582973L);
list1.add(1506987003095L);
list1.add(1506962797059L);
list1.add(1506774676535L);
list1.add(1511044834901L);
list1.add(1510904955797L);
list1.add(1510883452075L);
list1.add(1510790200105L);
list1.add(1510747571715L);
list1.add(1510662308710L);
list1.add(1510542646559L);
list1.add(1510257559848L);
list1.add(1510037484253L);

ArrayList<Long> list2 = new ArrayList<>(list1);
textView.append("before: " + (list1.equals(list2) + "\n"));

Collections.sort(list1, new Comparator<Long>() {
    @Override
    public int compare(Long o1, Long o2) {
        return (int) (o2 - o1);
    }
});
Collections.sort(list2, new Comparator<Long>() {
    @Override
    public int compare(Long o1, Long o2) {
        return Long.compare(o2, o1);
    }
});
textView.append("after: " + (list1.equals(list2) + "\n"));

The result is:

before: true
after: false

So the correct way should be like Long.compare (x, y) or Integer.compare(x, y):

return (x < y) ? -1 : ((x == y) ? 0 : 1);

drakeet avatar Oct 25 '17 06:10 drakeet

I tested the above code on an Android 7.1.2 device and got:

before: true
after: false

drakeet avatar Oct 25 '17 06:10 drakeet

In fact, when I sorted my real files with OrderTypes.DATE, I also encountered wrong results. So I wrote this test.

drakeet avatar Oct 25 '17 07:10 drakeet

If (aLong - bLong) > Integer.MAX_VALUE is true will cause the wrong result.

drakeet avatar Oct 27 '17 05:10 drakeet

And a month is 30 * 24 * 60 * 60 * 1000 > Integer.MAX_VALUE.

drakeet avatar Oct 27 '17 05:10 drakeet