Javadocs and minor refactoring for Nearby package

app/src/main/java/fr/free/nrw/commons/nearby/DirectUpload.java

@@ -1,20 +1,22 @@
 package fr.free.nrw.commons.nearby;
 
+import android.app.Activity;
 import android.os.Build;
 import android.support.v4.app.Fragment;
 import android.support.v4.content.ContextCompat;
 import android.support.v7.app.AlertDialog;
-import android.util.Log;
 
 import fr.free.nrw.commons.R;
 import fr.free.nrw.commons.contributions.ContributionController;
 import fr.free.nrw.commons.utils.PermissionUtils;
-import timber.log.Timber;
 
 import static android.Manifest.permission.READ_EXTERNAL_STORAGE;
 import static android.Manifest.permission.WRITE_EXTERNAL_STORAGE;
 import static android.content.pm.PackageManager.PERMISSION_GRANTED;
 
+/**
+ * Initiates the uploads made from a Nearby Place, in both the list and map views.
+ */
 class DirectUpload {
 
     private ContributionController controller;
@@ -25,59 +27,85 @@ class DirectUpload {
         this.controller = controller;
     }
 
-    // These permission requests will be handled by the Fragments.
-    // Do not use requestCode 1 as it will conflict with NearbyFragment's requestCodes
+    /**
+     * Initiates the upload if user selects the Gallery FAB.
+     * The permission requests will be handled by the Fragments.
+     * Do not use requestCode 1 as it will conflict with NearbyFragment's requestCodes.
+     */
     void initiateGalleryUpload() {
-        Log.d("deneme7","initiateGalleryUpload");
-        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
-            if (ContextCompat.checkSelfPermission(fragment.getActivity(), READ_EXTERNAL_STORAGE) != PERMISSION_GRANTED) {
-                if (fragment.shouldShowRequestPermissionRationale(READ_EXTERNAL_STORAGE)) {
-                    new AlertDialog.Builder(fragment.getActivity())
-                            .setMessage(fragment.getActivity().getString(R.string.read_storage_permission_rationale))
-                            .setPositiveButton(android.R.string.ok, (dialog, which) -> {
-                                Timber.d("Requesting permissions for read external storage");
-                                fragment.getActivity().requestPermissions
-                                        (new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP);
-                                dialog.dismiss();
-                            })
-                            .setNegativeButton(android.R.string.cancel, null)
-                            .create()
-                            .show();
-                } else {
-                    fragment.getActivity().requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP);
-                }
-            } else {
-                controller.startSingleGalleryPick();
-            }
+        // Only need to handle permissions for Marshmallow and above
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
+            return;
         }
-        else {
+
+        Activity parentActivity = fragment.getActivity();
+        if (parentActivity == null) {
+            controller.startSingleGalleryPick();
+            return;
+        }
+
+        // If we have permission, go ahead
+        if (ContextCompat.checkSelfPermission(parentActivity, READ_EXTERNAL_STORAGE) == PERMISSION_GRANTED) {
             controller.startSingleGalleryPick();
+            return;
         }
+
+        // If we don't have permission, and we need to show the rationale, show the rationale
+        if (fragment.shouldShowRequestPermissionRationale(READ_EXTERNAL_STORAGE)) {
+            new AlertDialog.Builder(parentActivity)
+                    .setMessage(parentActivity.getString(R.string.read_storage_permission_rationale))
+                    .setPositiveButton(android.R.string.ok, (dialog, which) -> {
+                        parentActivity.requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP);
+                        dialog.dismiss();
+                    })
+                    .setNegativeButton(android.R.string.cancel, null)
+                    .create()
+                    .show();
+            return;
+        }
+
+        // If we don't have permission, and we don't need to show rationale just request permission
+        parentActivity.requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP);
     }
 
