refactoring: more encapsulation in SearchResult

10 files changed, 103 insertions, 34 deletions

diff --git a/main/src/cgeo/geocaching/SearchResult.java b/main/src/cgeo/geocaching/SearchResult.java
index bac9c23..e7c9a88 100644
--- a/main/src/cgeo/geocaching/SearchResult.java
+++ b/main/src/cgeo/geocaching/SearchResult.java
@@ -22,10 +22,10 @@ import java.util.Set;
 public class SearchResult implements Parcelable {
 
     final private Set<String> geocodes;
-    public StatusCode error = null;
+    private StatusCode error = null;
     private String url = "";
     public String[] viewstates = null;
-    public int totalCnt = 0;
+    private int totalCnt = 0;
 
     final public static Parcelable.Creator<SearchResult> CREATOR = new Parcelable.Creator<SearchResult>() {
         public SearchResult createFromParcel(Parcel in) {
@@ -47,19 +47,24 @@ public class SearchResult implements Parcelable {
             this.error = searchResult.error;
             this.url = searchResult.url;
             this.viewstates = searchResult.viewstates;
-            this.totalCnt = searchResult.totalCnt;
+            this.setTotal(searchResult.getTotal());
         } else {
             this.geocodes = new HashSet<String>();
         }
     }
 
-    public SearchResult(final Set<String> geocodes) {
+    public SearchResult(final Set<String> geocodes, final int total) {
         if (geocodes == null) {
             this.geocodes = new HashSet<String>();
         } else {
             this.geocodes = new HashSet<String>(geocodes.size());
             this.geocodes.addAll(geocodes);
         }
+        this.setTotal(total);
+    }
+
+    public SearchResult(final Set<String> geocodes) {
+        this(geocodes, geocodes == null ? 0 : geocodes.size());
     }
 
     public SearchResult(final Parcel in) {
@@ -73,7 +78,7 @@ public class SearchResult implements Parcelable {
             viewstates = new String[length];
             in.readStringArray(viewstates);
         }
-        totalCnt = in.readInt();
+        setTotal(in.readInt());
     }
 
     @Override
@@ -87,7 +92,7 @@ public class SearchResult implements Parcelable {
             out.writeInt(viewstates.length);
             out.writeStringArray(viewstates);
         }
-        out.writeInt(totalCnt);
+        out.writeInt(getTotal());
     }
 
     @Override
@@ -135,6 +140,10 @@ public class SearchResult implements Parcelable {
         return totalCnt;
     }
 
+    public void setTotal(int totalCnt) {
+        this.totalCnt = totalCnt;
+    }
+
     /**
      * @param excludeDisabled
      * @param excludeMine
diff --git a/main/src/cgeo/geocaching/cgBase.java b/main/src/cgeo/geocaching/cgBase.java
index 0d39608..66354e8 100644
--- a/main/src/cgeo/geocaching/cgBase.java
+++ b/main/src/cgeo/geocaching/cgBase.java
@@ -300,7 +300,7 @@ public class cgBase {
         try {
             String result = BaseUtils.getMatch(page, GCConstants.PATTERN_SEARCH_TOTALCOUNT, false, 1, null, true);
             if (null != result) {
-                searchResult.totalCnt = Integer.parseInt(result);
+                searchResult.setTotal(Integer.parseInt(result));
             }
         } catch (NumberFormatException e) {
             Log.w(Settings.tag, "cgeoBase.parseSearch: Failed to parse cache count");
@@ -347,7 +347,7 @@ public class cgBase {
                     if (coordinates.contains("You have not agreed to the license agreement. The license agreement is required before you can start downloading GPX or LOC files from Geocaching.com")) {
                         Log.i(Settings.tag, "User has not agreed to the license agreement. Can\'t download .loc file.");
 
-                        searchResult.error = StatusCode.UNAPPROVED_LICENSE;
+                        searchResult.setError(StatusCode.UNAPPROVED_LICENSE);
 
                         return searchResult;
                     }
@@ -401,23 +401,23 @@ public class cgBase {
         final SearchResult searchResult = new SearchResult();
 
         if (page.contains("Cache is Unpublished") || page.contains("you cannot view this cache listing until it has been published")) {
-            searchResult.error = StatusCode.UNPUBLISHED_CACHE;
+            searchResult.setError(StatusCode.UNPUBLISHED_CACHE);
             return searchResult;
         }
 
         if (page.contains("Sorry, the owner of this listing has made it viewable to Premium Members only.")) {
-            searchResult.error = StatusCode.PREMIUM_ONLY;
+            searchResult.setError(StatusCode.PREMIUM_ONLY);
             return searchResult;
         }
 
         if (page.contains("has chosen to make this cache listing visible to Premium Members only.")) {
-            searchResult.error = StatusCode.PREMIUM_ONLY;
+            searchResult.setError(StatusCode.PREMIUM_ONLY);
             return searchResult;
         }
 
         final String cacheName = Html.fromHtml(BaseUtils.getMatch(page, GCConstants.PATTERN_NAME, true, "")).toString();
         if ("An Error Has Occurred".equalsIgnoreCase(cacheName)) {
-            searchResult.error = StatusCode.UNKNOWN_ERROR;
+            searchResult.setError(StatusCode.UNKNOWN_ERROR);
             return searchResult;
         }
 
@@ -769,7 +769,7 @@ public class cgBase {
 
         // last check for necessary cache conditions
         if (StringUtils.isBlank(cache.getGeocode())) {
-            searchResult.error = StatusCode.UNKNOWN_ERROR;
+            searchResult.setError(StatusCode.UNKNOWN_ERROR);
             return searchResult;
         }
 
@@ -1308,7 +1308,7 @@ public class cgBase {
         }
 
         // save to application
-        search.setError(searchResult.error);
+        search.setError(searchResult.getError());
         search.setViewstates(searchResult.viewstates);
         for (String geocode : searchResult.getGeocodes()) {
             search.addGeocode(geocode);
diff --git a/main/src/cgeo/geocaching/cgeoapplication.java b/main/src/cgeo/geocaching/cgeoapplication.java
index e2b39d8..fb0cfd4 100644
--- a/main/src/cgeo/geocaching/cgeoapplication.java
+++ b/main/src/cgeo/geocaching/cgeoapplication.java
@@ -294,9 +294,7 @@ public class cgeoapplication extends Application {
     /** {@link cgData#loadBatchOfStoredGeocodes(boolean, Geopoint, CacheType, int)} */
     public SearchResult getBatchOfStoredCaches(final boolean detailedOnly, final Geopoint coords, final CacheType cacheType, final int listId) {
         final Set<String> geocodes = storage.loadBatchOfStoredGeocodes(detailedOnly, coords, cacheType, listId);
-        final SearchResult search = new SearchResult(geocodes);
-        search.totalCnt = getAllStoredCachesCount(true, cacheType, listId);
-        return search;
+        return new SearchResult(geocodes, getAllStoredCachesCount(true, cacheType, listId));
     }
 
     /** {@link cgData#loadHistoryOfSearchedLocations()} */
@@ -306,10 +304,7 @@ public class cgeoapplication extends Application {
 
     public SearchResult getHistoryOfCaches(final boolean detailedOnly, final CacheType cacheType) {
         final Set<String> geocodes = storage.loadBatchOfHistoricGeocodes(detailedOnly, cacheType);
-        final SearchResult search = new SearchResult(geocodes);
-
-        search.totalCnt = getAllHistoricCachesCount();
-        return search;
+        return new SearchResult(geocodes, getAllHistoricCachesCount());
     }
 
     /** {@link cgData#loadCachedInViewport(Long, Long, Long, Long, CacheType)} */
diff --git a/main/src/cgeo/geocaching/cgeocaches.java b/main/src/cgeo/geocaching/cgeocaches.java
index 21ecb5e..70d1b85 100644
--- a/main/src/cgeo/geocaching/cgeocaches.java
+++ b/main/src/cgeo/geocaching/cgeocaches.java
@@ -727,7 +727,7 @@ public class cgeocaches extends AbstractListActivity {
         // refresh standard list if it has changed (new caches downloaded)
         if (type == CacheListType.OFFLINE && listId >= StoredList.STANDARD_LIST_ID && search != null) {
             SearchResult newSearch = cgBase.searchByStored(coords, cacheType, listId);
-            if (newSearch != null && newSearch.totalCnt != search.totalCnt) {
+            if (newSearch != null && newSearch.getTotal() != search.getTotal()) {
                 refreshCurrentList();
             }
         }
diff --git a/main/src/cgeo/geocaching/connector/gc/GCConnector.java b/main/src/cgeo/geocaching/connector/gc/GCConnector.java
index 1c57508..d0c49eb 100644
--- a/main/src/cgeo/geocaching/connector/gc/GCConnector.java
+++ b/main/src/cgeo/geocaching/connector/gc/GCConnector.java
@@ -117,12 +117,12 @@ public class GCConnector extends AbstractConnector {
                 } else {
                     search.addGeocode(geocode);
                 }
-                search.error = StatusCode.NO_ERROR;
+                search.setError(StatusCode.NO_ERROR);
                 return search;
             }
 
             Log.e(Settings.tag, "cgeoBase.searchByGeocode: No data from server");
-            search.error = StatusCode.COMMUNICATION_ERROR;
+            search.setError(StatusCode.COMMUNICATION_ERROR);
             return search;
         }
 
diff --git a/main/src/cgeo/geocaching/maps/CGeoMap.java b/main/src/cgeo/geocaching/maps/CGeoMap.java
index fd1bf13..08be66a 100644
--- a/main/src/cgeo/geocaching/maps/CGeoMap.java
+++ b/main/src/cgeo/geocaching/maps/CGeoMap.java
@@ -779,11 +779,6 @@ public class CGeoMap extends AbstractMap implements OnMapDragListener, ViewFacto
                 ActivityMixin.invalidateOptionsMenu(activity);
                 return true;
             case MENU_AS_LIST: {
-                final SearchResult searchResult = new SearchResult();
-                search.totalCnt = caches.size();
-                for (cgCache cache : caches) {
-                    searchResult.addCache(cache);
-                }
                 cgeocaches.startActivityMap(activity, search);
                 return true;
             }
@@ -1208,7 +1203,7 @@ public class CGeoMap extends AbstractMap implements OnMapDragListener, ViewFacto
                     search = ConnectorFactory.searchByViewport(viewport, tokens);
                     if (search != null) {
                         downloaded = true;
-                        if (search.error == StatusCode.NOT_LOGGED_IN) {
+                        if (search.getError() == StatusCode.NOT_LOGGED_IN) {
                             Login.login();
                             tokens = null;
                         } else {
diff --git a/tests/src/cgeo/geocaching/SearchResultTest.java b/tests/src/cgeo/geocaching/SearchResultTest.java
new file mode 100644
index 0000000..8fcd188
--- /dev/null
+++ b/tests/src/cgeo/geocaching/SearchResultTest.java
@@ -0,0 +1,17 @@
+package cgeo.geocaching;
+
+import android.test.AndroidTestCase;
+
+import java.util.HashSet;
+
+public class SearchResultTest extends AndroidTestCase {
+    public static void testCreateFromGeocodes() {
+        final HashSet<String> geocodes = new HashSet<String>();
+        geocodes.add("GC12345");
+        geocodes.add("GC23456");
+        final SearchResult searchResult = new SearchResult(geocodes);
+        assertEquals(2, searchResult.getCount());
+        assertEquals(2, searchResult.getTotal());
+        assertTrue(searchResult.getGeocodes().contains("GC12345"));
+    }
+}
diff --git a/tests/src/cgeo/geocaching/cgeoApplicationTest.java b/tests/src/cgeo/geocaching/cgeoApplicationTest.java
index 89c46c4..91d2320 100644
--- a/tests/src/cgeo/geocaching/cgeoApplicationTest.java
+++ b/tests/src/cgeo/geocaching/cgeoApplicationTest.java
@@ -102,7 +102,7 @@ public class cgeoApplicationTest extends ApplicationTestCase<cgeoapplication> {
     public static cgCache testSearchByGeocode(final String geocode) {
         final SearchResult search = cgBase.searchByGeocode(geocode, null, 0, true, null);
         assertNotNull(search);
-        if (Settings.isPremiumMember() || search.error == null) {
+        if (Settings.isPremiumMember() || search.getError() == null) {
             assertEquals(1, search.getGeocodes().size());
             assertTrue(search.getGeocodes().contains(geocode));
             return cgeoapplication.getInstance().loadCache(geocode, LoadFlags.LOAD_CACHE_OR_DB);
@@ -118,7 +118,7 @@ public class cgeoApplicationTest extends ApplicationTestCase<cgeoapplication> {
     public static void testSearchByGeocodeNotExisting() {
         final SearchResult search = cgBase.searchByGeocode("GC123456", null, 0, true, null);
         assertNotNull(search);
-        assertEquals(search.error, StatusCode.UNPUBLISHED_CACHE);
+        assertEquals(StatusCode.UNPUBLISHED_CACHE, search.getError());
     }
 
     /**
@@ -215,7 +215,7 @@ public class cgeoApplicationTest extends ApplicationTestCase<cgeoapplication> {
     public static void testSearchByUsername() {
         final SearchResult search = cgBase.searchByUsername(null, "blafoo", CacheType.WEBCAM, false);
         assertNotNull(search);
-        assertEquals(3, search.totalCnt);
+        assertEquals(3, search.getTotal());
         assertTrue(search.getGeocodes().contains("GCP0A9"));
     }
 
diff --git a/tests/src/cgeo/geocaching/network/ParametersTest.java b/tests/src/cgeo/geocaching/network/ParametersTest.java
new file mode 100644
index 0000000..4c56f05
--- /dev/null
+++ b/tests/src/cgeo/geocaching/network/ParametersTest.java
@@ -0,0 +1,53 @@
+package cgeo.geocaching.network;
+
+import cgeo.geocaching.network.Parameters;
+
+import android.test.AndroidTestCase;
+
+import java.security.InvalidParameterException;
+
+import junit.framework.Assert;
+
+public class ParametersTest extends AndroidTestCase {
+
+    public static void testException() {
+        try {
+            final Parameters params = new Parameters("aaa", "AAA", "bbb");
+            params.clear(); // this will never be invoked, but suppresses warnings about unused objects
+            Assert.fail("Exception not raised");
+        } catch (InvalidParameterException e) {
+            // Ok
+        }
+        try {
+            final Parameters params = new Parameters("aaa", "AAA");
+            params.put("bbb", "BBB", "ccc");
+            Assert.fail("Exception not raised");
+        } catch (InvalidParameterException e) {
+            // Ok
+        }
+    }
+
+    public static void testMultipleValues() {
+        final Parameters params = new Parameters("aaa", "AAA", "bbb", "BBB");
+        params.put("ccc", "CCC", "ddd", "DDD");
+        Assert.assertEquals("aaa=AAA&bbb=BBB&ccc=CCC&ddd=DDD", params.toString());
+    }
+
+    public static void testSort() {
+        final Parameters params = new Parameters();
+        params.put("aaa", "AAA");
+        params.put("ccc", "CCC");
+        params.put("bbb", "BBB");
+        Assert.assertEquals("aaa=AAA&ccc=CCC&bbb=BBB", params.toString());
+        params.sort();
+        Assert.assertEquals("aaa=AAA&bbb=BBB&ccc=CCC", params.toString());
+    }
+
+    public static void testToString() {
+        final Parameters params = new Parameters();
+        params.put("name", "foo&bar");
+        params.put("type", "moving");
+        Assert.assertEquals("name=foo%26bar&type=moving", params.toString());
+    }
+
+}
diff --git a/tests/src/cgeo/geocaching/test/RegExPerformanceTest.java b/tests/src/cgeo/geocaching/test/RegExPerformanceTest.java
index 0e2619d..552c225 100644
--- a/tests/src/cgeo/geocaching/test/RegExPerformanceTest.java
+++ b/tests/src/cgeo/geocaching/test/RegExPerformanceTest.java
@@ -115,7 +115,7 @@ public class RegExPerformanceTest extends TestCase {
                 diff2 = parse(page, p2, iterations);
                 output.add("Time pattern 2:\t" + diff2 + " ms");
             }
-            Float reduction = new Float((float) diff2 * 100 / diff1);
+            float reduction = (float) diff2 * 100 / diff1;
             output.add("New runtime:\t" + String.format("%.1f", reduction) + "%\n");
         }