fix #4359: imperial units checkbox not always correct

This has been fixed by initializing the key to its default value instead
of just the checkbox. Also, better names have been used throughout the
settings to reflect the fact that "units" meant "use imperial units". It
was used in both ways, including the wrong way while migrating the
settings.

12 files changed, 30 insertions, 22 deletions

diff --git a/main/res/values/preference_keys.xml b/main/res/values/preference_keys.xml
index aa9b77f..4787308 100644
--- a/main/res/values/preference_keys.xml
+++ b/main/res/values/preference_keys.xml
@@ -49,7 +49,7 @@
     <string name="pref_skin">skin</string>
     <string name="pref_showaddress">showaddress</string>
     <string name="pref_useenglish">useenglish</string>
-    <string name="pref_units">units</string>
+    <string name="pref_units_imperial">units</string>
     <string name="pref_autoloaddesc">autoloaddesc</string>
     <string name="pref_ratingwanted">ratingwanted</string>
     <string name="pref_friendlogswanted">friendlogswanted</string>
diff --git a/main/res/xml/preferences.xml b/main/res/xml/preferences.xml
index 78d79bd..362c633 100644
--- a/main/res/xml/preferences.xml
+++ b/main/res/xml/preferences.xml
@@ -430,7 +430,7 @@
             android:title="@string/init_useenglish" />
         <CheckBoxPreference
             android:defaultValue="false"
-            android:key="@string/pref_units"
+            android:key="@string/pref_units_imperial"
             android:summary="@string/init_summary_units"
             android:title="@string/init_units" />
     </PreferenceScreen>
diff --git a/main/src/cgeo/geocaching/EditWaypointActivity.java b/main/src/cgeo/geocaching/EditWaypointActivity.java
index 2666cdd..46a0000 100644
--- a/main/src/cgeo/geocaching/EditWaypointActivity.java
+++ b/main/src/cgeo/geocaching/EditWaypointActivity.java
@@ -250,7 +250,7 @@ public class EditWaypointActivity extends AbstractActionBarActivity implements C
     private void initializeDistanceUnitSelector() {
         distanceUnits = new ArrayList<>(Arrays.asList(res.getStringArray(R.array.distance_units)));
         if (initViews) {
-            distanceUnitSelector.setSelection(Settings.isUseImperialUnits() ? 2 : 0); //0:m, 2:ft
+            distanceUnitSelector.setSelection(Settings.useImperialUnits() ? 2 : 0); //0:m, 2:ft
         }
     }
 