+    /**
+     * Initiates the upload if user selects the Camera FAB.
+     * The permission requests will be handled by the Fragments.
+     * Do not use requestCode 1 as it will conflict with NearbyFragment's requestCodes.
+     */
     void initiateCameraUpload() {
-        Log.d("deneme7","initiateCameraUpload");
-        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
-            if (ContextCompat.checkSelfPermission(fragment.getActivity(), WRITE_EXTERNAL_STORAGE) != PERMISSION_GRANTED) {
-                if (fragment.shouldShowRequestPermissionRationale(WRITE_EXTERNAL_STORAGE)) {
-                    new AlertDialog.Builder(fragment.getActivity())
-                            .setMessage(fragment.getActivity().getString(R.string.write_storage_permission_rationale))
-                            .setPositiveButton(android.R.string.ok, (dialog, which) -> {
-                                fragment.getActivity().requestPermissions
-                                        (new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP);
-                                dialog.dismiss();
-                            })
-                            .setNegativeButton(android.R.string.cancel, null)
-                            .create()
-                            .show();
-                } else {
-                    fragment.getActivity().requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP);
-                }
-            } else {
-                controller.startCameraCapture();
-            }
-        } else {
+        // Only need to handle permissions for Marshmallow and above
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
+            return;
+        }
+
+        Activity parentActivity = fragment.getActivity();
+        if (parentActivity == null) {
+            controller.startCameraCapture();
+            return;
+        }
+
+        // If we have permission, go ahead
+        if (ContextCompat.checkSelfPermission(parentActivity, WRITE_EXTERNAL_STORAGE) == PERMISSION_GRANTED) {
             controller.startCameraCapture();
+            return;
         }
+
+        // If we don't have permission, and we need to show the rationale, show the rationale
+        if (fragment.shouldShowRequestPermissionRationale(WRITE_EXTERNAL_STORAGE)) {
+            new AlertDialog.Builder(parentActivity)
+                    .setMessage(parentActivity.getString(R.string.write_storage_permission_rationale))
+                    .setPositiveButton(android.R.string.ok, (dialog, which) -> {
+                        parentActivity.requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP);
+                        dialog.dismiss();
+                    })
+                    .setNegativeButton(android.R.string.cancel, null)
+                    .create()
+                    .show();
+            return;
+        }
+
+        // If we don't have permission, and we don't need to show rationale just request permission
+        parentActivity.requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP);
     }
 }

app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java

@@ -60,7 +60,7 @@ public NearbyPlacesInfo loadAttractionsFromLocation(LatLng curLatLng, LatLng lat
             Timber.d("Loading attractions neari, but curLatLng is null");
             return null;
         }
