refactoring: remove duplicated code from static maps app

* introduce shared super class for static maps
* move app enablement into apps, the factory should not know specific
apps

11 files changed, 140 insertions, 119 deletions

diff --git a/main/src/cgeo/geocaching/apps/AbstractApp.java b/main/src/cgeo/geocaching/apps/AbstractApp.java
index f79afe0..3008261 100644
--- a/main/src/cgeo/geocaching/apps/AbstractApp.java
+++ b/main/src/cgeo/geocaching/apps/AbstractApp.java
@@ -1,5 +1,6 @@
 package cgeo.geocaching.apps;
 
+import cgeo.geocaching.cgCache;
 import cgeo.geocaching.cgeo;
 import cgeo.geocaching.cgeoapplication;
 
@@ -9,10 +10,9 @@ import android.content.pm.PackageManager;
 
 public abstract class AbstractApp implements App {
 
-    protected String packageName;
-
-    private String intent;
-    private String name;
+    private final String packageName;
+    private final String intent;
+    private final String name;
 
     protected AbstractApp(final String name, final String intent,
             final String packageName) {
@@ -59,4 +59,9 @@ public abstract class AbstractApp implements App {
     protected static String getString(int ressourceId) {
         return cgeoapplication.getInstance().getString(ressourceId);
     }
+
+    @Override
+    public boolean isEnabled(cgCache cache) {
+        return true;
+    }
 }
diff --git a/main/src/cgeo/geocaching/apps/App.java b/main/src/cgeo/geocaching/apps/App.java
index 8241737..f3cefc3 100644
--- a/main/src/cgeo/geocaching/apps/App.java
+++ b/main/src/cgeo/geocaching/apps/App.java
@@ -1,5 +1,7 @@
 package cgeo.geocaching.apps;
 
+import cgeo.geocaching.cgCache;
+
 import android.content.Context;
 
 public interface App {
@@ -8,4 +10,12 @@ public interface App {
     public String getName();
 
     int getId();
+
+    /**
+     * whether or not the app can be used with the given cache (may depend on properties of the cache)
+     * 
+     * @param cache
+     * @return
+     */
+    boolean isEnabled(final cgCache cache);
 }
diff --git a/main/src/cgeo/geocaching/apps/cache/AbstractGeneralApp.java b/main/src/cgeo/geocaching/apps/cache/AbstractGeneralApp.java
index f4e92ac..2b1dff4 100644
--- a/main/src/cgeo/geocaching/apps/cache/AbstractGeneralApp.java
+++ b/main/src/cgeo/geocaching/apps/cache/AbstractGeneralApp.java
@@ -9,20 +9,11 @@ import android.content.Intent;
 abstract class AbstractGeneralApp extends AbstractApp implements GeneralApp {
 
     protected AbstractGeneralApp(String name, String packageName) {
-        super(name, null);
-        this.packageName = packageName;
-    }
-
-    @Override
-    public boolean isEnabled(cgCache cache) {
-        return true;
+        super(name, null, packageName);
     }
 
     @Override
     public boolean invoke(Activity activity, cgCache cache) {
-        if (packageName == null) {
-            return false;
-        }
         final Intent intent = getLaunchIntent(activity);
         if (intent != null) {
             intent.setFlags(Intent.FLAG_ACTIVITY_REORDER_TO_FRONT);
diff --git a/main/src/cgeo/geocaching/apps/cache/GeneralApp.java b/main/src/cgeo/geocaching/apps/cache/GeneralApp.java
index 805d64c..cdbedd5 100644
--- a/main/src/cgeo/geocaching/apps/cache/GeneralApp.java
+++ b/main/src/cgeo/geocaching/apps/cache/GeneralApp.java
@@ -7,8 +7,6 @@ import android.app.Activity;
 
 interface GeneralApp extends App {
 
-    boolean isEnabled(final cgCache cache);
-
     public boolean invoke(Activity activity, cgCache cache);
 
 }
diff --git a/main/src/cgeo/geocaching/apps/cache/navi/AbstractNavigationApp.java b/main/src/cgeo/geocaching/apps/cache/navi/AbstractNavigationApp.java
index 3b10f1a..9d58457 100644
--- a/main/src/cgeo/geocaching/apps/cache/navi/AbstractNavigationApp.java
+++ b/main/src/cgeo/geocaching/apps/cache/navi/AbstractNavigationApp.java
@@ -1,5 +1,6 @@
 package cgeo.geocaching.apps.cache.navi;
 
+import cgeo.geocaching.cgWaypoint;
 import cgeo.geocaching.apps.AbstractApp;
 
 abstract class AbstractNavigationApp extends AbstractApp implements NavigationApp {
@@ -12,4 +13,8 @@ abstract class AbstractNavigationApp extends AbstractApp implements NavigationAp
         super(name, intent);
     }
 
+    @Override
+    public boolean isEnabled(cgWaypoint waypoint) {
+        return true;
+    }
 }
diff --git a/main/src/cgeo/geocaching/apps/cache/navi/AbstractStaticMapsApp.java b/main/src/cgeo/geocaching/apps/cache/navi/AbstractStaticMapsApp.java
new file mode 100644
index 0000000..cfef583
--- /dev/null
+++ b/main/src/cgeo/geocaching/apps/cache/navi/AbstractStaticMapsApp.java
@@ -0,0 +1,70 @@
+package cgeo.geocaching.apps.cache.navi;
+
+import cgeo.geocaching.ILogable;
+import cgeo.geocaching.R;
+import cgeo.geocaching.StaticMapsActivity;
+import cgeo.geocaching.StaticMapsProvider;
+import cgeo.geocaching.cgCache;
+import cgeo.geocaching.cgWaypoint;
+import cgeo.geocaching.cgeoapplication;
+import cgeo.geocaching.activity.ActivityMixin;
+
+import org.apache.commons.lang3.StringUtils;
+
+import android.app.Activity;
+import android.content.Context;
+import android.content.Intent;
+
+abstract class AbstractStaticMapsApp extends AbstractNavigationApp {
+    public AbstractStaticMapsApp(String name) {
+        super(name, null);
+    }
+
+    @Override
+    public boolean isInstalled(Context context) {
+        return true;
+    }
+
+    protected static boolean hasStaticMap(cgCache cache) {
+        if (cache != null) {
+            String geocode = cache.getGeocode();
+            if (StringUtils.isNotEmpty(geocode) && cgeoapplication.getInstance().isOffline(geocode, null)) {
+                return StaticMapsProvider.doesExistStaticMapForCache(geocode);
+            }
+        }
+        return false;
+    }
+
+    protected static boolean hasStaticMap(cgWaypoint waypoint) {
+        if (waypoint != null) {
+            String geocode = waypoint.getGeocode();
+            int id = waypoint.getId();
+            if (StringUtils.isNotEmpty(geocode) && cgeoapplication.getInstance().isOffline(geocode, null)) {
+                return StaticMapsProvider.doesExistStaticMapForWaypoint(geocode, id);
+            }
+        }
+        return false;
+    }
+
+    protected static boolean invoke(final Activity activity, final cgCache cache, final cgWaypoint waypoint, final boolean download) {
+        final ILogable logable = cache != null && cache.getListId() != 0 ? cache : waypoint;
+        final String geocode = logable.getGeocode().toUpperCase();
+        if (geocode == null) {
+            ActivityMixin.showToast(activity, getString(R.string.err_detail_no_map_static));
+            return true;
+        }
+
+        final Intent intent = new Intent(activity, StaticMapsActivity.class);
+        intent.putExtra("geocode", geocode);
+        if (download) {
+            intent.putExtra("download", true);
+        }
+        if (waypoint != null) {
+            intent.putExtra("waypoint", waypoint.getId());
+        }
+        activity.startActivity(intent);
+
+        return true;
+    }
+
+}
diff --git a/main/src/cgeo/geocaching/apps/cache/navi/DownloadStaticMapsApp.java b/main/src/cgeo/geocaching/apps/cache/navi/DownloadStaticMapsApp.java
index d43cfc9..b6407e7 100644
--- a/main/src/cgeo/geocaching/apps/cache/navi/DownloadStaticMapsApp.java
+++ b/main/src/cgeo/geocaching/apps/cache/navi/DownloadStaticMapsApp.java
@@ -1,46 +1,31 @@
 package cgeo.geocaching.apps.cache.navi;
 
 import cgeo.geocaching.IGeoData;
-import cgeo.geocaching.ILogable;
 import cgeo.geocaching.R;
-import cgeo.geocaching.StaticMapsActivity;
 import cgeo.geocaching.cgCache;
 import cgeo.geocaching.cgWaypoint;
-import cgeo.geocaching.activity.ActivityMixin;
 import cgeo.geocaching.geopoint.Geopoint;
 
 import android.app.Activity;
-import android.content.Context;
-import android.content.Intent;
 
-class DownloadStaticMapsApp extends AbstractNavigationApp {
+class DownloadStaticMapsApp extends AbstractStaticMapsApp {
 
     DownloadStaticMapsApp() {
-        super(getString(R.string.cache_menu_download_map_static), null);
+        super(getString(R.string.cache_menu_download_map_static));
     }
 
     @Override
-    public boolean isInstalled(Context context) {
-        return true;
+    public boolean invoke(IGeoData geo, Activity activity, cgCache cache, cgWaypoint waypoint, final Geopoint coords) {
+        return invoke(activity, cache, waypoint, true);
     }
 
     @Override
-    public boolean invoke(IGeoData geo, Activity activity, cgCache cache, cgWaypoint waypoint, final Geopoint coords) {
-        final ILogable logable = cache != null && cache.getListId() != 0 ? cache : waypoint;
-        final String geocode = logable.getGeocode().toUpperCase();
-        if (geocode == null) {
-            ActivityMixin.showToast(activity, getString(R.string.err_detail_no_map_static));
-            return true;
-        }
-
-        final Intent intent = new Intent(activity, StaticMapsActivity.class);
-        intent.putExtra("download", true);
-        intent.putExtra("geocode", geocode);
-        if (waypoint != null) {
-            intent.putExtra("waypoint", waypoint.getId());
-        }
-        activity.startActivity(intent);
+    public boolean isEnabled(cgCache cache) {
+        return !hasStaticMap(cache);
+    }
 
-        return true;
+    @Override
+    public boolean isEnabled(cgWaypoint waypoint) {
+        return !hasStaticMap(waypoint);
     }
 }
diff --git a/main/src/cgeo/geocaching/apps/cache/navi/LocusApp.java b/main/src/cgeo/geocaching/apps/cache/navi/LocusApp.java
index 60e3315..43e86c4 100644
--- a/main/src/cgeo/geocaching/apps/cache/navi/LocusApp.java
+++ b/main/src/cgeo/geocaching/apps/cache/navi/LocusApp.java
@@ -1,9 +1,9 @@
 package cgeo.geocaching.apps.cache.navi;
 
 import cgeo.geocaching.IGeoData;
-import cgeo.geocaching.apps.AbstractLocusApp;
 import cgeo.geocaching.cgCache;
 import cgeo.geocaching.cgWaypoint;
+import cgeo.geocaching.apps.AbstractLocusApp;
 import cgeo.geocaching.geopoint.Geopoint;
 
 import android.app.Activity;
@@ -37,4 +37,9 @@ class LocusApp extends AbstractLocusApp implements NavigationApp {
         return showInLocus(points, true, false, activity);
     }
 
+    @Override
+    public boolean isEnabled(cgWaypoint waypoint) {
+        return true;
+    }
+
 }
diff --git a/main/src/cgeo/geocaching/apps/cache/navi/NavigationApp.java b/main/src/cgeo/geocaching/apps/cache/navi/NavigationApp.java
index 62629de..f073331 100644
--- a/main/src/cgeo/geocaching/apps/cache/navi/NavigationApp.java
+++ b/main/src/cgeo/geocaching/apps/cache/navi/NavigationApp.java
@@ -1,9 +1,9 @@
 package cgeo.geocaching.apps.cache.navi;
 
 import cgeo.geocaching.IGeoData;
-import cgeo.geocaching.apps.App;
 import cgeo.geocaching.cgCache;
 import cgeo.geocaching.cgWaypoint;
+import cgeo.geocaching.apps.App;
 import cgeo.geocaching.geopoint.Geopoint;
 
 import android.app.Activity;
@@ -12,4 +12,6 @@ public interface NavigationApp extends App {
     public boolean invoke(final IGeoData geo, final Activity activity,
             final cgCache cache, final cgWaypoint waypoint,
             final Geopoint coords);
+
+    boolean isEnabled(final cgWaypoint waypoint);
 }
diff --git a/main/src/cgeo/geocaching/apps/cache/navi/NavigationAppFactory.java b/main/src/cgeo/geocaching/apps/cache/navi/NavigationAppFactory.java
index 9caa856..4602a04 100644
--- a/main/src/cgeo/geocaching/apps/cache/navi/NavigationAppFactory.java
+++ b/main/src/cgeo/geocaching/apps/cache/navi/NavigationAppFactory.java
@@ -3,16 +3,12 @@ package cgeo.geocaching.apps.cache.navi;
 import cgeo.geocaching.IGeoData;
 import cgeo.geocaching.R;
 import cgeo.geocaching.Settings;
-import cgeo.geocaching.StaticMapsProvider;
 import cgeo.geocaching.cgCache;
 import cgeo.geocaching.cgWaypoint;
-import cgeo.geocaching.cgeoapplication;
 import cgeo.geocaching.apps.AbstractAppFactory;
 import cgeo.geocaching.geopoint.Geopoint;
 import cgeo.geocaching.utils.Log;
 
-import org.apache.commons.lang3.StringUtils;
-
 import android.app.Activity;
 import android.app.AlertDialog;
 import android.content.DialogInterface;
@@ -34,22 +30,22 @@ public final class NavigationAppFactory extends AbstractAppFactory {
         INTERNAL_MAP(new InternalMap(), 2),
         /** The internal static map activity */
         STATIC_MAP(new StaticMapApp(), 3),
-        /** Download static maps on demand */
-        DOWNLOAD_STATIC_MAPS(new DownloadStaticMapsApp(), 4),
         /** The external Locus app */
-        LOCUS(new LocusApp(), 5),
+        DOWNLOAD_STATIC_MAPS(new DownloadStaticMapsApp(), 20),
+        /** The external Locus app */
+        LOCUS(new LocusApp(), 4),
         /** The external RMaps app */
-        RMAPS(new RMapsApp(), 6),
+        RMAPS(new RMapsApp(), 5),
         /** Google Maps */
-        GOOGLE_MAPS(new GoogleMapsApp(), 7),
+        GOOGLE_MAPS(new GoogleMapsApp(), 6),
         /** Google Navigation */
-        GOOGLE_NAVIGATION(new GoogleNavigationApp(), 8),
+        GOOGLE_NAVIGATION(new GoogleNavigationApp(), 7),
         /** Google Streetview */
-        GOOGLE_STREETVIEW(new StreetviewApp(), 9),
+        GOOGLE_STREETVIEW(new StreetviewApp(), 8),
         /** The external OruxMaps app */
-        ORUX_MAPS(new OruxMapsApp(), 10),
+        ORUX_MAPS(new OruxMapsApp(), 9),
         /** The external navigon app */
-        NAVIGON(new NavigonApp(), 11);
+        NAVIGON(new NavigonApp(), 10);
 
         NavigationAppsEnum(NavigationApp app, int id) {
             this.app = app;
@@ -122,19 +118,12 @@ public final class NavigationAppFactory extends AbstractAppFactory {
         builder.setIcon(R.drawable.ic_menu_mapmode);
         final List<NavigationAppsEnum> items = new ArrayList<NavigationAppFactory.NavigationAppsEnum>();
         final int defaultNavigationTool = Settings.getDefaultNavigationTool();
-        final boolean hasStaticMaps = hasStaticMap(cache, waypoint);
         for (NavigationAppsEnum navApp : getInstalledNavigationApps(activity)) {
-            if (NavigationAppsEnum.STATIC_MAP.id == navApp.id) {
-                if (hasStaticMaps) {
-                    items.add(navApp);
-                }
-            } else if (NavigationAppsEnum.DOWNLOAD_STATIC_MAPS.id == navApp.id) {
-                if (!hasStaticMaps) {
+            if ((showInternalMap || !(navApp.app instanceof InternalMap)) &&
+                    (showDefaultNavigation || defaultNavigationTool != navApp.id)) {
+                if ((cache != null && navApp.app.isEnabled(cache)) || (waypoint != null && navApp.app.isEnabled(waypoint))) {
                     items.add(navApp);
                 }
-            } else if ((showInternalMap || !(navApp.app instanceof InternalMap)) &&
-                    (showDefaultNavigation || defaultNavigationTool != navApp.id)) {
-                items.add(navApp);
             }
         }
         /*
@@ -153,23 +142,6 @@ public final class NavigationAppFactory extends AbstractAppFactory {
         alert.show();
     }
 
-    private static boolean hasStaticMap(cgCache cache, cgWaypoint waypoint) {
-        if (waypoint != null) {
-            String geocode = waypoint.getGeocode();
-            int id = waypoint.getId();
-            if (StringUtils.isNotEmpty(geocode) && cgeoapplication.getInstance().isOffline(geocode, null)) {
-                return StaticMapsProvider.doesExistStaticMapForWaypoint(geocode, id);
-            }
-        }
-        if (cache != null) {
-            String geocode = cache.getGeocode();
-            if (StringUtils.isNotEmpty(geocode) && cgeoapplication.getInstance().isOffline(geocode, null)) {
-                return StaticMapsProvider.doesExistStaticMapForCache(geocode);
-            }
-        }
-        return false;
-    }
-
     /**
      * Returns all installed navigation apps.
      *
@@ -304,8 +276,10 @@ public final class NavigationAppFactory extends AbstractAppFactory {
      * @return never <code>null</code>
      */
     public static NavigationApp getDefaultNavigationApplication(Activity activity) {
-        final int defaultNavigationTool = Settings.getDefaultNavigationTool();
+        return getNavigationAppFromSetting(activity, Settings.getDefaultNavigationTool());
+    }
 
+    private static NavigationApp getNavigationAppFromSetting(Activity activity, final int defaultNavigationTool) {
         final List<NavigationAppsEnum> installedNavigationApps = getInstalledNavigationApps(activity);
 
         for (NavigationAppsEnum navigationApp : installedNavigationApps) {
@@ -324,17 +298,7 @@ public final class NavigationAppFactory extends AbstractAppFactory {
      * @return never <code>null</code>
      */
     public static NavigationApp getDefaultNavigationApplication2(Activity activity) {
-        final int defaultNavigationTool = Settings.getDefaultNavigationTool2();
-
-        final List<NavigationAppsEnum> installedNavigationApps = getInstalledNavigationApps(activity);
-
-        for (NavigationAppsEnum navigationApp : installedNavigationApps) {
-            if (navigationApp.id == defaultNavigationTool) {
-                return navigationApp.app;
-            }
-        }
-        // second default navigation tool wasn't set already or couldn't be found (not installed any more for example)
-        return NavigationAppsEnum.COMPASS.app;
+        return getNavigationAppFromSetting(activity, Settings.getDefaultNavigationTool2());
     }
 
 }
diff --git a/main/src/cgeo/geocaching/apps/cache/navi/StaticMapApp.java b/main/src/cgeo/geocaching/apps/cache/navi/StaticMapApp.java
index d8b66c2..2f6e7c0 100644
--- a/main/src/cgeo/geocaching/apps/cache/navi/StaticMapApp.java
+++ b/main/src/cgeo/geocaching/apps/cache/navi/StaticMapApp.java
@@ -1,45 +1,31 @@
 package cgeo.geocaching.apps.cache.navi;
 
 import cgeo.geocaching.IGeoData;
-import cgeo.geocaching.ILogable;
 import cgeo.geocaching.R;
-import cgeo.geocaching.StaticMapsActivity;
-import cgeo.geocaching.activity.ActivityMixin;
 import cgeo.geocaching.cgCache;
 import cgeo.geocaching.cgWaypoint;
 import cgeo.geocaching.geopoint.Geopoint;
 
 import android.app.Activity;
-import android.content.Context;
-import android.content.Intent;
 
-class StaticMapApp extends AbstractNavigationApp {
+class StaticMapApp extends AbstractStaticMapsApp {
 
     StaticMapApp() {
-        super(getString(R.string.cache_menu_map_static), null);
+        super(getString(R.string.cache_menu_map_static));
     }
 
     @Override
-    public boolean isInstalled(Context context) {
-        return true;
+    public boolean invoke(IGeoData geo, Activity activity, cgCache cache, cgWaypoint waypoint, final Geopoint coords) {
+        return invoke(activity, cache, waypoint, false);
     }
 
     @Override
-    public boolean invoke(IGeoData geo, Activity activity, cgCache cache, cgWaypoint waypoint, final Geopoint coords) {
-        final ILogable logable = cache != null && cache.getListId() != 0 ? cache : waypoint;
-        final String geocode = logable.getGeocode().toUpperCase();
-        if (geocode == null) {
-            ActivityMixin.showToast(activity, getString(R.string.err_detail_no_map_static));
-            return true;
-        }
-
-        final Intent intent = new Intent(activity, StaticMapsActivity.class);
-        intent.putExtra("geocode", geocode);
-        if (waypoint != null) {
-            intent.putExtra("waypoint", waypoint.getId());
-        }
-        activity.startActivity(intent);
+    public boolean isEnabled(cgCache cache) {
+        return hasStaticMap(cache);
+    }
 
-        return true;
+    @Override
+    public boolean isEnabled(cgWaypoint waypoint) {
+        return hasStaticMap(waypoint);
     }
 }