@@ -397,7 +397,7 @@ public class EditWaypointActivity extends AbstractActionBarActivity implements C
                 double distance;
                 try {
                     distance = DistanceParser.parseDistance(distanceText,
-                            !Settings.isUseImperialUnits());
+                            !Settings.useImperialUnits());
                 } catch (final NumberFormatException ignored) {
                     showToast(res.getString(R.string.err_parse_dist));
                     return;
diff --git a/main/src/cgeo/geocaching/NavigateAnyPointActivity.java b/main/src/cgeo/geocaching/NavigateAnyPointActivity.java
index e148717..925e5b0 100644
--- a/main/src/cgeo/geocaching/NavigateAnyPointActivity.java
+++ b/main/src/cgeo/geocaching/NavigateAnyPointActivity.java
@@ -261,7 +261,7 @@ public class NavigateAnyPointActivity extends AbstractActionBarActivity implemen
 
     private void initializeDistanceUnitSelector() {
         if (StringUtils.isBlank(distanceUnit)) {
-            if (Settings.isUseImperialUnits()) {
+            if (Settings.useImperialUnits()) {
                 distanceUnitSelector.setSelection(2); // ft
                 distanceUnit = res.getStringArray(R.array.distance_units)[2];
             } else {
@@ -526,7 +526,7 @@ public class NavigateAnyPointActivity extends AbstractActionBarActivity implemen
             double distance;
             try {
                 distance = DistanceParser.parseDistance(distanceText,
-                        !Settings.isUseImperialUnits());
+                        !Settings.useImperialUnits());
             } catch (final NumberFormatException ignored) {
                 showToast(res.getString(R.string.err_parse_dist));
                 return null;
diff --git a/main/src/cgeo/geocaching/connector/gc/GCParser.java b/main/src/cgeo/geocaching/connector/gc/GCParser.java
index 6cb1bbe..d814bbd 100644
--- a/main/src/cgeo/geocaching/connector/gc/GCParser.java
+++ b/main/src/cgeo/geocaching/connector/gc/GCParser.java
@@ -192,7 +192,7 @@ public abstract class GCParser {
             final String distance = TextUtils.getMatch(row, GCConstants.PATTERN_SEARCH_DIRECTION_DISTANCE, false, 2, null, false);
             if (distance != null) {
                 cache.setDistance(DistanceParser.parseDistance(distance,
-                        !Settings.isUseImperialUnits()));
+                        !Settings.useImperialUnits()));
             }
 
             // difficulty/terrain
@@ -1544,7 +1544,7 @@ public abstract class GCParser {
         if (null != distance) {
             try {
                 trackable.setDistance(DistanceParser.parseDistance(distance,
-                        !Settings.isUseImperialUnits()));
+                        !Settings.useImperialUnits()));
             } catch (final NumberFormatException e) {
                 Log.e("GCParser.parseTrackable: Failed to parse distance", e);
             }
diff --git a/main/src/cgeo/geocaching/geopoint/Units.java b/main/src/cgeo/geocaching/geopoint/Units.java
index 018216d..c657ede 100644
--- a/main/src/cgeo/geocaching/geopoint/Units.java
+++ b/main/src/cgeo/geocaching/geopoint/Units.java
@@ -11,7 +11,7 @@ public class Units {
     public static ImmutablePair<Double, String> scaleDistance(final double distanceKilometers) {
         double distance;
         String units;
-        if (Settings.isUseImperialUnits()) {
+        if (Settings.useImperialUnits()) {
             distance = distanceKilometers / IConversion.MILES_TO_KILOMETER;
             if (distance >= 0.1) {
                 units = "mi";
@@ -54,7 +54,7 @@ public class Units {
     }
 
     public static String getSpeed(final float kilometersPerHour) {
-        if (Settings.isUseImperialUnits()) {
+        if (Settings.useImperialUnits()) {
             return String.format(Locale.US, "%.0f mph", kilometersPerHour / IConversion.MILES_TO_KILOMETER);
         }
         return String.format(Locale.US, "%.0f km/h", kilometersPerHour);
diff --git a/main/src/cgeo/geocaching/settings/Settings.java b/main/src/cgeo/geocaching/settings/Settings.java
index f1eaaee..aeaf078 100644
--- a/main/src/cgeo/geocaching/settings/Settings.java
+++ b/main/src/cgeo/geocaching/settings/Settings.java
@@ -149,7 +149,7 @@ public class Settings {
             e.putBoolean(getKey(R.string.pref_maptrail), prefsV0.getInt(getKey(R.string.pref_maptrail), 1) != 0);
             e.putInt(getKey(R.string.pref_lastmapzoom), prefsV0.getInt(getKey(R.string.pref_lastmapzoom), 14));
             e.putBoolean(getKey(R.string.pref_livelist), 0 != prefsV0.getInt(getKey(R.string.pref_livelist), 1));
-            e.putBoolean(getKey(R.string.pref_units), prefsV0.getInt(getKey(R.string.pref_units), unitsMetric) == unitsMetric);
+            e.putBoolean(getKey(R.string.pref_units_imperial), prefsV0.getInt(getKey(R.string.pref_units_imperial), unitsMetric) != unitsMetric);
             e.putBoolean(getKey(R.string.pref_skin), prefsV0.getInt(getKey(R.string.pref_skin), 0) != 0);
             e.putInt(getKey(R.string.pref_lastusedlist), prefsV0.getInt(getKey(R.string.pref_lastusedlist), StoredList.STANDARD_LIST_ID));
             e.putString(getKey(R.string.pref_cachetype), prefsV0.getString(getKey(R.string.pref_cachetype), CacheType.ALL.id));
@@ -195,7 +195,7 @@ public class Settings {
         if (currentVersion < 2) {
             final Editor e = sharedPrefs.edit();
 
-            e.putBoolean(getKey(R.string.pref_units), !isUseImperialUnits());
+            e.putBoolean(getKey(R.string.pref_units_imperial), useImperialUnits());
 
             // show waypoints threshold now as a slider
             int wpThreshold = getWayPointsThreshold();
@@ -595,11 +595,15 @@ public class Settings {
         return getBoolean(R.string.pref_sigautoinsert, false);
     }
 
-    public static boolean isUseImperialUnits() {
-        return getBoolean(R.string.pref_units, getImperialUnitsDefault());
+    public static void setUseImperialUnits(final boolean useImperialUnits) {
+        putBoolean(R.string.pref_units_imperial, useImperialUnits);
     }
 
-    static boolean getImperialUnitsDefault() {
+    public static boolean useImperialUnits() {
+        return getBoolean(R.string.pref_units_imperial, useImperialUnitsByDefault());
+    }
+
+    static boolean useImperialUnitsByDefault() {
         final String countryCode = Locale.getDefault().getCountry();
         return "US".equals(countryCode)  // USA
             || "LR".equals(countryCode)  // Liberia
diff --git a/main/src/cgeo/geocaching/settings/SettingsActivity.java b/main/src/cgeo/geocaching/settings/SettingsActivity.java
index b678239..187d124 100644
--- a/main/src/cgeo/geocaching/settings/SettingsActivity.java
+++ b/main/src/cgeo/geocaching/settings/SettingsActivity.java
@@ -89,6 +89,7 @@ public class SettingsActivity extends PreferenceActivity {
         super.onCreate(savedInstanceState);
 
         initHardwareAccelerationPreferences();
+        initUnitPreferences();
         SettingsActivity.addPreferencesFromResource(this, R.xml.preferences);
         initPreferences();
 
@@ -142,7 +143,6 @@ public class SettingsActivity extends PreferenceActivity {
                 R.string.pref_ecusername, R.string.pref_ecpassword, R.string.pref_ec_icons }) {
             bindSummaryToStringValue(k);
         }
-        getPreference(R.string.pref_units).setDefaultValue(Settings.getImperialUnitsDefault());
     }
 
     private void initNavigationMenuPreferences() {
@@ -387,6 +387,10 @@ public class SettingsActivity extends PreferenceActivity {
         Settings.setUseHardwareAcceleration(Settings.useHardwareAcceleration());
     }
 
+    private static void initUnitPreferences() {
+        Settings.setUseImperialUnits(Settings.useImperialUnits());
+    }
+
     private void initDbLocationPreference() {
         final Preference p = getPreference(R.string.pref_dbonsdcard);
         p.setPersistent(false);
diff --git a/main/src/cgeo/geocaching/speech/TextFactory.java b/main/src/cgeo/geocaching/speech/TextFactory.java
index eb780c6..4b87f1a 100644
--- a/main/src/cgeo/geocaching/speech/TextFactory.java
+++ b/main/src/cgeo/geocaching/speech/TextFactory.java
@@ -31,7 +31,7 @@ public class TextFactory {
     private static String getDistance(Geopoint position, Geopoint target) {
         final float kilometers = position.distanceTo(target);
 
-        if (Settings.isUseImperialUnits()) {
+        if (Settings.useImperialUnits()) {
             return getDistance(kilometers / IConversion.MILES_TO_KILOMETER,
                     (int) (kilometers * 1000.0 * IConversion.METERS_TO_FEET),
                     3.0f, 0.2f, 300,
diff --git a/tests/src/cgeo/geocaching/geopoint/UnitsTest.java b/tests/src/cgeo/geocaching/geopoint/UnitsTest.java
index f208984..7f3bdab 100644
--- a/tests/src/cgeo/geocaching/geopoint/UnitsTest.java
+++ b/tests/src/cgeo/geocaching/geopoint/UnitsTest.java
@@ -14,7 +14,7 @@ public class UnitsTest extends CGeoTestCase {
         final String actual = Units.getDistanceFromKilometers(distance);
         if (!StringUtils.equals(expected, actual.replace(',', '.'))) { // make 1.2 the same as 1,2
             fail("getHumanDistance(" + distance +
-                    ") [metric: " + (!Settings.isUseImperialUnits() ? "yes" : "no") +
+                    ") [metric: " + (!Settings.useImperialUnits() ? "yes" : "no") +
                     "] fails to match " + expected + ": " + actual);
         }
     }
@@ -23,7 +23,7 @@ public class UnitsTest extends CGeoTestCase {
     @SuppressWarnings("static-method")
     public void testDistance() {
         assertThat(Units.getDistanceFromKilometers(null)).isEqualTo("?");
-        final boolean savedImperial = Settings.isUseImperialUnits();
+        final boolean savedImperial = Settings.useImperialUnits();
         try {
             TestSettings.setUseImperialUnits(false);
             assertDistance("123 km", 122.782f);
@@ -46,7 +46,7 @@ public class UnitsTest extends CGeoTestCase {
     @SuppressWarnings("static-method")
     public void testSpeed() {
         assertThat(Units.getDistanceFromKilometers(null)).isEqualTo("?");
-        final boolean savedImperial = Settings.isUseImperialUnits();
+        final boolean savedImperial = Settings.useImperialUnits();
         try {
             TestSettings.setUseImperialUnits(false);
             assertSpeed("123 km/h", 122.782f);
diff --git a/tests/src/cgeo/geocaching/settings/TestSettings.java b/tests/src/cgeo/geocaching/settings/TestSettings.java
index 279f658..b82c120 100644
--- a/tests/src/cgeo/geocaching/settings/TestSettings.java
+++ b/tests/src/cgeo/geocaching/settings/TestSettings.java
@@ -37,7 +37,7 @@ public final class TestSettings extends Settings {
     }
 
     public static void setUseImperialUnits(final boolean imperial) {
-        putBoolean(R.string.pref_units, imperial);
+        putBoolean(R.string.pref_units_imperial, imperial);
     }
 
     public static void setCacheTwitterMessage(final String template) {
diff --git a/tests/src/cgeo/geocaching/speech/TextFactoryTest.java b/tests/src/cgeo/geocaching/speech/TextFactoryTest.java
index c53aec9..1f309ba 100644
--- a/tests/src/cgeo/geocaching/speech/TextFactoryTest.java
+++ b/tests/src/cgeo/geocaching/speech/TextFactoryTest.java
@@ -35,7 +35,7 @@ public class TextFactoryTest extends AndroidTestCase {
         final Configuration config = resources.getConfiguration();
         defaultLocale1 = config.locale;
         defaultLocale2 = Locale.getDefault();
-        defaultMetric = !Settings.isUseImperialUnits();
+        defaultMetric = !Settings.useImperialUnits();
     }
 
     @Override