BearND has uploaded a new change for review.
https://gerrit.wikimedia.org/r/166714
Change subject: Fix crash in sorting of nearby pages
......................................................................
Fix crash in sorting of nearby pages
IllegalArgumentException: Comparison method violates its general contract
bug: 72066
Change-Id: Ife50b225797cf0a471ac29a6db1d676316a7a860
---
M wikipedia-it/src/main/java/org/wikipedia/nearby/NearbyUnitTests.java
M wikipedia/src/main/java/org/wikipedia/nearby/NearbyActivity.java
M wikipedia/src/main/java/org/wikipedia/nearby/NearbyPage.java
3 files changed, 75 insertions(+), 36 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia
refs/changes/14/166714/1
diff --git
a/wikipedia-it/src/main/java/org/wikipedia/nearby/NearbyUnitTests.java
b/wikipedia-it/src/main/java/org/wikipedia/nearby/NearbyUnitTests.java
index 45f6d57..80db0f5 100644
--- a/wikipedia-it/src/main/java/org/wikipedia/nearby/NearbyUnitTests.java
+++ b/wikipedia-it/src/main/java/org/wikipedia/nearby/NearbyUnitTests.java
@@ -19,7 +19,8 @@
private static final int FOUR = 4;
private static final double SHORT_DISTANCE = 0.001d;
private static final double LONGER_DISTANCE = 0.01d;
- private static final int A_B_DISTANCE = 111320;
+ /** dist(origin, point a) */
+ private static final int A = 111319;
private Location nextLocation;
private List<NearbyPage> nearbyPages;
@@ -37,6 +38,7 @@
}
public void testSort() throws Exception {
+ calcDistances(nearbyPages);
Collections.sort(nearbyPages, new NearbyDistanceComparator());
assertEquals("a", nearbyPages.get(0).getTitle());
assertEquals("b", nearbyPages.get(1).getTitle());
@@ -46,6 +48,7 @@
public void testSortWithNullLocations() throws Exception {
nearbyPages.add(new NearbyPage(new JSONObject("{ \"title\": \"d\"
}")));
nearbyPages.add(new NearbyPage(new JSONObject("{ \"title\": \"e\"
}")));
+ calcDistances(nearbyPages);
Collections.sort(nearbyPages, new NearbyDistanceComparator());
assertEquals("a", nearbyPages.get(0).getTitle());
assertEquals("b", nearbyPages.get(1).getTitle());
@@ -57,10 +60,16 @@
public void testCompare() throws Exception {
NearbyPage nullLocPage = new NearbyPage(new JSONObject("{ \"title\":
\"nowhere\" }"));
+
+ calcDistances(nearbyPages);
+ nullLocPage.setDistance(getDistance(nullLocPage.getLocation()));
+ assertEquals(Integer.MAX_VALUE, nullLocPage.getDistance());
+
NearbyDistanceComparator comp = new NearbyDistanceComparator();
- assertEquals(A_B_DISTANCE, comp.compare(nearbyPages.get(0),
nearbyPages.get(1)));
- assertEquals(-1, comp.compare(nearbyPages.get(0), nullLocPage));
- assertEquals(1, comp.compare(nullLocPage, nearbyPages.get(0)));
+ assertEquals(A, comp.compare(nearbyPages.get(1), nearbyPages.get(2)));
+ assertEquals(-1 * A, comp.compare(nearbyPages.get(2),
nearbyPages.get(1)));
+ assertEquals(Integer.MAX_VALUE - A, comp.compare(nullLocPage,
nearbyPages.get(2)));
+ assertEquals((Integer.MIN_VALUE + 1) + A,
comp.compare(nearbyPages.get(2), nullLocPage)); // - (max - a)
assertEquals(0, comp.compare(nullLocPage, nullLocPage));
}
@@ -91,22 +100,30 @@
private class NearbyDistanceComparator implements Comparator<NearbyPage> {
public int compare(NearbyPage a, NearbyPage b) {
- if (a.getLocation() == null) {
- if (b.getLocation() == null) {
- return 0;
- } else {
- return 1;
- }
- } else if (b.getLocation() == null) {
- return -1;
- } else {
- return getDistance(a.getLocation()) -
getDistance(b.getLocation());
- }
+ return a.getDistance() - b.getDistance();
+ }
+ }
+
+ //
+ // UGLY: copy of production code
+ //
+
+ /**
+ * Calculates the distances from the origin to the given pages.
+ * This method should be called before sorting.
+ */
+ private void calcDistances(List<NearbyPage> pages) {
+ for (NearbyPage page : pages) {
+ page.setDistance(getDistance(page.getLocation()));
}
}
private int getDistance(Location otherLocation) {
- return (int) nextLocation.distanceTo(otherLocation);
+ if (otherLocation == null) {
+ return Integer.MAX_VALUE;
+ } else {
+ return (int) nextLocation.distanceTo(otherLocation);
+ }
}
private static final int ONE_KM = 1000;
diff --git a/wikipedia/src/main/java/org/wikipedia/nearby/NearbyActivity.java
b/wikipedia/src/main/java/org/wikipedia/nearby/NearbyActivity.java
index a4966ca..4a0bbe4 100644
--- a/wikipedia/src/main/java/org/wikipedia/nearby/NearbyActivity.java
+++ b/wikipedia/src/main/java/org/wikipedia/nearby/NearbyActivity.java
@@ -1,6 +1,5 @@
package org.wikipedia.nearby;
-import android.content.ActivityNotFoundException;
import org.wikipedia.PageTitle;
import org.wikipedia.R;
import org.wikipedia.Site;
@@ -14,6 +13,7 @@
import de.keyboardsurfer.android.widget.crouton.Crouton;
import de.keyboardsurfer.android.widget.crouton.Style;
import android.app.AlertDialog;
+import android.content.ActivityNotFoundException;
import android.content.Context;
import android.content.DialogInterface;
import android.content.Intent;
@@ -147,11 +147,11 @@
NearbyPage nearbyPage = adapter.getItem(position);
PageTitle title = new PageTitle(nearbyPage.getTitle(), site,
nearbyPage.getThumblUrl());
String geoUri = String.format(Locale.ENGLISH,
- "geo:0,0?q=%s,%s(%s)",
- nearbyPage.getLocation().getLatitude(),
- nearbyPage.getLocation().getLongitude(),
- title.getDisplayText()
- );
+ "geo:0,0?q=%s,%s(%s)",
+
nearbyPage.getLocation().getLatitude(),
+
nearbyPage.getLocation().getLongitude(),
+ title.getDisplayText()
+ );
Intent geoIntent = new Intent(Intent.ACTION_VIEW,
Uri.parse(geoUri));
try {
startActivity(geoIntent);
@@ -415,25 +415,31 @@
}
private void sortByDistance(List<NearbyPage> nearbyPages) {
+ calcDistances(nearbyPages);
+
Collections.sort(nearbyPages, new Comparator<NearbyPage>() {
public int compare(NearbyPage a, NearbyPage b) {
- if (a.getLocation() == null) {
- if (b.getLocation() == null) {
- return 0;
- } else {
- return 1;
- }
- } else if (b.getLocation() == null) {
- return -1;
- } else {
- return getDistance(a.getLocation()) -
getDistance(b.getLocation());
- }
+ return a.getDistance() - b.getDistance();
}
});
}
+ /**
+ * Calculates the distances from the origin to the given pages.
+ * This method should be called before sorting.
+ */
+ private void calcDistances(List<NearbyPage> pages) {
+ for (NearbyPage page : pages) {
+ page.setDistance(getDistance(page.getLocation()));
+ }
+ }
+
private int getDistance(Location otherLocation) {
- return (int) nextLocation.distanceTo(otherLocation);
+ if (otherLocation == null) {
+ return Integer.MAX_VALUE;
+ } else {
+ return (int) nextLocation.distanceTo(otherLocation);
+ }
}
private String getDistanceLabel(Location otherLocation) {
diff --git a/wikipedia/src/main/java/org/wikipedia/nearby/NearbyPage.java
b/wikipedia/src/main/java/org/wikipedia/nearby/NearbyPage.java
index 2a73c99..748528f 100644
--- a/wikipedia/src/main/java/org/wikipedia/nearby/NearbyPage.java
+++ b/wikipedia/src/main/java/org/wikipedia/nearby/NearbyPage.java
@@ -1,9 +1,9 @@
package org.wikipedia.nearby;
-import android.location.Location;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
+import android.location.Location;
/**
* Data object holding information about a nearby page.
@@ -30,6 +30,9 @@
private String title;
private String thumblUrl;
private Location location;
+
+ /** calculated externally */
+ private int distance;
public NearbyPage(JSONObject json) {
try {
@@ -76,7 +79,20 @@
return "NearbyPage{"
+ "title='" + title + '\''
+ ", thumblUrl='" + thumblUrl + '\''
- + ", location=" + location
+ + ", location=" + location + '\''
+ + ", distance='" + distance
+ '}';
}
+
+ /**
+ * Returns the distance from the point where the device is.
+ * Calculated later and can change. Needs to be set first by #setDistance!
+ */
+ public int getDistance() {
+ return distance;
+ }
+
+ public void setDistance(int distance) {
+ this.distance = distance;
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/166714
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ife50b225797cf0a471ac29a6db1d676316a7a860
Gerrit-PatchSet: 1
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: BearND <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
