Refactor and add tests to LengthUtils (#2201)

app/src/main/java/fr/free/nrw/commons/utils/LengthUtils.java

@@ -4,8 +4,12 @@ import java.text.NumberFormat;
 
 import fr.free.nrw.commons.location.LatLng;
 
+import android.support.annotation.NonNull;
+
 public class LengthUtils {
-    /** Returns a formatted distance string between two points.
+    /**
+     * Returns a formatted distance string between two points.
+     *
      * @param point1 LatLng type point1
      * @param point2 LatLng type point2
      * @return string distance
@@ -15,44 +19,68 @@ public class LengthUtils {
             return null;
         }
 
-        NumberFormat numberFormat = NumberFormat.getNumberInstance();
+        int distance = (int) Math.round(computeDistanceBetween(point1, point2));
-        double distance = Math.round(computeDistanceBetween(point1, point2));
+        return formatDistance(distance);
+    }
 
-        // Adjust to KM if M goes over 1000 (see javadoc of method for note
+    /**
-        // on only supporting metric)
+     * Format a distance (in meters) as a string
+     * Example: 140 -> "140m"
+     * 3841 -> "3.8km"
+     *
+     * @param distance Distance, in meters
+     * @return A string representing the distance
+     * @throws IllegalArgumentException If distance is negative
+     */
+    public static String formatDistance(int distance) {
+        if (distance < 0) {
+            throw new IllegalArgumentException("Distance must be non-negative");
+        }
+
+        NumberFormat numberFormat = NumberFormat.getNumberInstance();
+
+        // Adjust to km if distance is over 1000m (1km)
         if (distance >= 1000) {
             numberFormat.setMaximumFractionDigits(1);
-            return numberFormat.format(distance / 1000) + "km";
+            return numberFormat.format(distance / 1000.0) + "km";
         }
+
+        // Otherwise just return in meters
         return numberFormat.format(distance) + "m";
     }
 
     /**
      * Computes the distance between two points.
-     * @param from one of the two end points
+     *
-     * @param to one of the two end points
+     * @param point1 LatLng type point1
-     * @return distance between the points in meter
+     * @param point2 LatLng type point2
+     * @return distance between the points in meters
+     * @throws NullPointerException if one or both the points are null
      */
-    public static double computeDistanceBetween(LatLng from, LatLng to) {
+    public static double computeDistanceBetween(@NonNull LatLng point1, @NonNull LatLng point2) {
-        return computeAngleBetween(from, to) * 6371009.0D; // Earth's radius in meter
+        return computeAngleBetween(point1, point2) * 6371009.0D; // Earth's radius in meter
     }
 
     /**
      * Computes angle between two points
      *
-     * @param from Point A
+     * @param point1 one of the two end points
-     * @param to   Point B
+     * @param point2 one of the two end points
      * @return Angle in radius
+     * @throws NullPointerException if one or both the points are null
      */
-    private static double computeAngleBetween(LatLng from, LatLng to) {
+    private static double computeAngleBetween(@NonNull LatLng point1, @NonNull LatLng point2) {
-        return distanceRadians(Math.toRadians(from.getLatitude()),
+        return distanceRadians(
-                Math.toRadians(from.getLongitude()),
+                Math.toRadians(point1.getLatitude()),
-                Math.toRadians(to.getLatitude()),
+                Math.toRadians(point1.getLongitude()),
-                Math.toRadians(to.getLongitude()));
+                Math.toRadians(point2.getLatitude()),
+                Math.toRadians(point2.getLongitude())
+        );
     }
 
     /**
      * Computes arc length between 2 points
+     *
      * @param lat1 Latitude of point A
      * @param lng1 Longitude of point A
      * @param lat2 Latitude of point B
@@ -65,6 +93,7 @@ public class LengthUtils {
 
     /**
      * Computes inverse of haversine
+     *
      * @param x Angle in radian
      * @return Inverse of haversine
      */
@@ -74,8 +103,9 @@ public class LengthUtils {
 
     /**
      * Computes distance between two points that are on same Longitude
-     * @param lat1 Latitude of point A
+     *
-     * @param lat2 Latitude of point B
+     * @param lat1      Latitude of point A
+     * @param lat2      Latitude of point B
      * @param longitude Longitude on which they lie
      * @return Arc length between points
      */
@@ -85,6 +115,7 @@ public class LengthUtils {
 
     /**
      * Computes haversine
+     *
      * @param x Angle in radians
      * @return Haversine of x
      */

app/src/test/kotlin/fr/free/nrw/commons/utils/LengthUtilsTest.kt

@@ -1,46 +1,120 @@
 package fr.free.nrw.commons.utils
 
 import fr.free.nrw.commons.location.LatLng
-import fr.free.nrw.commons.utils.LengthUtils
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
 import org.junit.Test
 
 class LengthUtilsTest {
+    // Test LengthUtils.formatDistanceBetween()
+
     @Test
-    fun testZeroDistance() {
+    fun testFormattedDistanceBetweenSamePoints() {
         val pointA = LatLng(0.0, 0.0, 0f)
         val pointB = LatLng(0.0, 0.0, 0f)
-        assertDistanceBetween("0m", pointA, pointB)
+        assertFormattedDistanceBetween("0m", pointA, pointB)
     }
 
     @Test
-    fun testOneDegreeOnEquator() {
+    fun testFormattedOneDegreeOnEquator() {
         val pointA = LatLng(0.0, 0.0, 0f)
         val pointB = LatLng(0.0, 1.0, 0f)
-        assertDistanceBetween("111.2km", pointA, pointB)
+        assertFormattedDistanceBetween("111.2km", pointA, pointB)
     }
 
     @Test
-    fun testOneDegreeFortyFiveDegrees() {
+    fun testFormattedOneDegreeFortyFiveDegrees() {
         val pointA = LatLng(45.0, 0.0, 0f)
         val pointB = LatLng(45.0, 1.0, 0f)
-        assertDistanceBetween("78.6km", pointA, pointB)
+        assertFormattedDistanceBetween("78.6km", pointA, pointB)
     }
 
     @Test
-    fun testOneDegreeSouthPole() {
+    fun testFormattedOneDegreeSouthPole() {
         val pointA = LatLng(-90.0, 0.0, 0f)
         val pointB = LatLng(-90.0, 1.0, 0f)
-        assertDistanceBetween("0m", pointA, pointB)
+        assertFormattedDistanceBetween("0m", pointA, pointB)
     }
 
     @Test
-    fun testPoleToPole() {
+    fun testFormattedPoleToPole() {
         val pointA = LatLng(90.0, 0.0, 0f)
         val pointB = LatLng(-90.0, 0.0, 0f)
-        assertDistanceBetween("20,015.1km", pointA, pointB)
+        assertFormattedDistanceBetween("20,015.1km", pointA, pointB)
     }
 
-    private fun assertDistanceBetween(expected: String, pointA: LatLng, pointB: LatLng) =
+    @Test
+    fun testFormattedNullToNull() {
+        assertNull(LengthUtils.formatDistanceBetween(null, null))
+    }
+
+    // Test LengthUtils.formatDistance()
+
+    @Test
+    fun testFormatDistance() {
+        assertFormattedDistance("100m", 100)
+        assertFormattedDistance("123m", 123)
+        assertFormattedDistance("450m", 450)
+        assertFormattedDistance("999m", 999)
+        assertFormattedDistance("1km", 1000)
+        assertFormattedDistance("1km", 1001)
+        assertFormattedDistance("1.1km", 1050)
+        assertFormattedDistance("1.2km", 1234)
+        assertFormattedDistance("123,456.8km", 123456789)
+        assertFormattedDistance("0m", 0)
+    }
+
+    @Test(expected = IllegalArgumentException::class)
+    fun testIllegalFormatDistance() {
+        LengthUtils.formatDistance(-1)
+    }
+
+    // Test LengthUtils.computeDistanceBetween()
+
+    @Test
+    fun testDistanceBetweenSamePoints() {
+        val pointA = LatLng(0.0, 0.0, 0f)
+        val pointB = LatLng(0.0, 0.0, 0f)
+        assertDistanceBetween(0.0, pointA, pointB)
+    }
+
+    @Test
+    fun testDistanceOneDegreeOnEquator() {
+        val pointA = LatLng(0.0, 0.0, 0f)
+        val pointB = LatLng(0.0, 1.0, 0f)
+        assertDistanceBetween(111195.08, pointA, pointB)
+    }
+
+    @Test
+    fun testDistanceOneDegreeFortyFiveDegrees() {
+        val pointA = LatLng(45.0, 0.0, 0f)
+        val pointB = LatLng(45.0, 1.0, 0f)
+        assertDistanceBetween(78626.30, pointA, pointB)
+    }
+
+    @Test
+    fun testDistanceOneDegreeSouthPole() {
+        val pointA = LatLng(-90.0, 0.0, 0f)
+        val pointB = LatLng(-90.0, 1.0, 0f)
+        assertDistanceBetween(0.0, pointA, pointB)
+    }
+
+    @Test
+    fun testDistancePoleToPole() {
+        val pointA = LatLng(90.0, 0.0, 0f)
+        val pointB = LatLng(-90.0, 0.0, 0f)
+        assertDistanceBetween(20015115.07, pointA, pointB)
+    }
+
+    // Test assertion helper functions
+
+    private fun assertFormattedDistanceBetween(expected: String, pointA: LatLng, pointB: LatLng) =
             assertEquals(expected, LengthUtils.formatDistanceBetween(pointA, pointB))
-}
+
+    private fun assertFormattedDistance(expected: String, distance: Int) =
+            assertEquals(expected, LengthUtils.formatDistance(distance))
+
+    private fun assertDistanceBetween(expected: Double, pointA: LatLng, pointB: LatLng) =
+    // Acceptable error: 1cm
+            assertEquals(expected, LengthUtils.computeDistanceBetween(pointA, pointB), 0.01)
+}