Refactor and cleanup based on Tim's suggestions

astrid/src/com/todoroo/astrid/dao/ABTestEventDao.java

@@ -36,7 +36,7 @@ public class ABTestEventDao extends DatabaseDao<ABTestEvent> {
         event.setValue(ABTestEvent.TEST_VARIANT, testVariant);
         event.setValue(ABTestEvent.NEW_USER, newUser ? 1 : 0);
         event.setValue(ABTestEvent.ACTIVATED_USER, activeUser ? 1 : 0);
-        event.setValue(ABTestEvent.TIME_INTERVAL, ABTestEvent.TIME_INTERVAL_0);
+        event.setValue(ABTestEvent.TIME_INTERVAL, 0);
         event.setValue(ABTestEvent.DATE_RECORDED, DateUtilities.now());
 
         createNew(event);
@@ -90,25 +90,22 @@ public class ABTestEventDao extends DatabaseDao<ABTestEvent> {
      */
     public void createRelativeDateEvents() {
         TodorooCursor<ABTestEvent> allEvents = query(Query.select(ABTestEvent.TEST_NAME, ABTestEvent.DATE_RECORDED)
-                .where(ABTestEvent.TIME_INTERVAL.eq(ABTestEvent.TIME_INTERVAL_0)));
+                .where(ABTestEvent.TIME_INTERVAL.eq(0)));
 
         try {
             long now = DateUtilities.now();
+            ABTestEvent event = new ABTestEvent();
             for (allEvents.moveToFirst(); !allEvents.isAfterLast(); allEvents.moveToNext()) {
-                ABTestEvent event = new ABTestEvent(allEvents);
+                event.readFromCursor(allEvents);
                 long baseTime = event.getValue(ABTestEvent.DATE_RECORDED);
                 long timeSinceBase = now - baseTime;
 
                 String testName = event.getValue(ABTestEvent.TEST_NAME);
                 int timeInterval = -1;
-                if (timeSinceBase > 3 * DateUtilities.ONE_WEEK)
+                long days = timeSinceBase / DateUtilities.ONE_DAY;
-                    timeInterval = ABTestEvent.TIME_INTERVAL_21;
+                for(int i = 0; i < ABTestEvent.TIME_INTERVALS.length; i++)
-                else if (timeSinceBase > 2 * DateUtilities.ONE_WEEK)
+                    if(days >= ABTestEvent.TIME_INTERVALS[i])
-                    timeInterval = ABTestEvent.TIME_INTERVAL_14;
+                        timeInterval = ABTestEvent.TIME_INTERVALS[i];
-                else if (timeSinceBase > DateUtilities.ONE_WEEK)
-                    timeInterval = ABTestEvent.TIME_INTERVAL_7;
-                else if (timeSinceBase > 3 * DateUtilities.ONE_DAY)
-                    timeInterval = ABTestEvent.TIME_INTERVAL_3;
 
                 if (timeInterval > 0)
                     createTestEventWithTimeInterval(testName, timeInterval);

astrid/src/com/todoroo/astrid/data/ABTestEvent.java

@@ -20,19 +20,7 @@ import com.todoroo.astrid.api.AstridApiConstants;
 @SuppressWarnings("nls")
 public class ABTestEvent extends AbstractModel {
 
-    public static final int TIME_INTERVAL_0 = 0;
+    public static final int[] TIME_INTERVALS = { 0, 3, 7, 14, 21 };
-    public static final int TIME_INTERVAL_3 = 3;
-    public static final int TIME_INTERVAL_7 = 7;
-    public static final int TIME_INTERVAL_14 = 14;
-    public static final int TIME_INTERVAL_21 = 21;
-
-    public static final int[] TIME_INTERVALS = {
-        TIME_INTERVAL_0,
-        TIME_INTERVAL_3,
-        TIME_INTERVAL_7,
-        TIME_INTERVAL_14,
-        TIME_INTERVAL_21
-    };
 
     // --- table and uri
 
@@ -80,7 +68,8 @@ public class ABTestEvent extends AbstractModel {
 
     /**
      * Which time interval event this data point corresponds to.
-     * Should be one of the time interval constants defined above.
+     * Should be one of the time interval constants defined int the
+     * above array.
      */
     public static final IntegerProperty TIME_INTERVAL = new IntegerProperty(
             TABLE, "timeInterval"); // one of the constants defined above
@@ -121,6 +110,10 @@ public class ABTestEvent extends AbstractModel {
         readPropertiesFromCursor(cursor);
     }
 
+    public void readFromCursor(TodorooCursor<ABTestEvent> cursor) {
+        super.readPropertiesFromCursor(cursor);
+    }
+
     @Override
     public long getId() {
         return getIdHelper(ID);

astrid/src/com/todoroo/astrid/service/abtesting/ABTestEventReportingService.java

@@ -74,8 +74,9 @@ public final class ABTestEventReportingService {
                     try {
                         JSONArray payload = jsonArrayFromABTestEvents(unreported);
                         abTestInvoker.post(payload);
+                        ABTestEvent model = new ABTestEvent();
                         for (unreported.moveToFirst(); !unreported.isAfterLast(); unreported.moveToNext()) {
-                            ABTestEvent model = new ABTestEvent(unreported);
+                            model.readFromCursor(unreported);
                             model.setValue(ABTestEvent.REPORTED, 1);
                             abTestEventDao.saveExisting(model);
                         }
@@ -114,9 +115,9 @@ public final class ABTestEventReportingService {
 
         String lastTestKey = "";
         JSONObject testAcc = null;
+        ABTestEvent model = new ABTestEvent();
         for (events.moveToFirst(); !events.isAfterLast(); events.moveToNext()) {
-            ABTestEvent model = new ABTestEvent(events);
+            model.readFromCursor(events);
-
             if (!model.getValue(ABTestEvent.TEST_NAME).equals(lastTestKey)) {
                 if (testAcc != null)
                     result.put(testAcc);

astrid/src/com/todoroo/astrid/service/abtesting/ABTestInvoker.java

@@ -48,8 +48,6 @@ public class ABTestInvoker {
     public JSONObject post(JSONArray payload) throws IOException {
         try {
             HttpEntity postData  = createPostData(payload);
-            if (postData == null)
-                throw new IOException("Unsupported URL encoding");
             String response = restClient.post(URL, postData);
             JSONObject object = new JSONObject(response);
             if (object.getString("status").equals("error")) {
@@ -69,7 +67,7 @@ public class ABTestInvoker {
      * @param payload
      * @return
      */
-    private HttpEntity createPostData(JSONArray payload) {
+    private HttpEntity createPostData(JSONArray payload) throws IOException {
         List<NameValuePair> params = new ArrayList<NameValuePair>();
         params.add(new BasicNameValuePair("apikey", API_KEY));
         params.add(new BasicNameValuePair("payload", payload.toString()));
@@ -92,7 +90,7 @@ public class ABTestInvoker {
         try {
             return new UrlEncodedFormEntity(params, HTTP.UTF_8);
         } catch (UnsupportedEncodingException e) {
-            return null;
+            throw new IOException("Unsupported URL encoding");
         }
     }
 

astrid/src/com/todoroo/astrid/service/abtesting/ABTests.java

@@ -116,21 +116,13 @@ public class ABTests {
 
     private void initialize() { // Set up
         //Calls to addTest go here
-        addTest(AB_TEST_SWIPE_ENABLED_KEY, AB_TEST_SWIPE_ENABLED_PROBS_NEW_USER,
+        addTest(AB_TEST_SWIPE_ENABLED_KEY, new int[] { 1, 1 },
-                AB_TEST_SWIPE_ENABLED_PROBS_EXISTING_USER, AB_TEST_SWIPE_ENABLED_DESC);
+                new int[] { 1, 1 }, new String[] { "swipe-lists-disabled", "swipe-lists-enabled" }); //$NON-NLS-1$//$NON-NLS-2$
-        addTest(AB_TEST_CONTACTS_PICKER_ENABLED, AB_TEST_CONTACTS_ENABLED_PROBS_NEW_USER,
+        addTest(AB_TEST_CONTACTS_PICKER_ENABLED, new int[] { 1, 1 },
-                AB_TEST_CONTACTS_ENABLED_PROBS_EXISTING_USER, AB_TEST_CONTACTS_ENABLED_DESC);
+                new int[] { 1, 1 }, new String[] { "contacts-disabled", "contacts-enabled" }); //$NON-NLS-1$//$NON-NLS-2$
     }
 
     public static final String AB_TEST_SWIPE_ENABLED_KEY = "swipeEnabled"; //$NON-NLS-1$
-    private static final int[] AB_TEST_SWIPE_ENABLED_PROBS_NEW_USER = { 1, 1 };
-    private static final int[] AB_TEST_SWIPE_ENABLED_PROBS_EXISTING_USER = { 1, 1 };
-    private static final String[] AB_TEST_SWIPE_ENABLED_DESC = { "swipe-lists-disabled", "swipe-lists-enabled" };  //$NON-NLS-1$//$NON-NLS-2$
-
     public static final String AB_TEST_CONTACTS_PICKER_ENABLED = "contactsEnabled"; //$NON-NLS-1$
-    private static final int[] AB_TEST_CONTACTS_ENABLED_PROBS_NEW_USER = { 1, 1 };
-    private static final int[] AB_TEST_CONTACTS_ENABLED_PROBS_EXISTING_USER = { 1, 1 };
-    private static final String[] AB_TEST_CONTACTS_ENABLED_DESC = { "contacts-disabled", "contacts-enabled" };  //$NON-NLS-1$//$NON-NLS-2$
-
 
 }

tests/src/com/todoroo/astrid/service/ABTestingServiceTest.java

@@ -27,7 +27,7 @@ public class ABTestingServiceTest extends DatabaseTestCase {
     }
 
     private void testInitialEvents(boolean newUser, boolean activatedUser) {
-        testInterval(newUser, activatedUser, ABTestEvent.TIME_INTERVAL_0);
+        testInterval(newUser, activatedUser, 0);
     }
 
     public void testIntervalEventWithShortIntervalNewUser() {
@@ -47,11 +47,11 @@ public class ABTestingServiceTest extends DatabaseTestCase {
     }
 
     private void testIntervalEventWithShortInterval(boolean newUser, boolean activatedUser) {
-        testInterval(newUser, activatedUser, ABTestEvent.TIME_INTERVAL_3);
+        testInterval(newUser, activatedUser, 3);
     }
 
     private void testIntervalEventWithLongInterval(boolean newUser, boolean activatedUser) {
-        testInterval(newUser, activatedUser, ABTestEvent.TIME_INTERVAL_14);
+        testInterval(newUser, activatedUser, 14);
     }
 
     private void testInterval(boolean newUser, boolean activatedUser, int testInterval) {