-        List<Place> places = nearbyPlaces.getFromWikidataQuery(latLangToSearchAround, Locale.getDefault().getLanguage(), returnClosestResult);
+        List<Place> places = nearbyPlaces.radiusExpander(latLangToSearchAround, Locale.getDefault().getLanguage(), returnClosestResult);
 
         if (null != places && places.size() > 0) {
             LatLng[] boundaryCoordinates = {places.get(0).location,   // south

app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java

@@ -21,11 +21,12 @@
 import fr.free.nrw.commons.upload.FileUtils;
 import timber.log.Timber;
 
+/**
+ * Handles the Wikidata query to obtain Places around search location
+ */
 public class NearbyPlaces {
 
-    private static int MIN_RESULTS = 40;
     private static final double INITIAL_RADIUS = 1.0; // in kilometers
-    private static double MAX_RADIUS = 300.0; // in kilometers
     private static final double RADIUS_MULTIPLIER = 1.618;
     private static final Uri WIKIDATA_QUERY_URL = Uri.parse("https://query.wikidata.org/sparql");
     private static final Uri WIKIDATA_QUERY_UI_URL = Uri.parse("https://query.wikidata.org/");
@@ -46,73 +47,63 @@ public NearbyPlaces() {
     }
 
     /**
-     * Returns list of nearby places around curLatLng or closest result near curLatLng. This decision
-     * is made according to returnClosestResult variable. If returnClosestResult true, then our
-     * number of min results set to 1, and max radius to search is set to 5. If there is no place
-     * in 5 km radius, empty list will be returned. This method sets radius variable according to
-     * search type (returnClosestResult or not), but search operation will be handled in overloaded
-     * method below, which is called from this method.
-     * @param curLatLng Our reference location
-     * @param lang Language we will display results in
-     * @param returnClosestResult If true, return only first result found, else found satisfactory
-     *                            number of results
-     * @return List of nearby places around, or closest nearby place
-     * @throws IOException
+     * Expands the radius as needed for the Wikidata query
+     * @param curLatLng coordinates of search location
+     * @param lang user's language
+     * @param returnClosestResult true if only the nearest point is desired
+     * @return list of places obtained
+     * @throws IOException if query fails
      */
-    List<Place> getFromWikidataQuery(LatLng curLatLng, String lang, boolean returnClosestResult) throws IOException {
+    List<Place> radiusExpander(LatLng curLatLng, String lang, boolean returnClosestResult) throws IOException {
+
+        int minResults;
+        double maxRadius;
+
         List<Place> places = Collections.emptyList();
 
-        /**
-         * If returnClosestResult is true, then this means that we are trying to get closest point
-         * to use in cardView above contributions list
-         */
+        // If returnClosestResult is true, then this means that we are trying to get closest point
+        // to use in cardView in Contributions fragment
         if (returnClosestResult) {
-            MIN_RESULTS = 1; // Return closest nearby place
-            MAX_RADIUS = 5;  // Return places only in 5 km area
+            minResults = 1; // Return closest nearby place
+            maxRadius = 5;  // Return places only in 5 km area
             radius = INITIAL_RADIUS; // refresh radius again, otherwise increased radius is grater than MAX_RADIUS, thus returns null
         } else {
-            MIN_RESULTS = 40;
-            MAX_RADIUS = 300.0; // in kilometers
+            minResults = 40;
+            maxRadius = 300.0; // in kilometers
             radius = INITIAL_RADIUS;
         }
 
-            // increase the radius gradually to find a satisfactory number of nearby places
-            while (radius <= MAX_RADIUS) {
+            // Increase the radius gradually to find a satisfactory number of nearby places
+            while (radius <= maxRadius) {
                 try {
                     places = getFromWikidataQuery(curLatLng, lang, radius);
                 } catch (InterruptedIOException e) {
                     Timber.d("exception in fetching nearby places", e.getLocalizedMessage());
                     return places;
                 }
                 Timber.d("%d results at radius: %f", places.size(), radius);
-                if (places.size() >= MIN_RESULTS) {
+                if (places.size() >= minResults) {
                     break;
                 } else {
                     radius *= RADIUS_MULTIPLIER;
                 }
             }
         // make sure we will be able to send at least one request next time
-        if (radius > MAX_RADIUS) {
-            radius = MAX_RADIUS;
+        if (radius > maxRadius) {
+            radius = maxRadius;
         }
-
         return places;
     }
 
     /**
-     * Creates and sends query for nearby wikidata items needs picture.
-     * Reads results from query and turns them into meaningful place variables.
-     * Adds them to the list of places.
-     * @param cur Our reference location to check around
-     * @param lang Language we will display results in
-     * @param radius Our query area is a circle, where cur is center and radius is radius
-     * @return
-     * @throws IOException
+     * Runs the Wikidata query to populate the Places around search location
+     * @param cur coordinates of search location
+     * @param lang user's language
+     * @param radius radius for search, as determined by radiusExpander()
+     * @return list of places obtained
+     * @throws IOException if query fails
      */
-    private List<Place> getFromWikidataQuery(LatLng cur,
-                                             String lang,
-                                             double radius)
-            throws IOException {
+    private List<Place> getFromWikidataQuery(LatLng cur, String lang, double radius) throws IOException {
         List<Place> places = new ArrayList<>();
 
         String query = wikidataQuery
@@ -125,8 +116,7 @@ private List<Place> getFromWikidataQuery(LatLng cur,
 
         // format as a URL
         Timber.d(WIKIDATA_QUERY_UI_URL.buildUpon().fragment(query).build().toString());
-        String url = WIKIDATA_QUERY_URL.buildUpon()
-                .appendQueryParameter("query", query).build().toString();
+        String url = WIKIDATA_QUERY_URL.buildUpon().appendQueryParameter("query", query).build().toString();
         URLConnection conn = new URL(url).openConnection();
         conn.setRequestProperty("Accept", "text/tab-separated-values");
         BufferedReader in = new BufferedReader(new InputStreamReader(conn.getInputStream()));
@@ -162,8 +152,7 @@ private List<Place> getFromWikidataQuery(LatLng cur,
 
             double latitude;
             double longitude;
-            Matcher matcher =
-                    Pattern.compile("Point\\(([^ ]+) ([^ ]+)\\)").matcher(point);
+            Matcher matcher = Pattern.compile("Point\\(([^ ]+) ([^ ]+)\\)").matcher(point);
             if (!matcher.find()) {
                 continue;
             }
@@ -192,5 +181,4 @@ private List<Place> getFromWikidataQuery(LatLng cur,
 
         return places;
     }
-
 }