search

remulasce / metroapp

Realtime arrival notification utility for LA Metro
2 stars 0 forks source link

User reported crash #216

Closed remulasce closed 9 years ago

remulasce commented 9 years ago

Looks like I goofed in a priority comparison method.

Offending comparison: public int compare(Trip lhs, Trip rhs) { return (lhs.getPriority() < rhs.getPriority()) ? 1 : -1; }

Violates the contract of comparitors, because if the priorities are equal, -1 will be returned, which denotes that one side is greater.

This is obviously incorrect.

Also, it's a simple fix.

User reports this happened with screen off when notification sounded. I think it was when he turned on his phone, all the priorities got recalculated and reordered, revealing this issue.

It was likely for priorities to be exactly equal because each trip in a stop has identical priority.

USER_COMMENT=alarm sounded when the screen was off ANDROID_VERSION=4.4.2 APP_VERSION_NAME=0.71 BRAND=MetroPCS PHONE_MODEL=LGMS323 CUSTOM_DATA= STACK_TRACE=java.lang.IllegalArgumentException: Comparison method violates its general contract! at java.util.TimSort.mergeHi(TimSort.java:864) at java.util.TimSort.mergeAt(TimSort.java:481) at java.util.TimSort.mergeForceCollapse(TimSort.java:422) at java.util.TimSort.sort(TimSort.java:219) at java.util.TimSort.sort(TimSort.java:169) at java.util.Arrays.sort(Arrays.java:2023) at java.util.Collections.sort(Collections.java:1883) at com.remulasce.lametroapp.c.b.a(ServiceRequestHandler.java:39) at com.remulasce.lametroapp.c.b.a(ServiceRequestHandler.java:51) at com.remulasce.lametroapp.p.a(TripPopulator.java:226) at com.remulasce.lametroapp.p.run(TripPopulator.java:195) at java.lang.Thread.run(Thread.java:841)

remulasce commented 9 years ago

I have a similar priority comparison elsewhere, but I remember I specifically did it correctly there.

I did do a ctrl-f across the project. There's ~4 other comparisons similar to this one, none of them are broken.

nighelles commented 9 years ago

The java_core uses a comparison based on getDistanceToStop which is never actually declared

On Thursday, April 23, 2015, remulasce notifications@github.com wrote:

I have a similar priority comparison elsewhere, but I remember I specifically did it correctly there.

I did do a ctrl-f across the project. There's ~4 other comparisons similar to this one, none of them are broken.

— Reply to this email directly or view it on GitHub https://github.com/remulasce/metroapp/issues/216#issuecomment-95766590.

remulasce commented 9 years ago

Then make an issue about it. This issue has been fixed. Beta apk is on Play Store.