Refactor and write tests for checkmarks

app/src/androidTest/java/org/isoron/uhabits/unit/models/CheckmarkListTest.java

@@ -0,0 +1,167 @@
+/*
+ * Copyright (C) 2016 Álinson Santos Xavier <isoron@gmail.com>
+ *
+ * This file is part of Loop Habit Tracker.
+ *
+ * Loop Habit Tracker is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by the
+ * Free Software Foundation, either version 3 of the License, or (at your
+ * option) any later version.
+ *
+ * Loop Habit Tracker is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package org.isoron.uhabits.unit.models;
+
+import android.support.test.runner.AndroidJUnit4;
+import android.test.suitebuilder.annotation.SmallTest;
+
+import org.isoron.helpers.DateHelper;
+import org.isoron.uhabits.models.Habit;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.isoron.uhabits.models.Checkmark.CHECKED_EXPLICITLY;
+import static org.isoron.uhabits.models.Checkmark.CHECKED_IMPLICITLY;
+import static org.isoron.uhabits.models.Checkmark.UNCHECKED;
+
+@RunWith(AndroidJUnit4.class)
+@SmallTest
+public class CheckmarkListTest
+{
+    Habit nonDailyHabit;
+    private Habit emptyHabit;
+
+    public static final long FIXED_LOCAL_TIME = 1422172800000L; // 8:00am, January 25th, 2015 (UTC)
+
+    @Before
+    public void prepare()
+    {
+        DateHelper.setFixedLocalTime(FIXED_LOCAL_TIME);
+        createNonDailyHabit();
+
+        emptyHabit = new Habit();
+        emptyHabit.save();
+    }
+
+    private void createNonDailyHabit()
+    {
+        nonDailyHabit = new Habit();
+        nonDailyHabit.freqNum = 2;
+        nonDailyHabit.freqDen = 3;
+        nonDailyHabit.save();
+
+        boolean check[] = { true, false, false, true, true, true, false, false, true, true };
+
+        long timestamp = DateHelper.getStartOfToday();
+        for(boolean c : check)
+        {
+            if(c) nonDailyHabit.repetitions.toggle(timestamp);
+            timestamp -= DateHelper.millisecondsInOneDay;
+        }
+    }
+
+    @After
+    public void tearDown()
+    {
+        DateHelper.setFixedLocalTime(null);
+    }
+
+    @Test
+    public void getAllValues_testNonDailyHabit()
+    {
+        int[] expectedValues = { CHECKED_EXPLICITLY, UNCHECKED, CHECKED_IMPLICITLY,
+                CHECKED_EXPLICITLY, CHECKED_EXPLICITLY, CHECKED_EXPLICITLY, UNCHECKED,
+                CHECKED_IMPLICITLY, CHECKED_EXPLICITLY, CHECKED_EXPLICITLY };
+
+        int[] actualValues = nonDailyHabit.checkmarks.getAllValues();
+
+        assertThat(actualValues, equalTo(expectedValues));
+    }
+
+    @Test
+    public void getAllValues_testMoveForwardInTime()
+    {
+        travelInTime(3);
+
+        int[] expectedValues = { UNCHECKED, UNCHECKED, UNCHECKED, CHECKED_EXPLICITLY, UNCHECKED,
+                CHECKED_IMPLICITLY, CHECKED_EXPLICITLY, CHECKED_EXPLICITLY, CHECKED_EXPLICITLY,
+                UNCHECKED, CHECKED_IMPLICITLY, CHECKED_EXPLICITLY, CHECKED_EXPLICITLY };
+
+        int[] actualValues = nonDailyHabit.checkmarks.getAllValues();
+
+        assertThat(actualValues, equalTo(expectedValues));
+    }
+
+    @Test
+    public void getAllValues_testMoveBackwardsInTime()
+    {
+        travelInTime(-3);
+
+        int[] expectedValues = { CHECKED_EXPLICITLY, CHECKED_EXPLICITLY, CHECKED_EXPLICITLY,
+                UNCHECKED, CHECKED_IMPLICITLY, CHECKED_EXPLICITLY, CHECKED_EXPLICITLY };
+
+        int[] actualValues = nonDailyHabit.checkmarks.getAllValues();
+
+        assertThat(actualValues, equalTo(expectedValues));
+    }
+
+    @Test
+    public void getAllValues_testEmptyHabit()
+    {
+        int[] expectedValues = new int[0];
+        int[] actualValues = emptyHabit.checkmarks.getAllValues();
+
+        assertThat(actualValues, equalTo(expectedValues));
+    }
+
+    @Test
+    public void getValues_testInvalidInterval()
+    {
+        int values[] = nonDailyHabit.checkmarks.getValues(100L, -100L);
+        assertThat(values, equalTo(new int[0]));
+    }
+
+    @Test
+    public void getValues_testValidInterval()
+    {
+        long from = DateHelper.getStartOfToday() - 15 * DateHelper.millisecondsInOneDay;
+        long to = DateHelper.getStartOfToday() - 5 * DateHelper.millisecondsInOneDay;
+
+        int[] expectedValues = { CHECKED_EXPLICITLY, UNCHECKED, CHECKED_IMPLICITLY,
+                CHECKED_EXPLICITLY, CHECKED_EXPLICITLY, UNCHECKED, UNCHECKED, UNCHECKED, UNCHECKED,
+                UNCHECKED, UNCHECKED };
+
+        int[] actualValues = nonDailyHabit.checkmarks.getValues(from, to);
+
+        assertThat(actualValues, equalTo(expectedValues));
+    }
+
+    @Test
+    public void getTodayValue_testNonDailyHabit()
+    {
+        travelInTime(-1);
+        assertThat(nonDailyHabit.checkmarks.getTodayValue(), equalTo(UNCHECKED));
+
+        travelInTime(0);
+        assertThat(nonDailyHabit.checkmarks.getTodayValue(), equalTo(CHECKED_EXPLICITLY));
+
+        travelInTime(1);
+        assertThat(nonDailyHabit.checkmarks.getTodayValue(), equalTo(UNCHECKED));
+    }
+
+    private void travelInTime(int days)
+    {
+        DateHelper.setFixedLocalTime(FIXED_LOCAL_TIME + days * DateHelper.millisecondsInOneDay);
+    }
+}

app/src/androidTest/java/org/isoron/uhabits/unit/models/HabitTest.java

@@ -75,21 +75,19 @@ public class HabitTest
     public  void rebuildOrderTest()
     {
         List<Long> ids = new LinkedList<>();
-
         int originalPositions[] = { 0, 1, 1, 4, 6, 8, 10, 10, 13};
-        int length = originalPositions.length;
 
-        for (int i = 0; i < length; i++)
+        for (int p : originalPositions)
         {
             Habit h = new Habit();
-            h.position = originalPositions[i];
+            h.position = p;
             h.save();
             ids.add(h.getId());
         }
 
         Habit.rebuildOrder();
 
-        for (int i = 0; i < length; i++)
+        for (int i = 0; i < originalPositions.length; i++)
         {
             Habit h = Habit.get(ids.get(i));
             assertThat(h.position, is(i));

app/src/main/java/org/isoron/uhabits/fragments/ShowHabitFragment.java

@@ -71,8 +71,6 @@ public class ShowHabitFragment extends Fragment
         activity = (ShowHabitActivity) getActivity();
         habit = activity.habit;
 
-        habit.checkmarks.rebuild();
-
         Button btEditHistory = (Button) view.findViewById(R.id.btEditHistory);
         streakView = (HabitStreakView) view.findViewById(R.id.streakView);
         scoreView = (HabitScoreView) view.findViewById(R.id.scoreView);

app/src/main/java/org/isoron/uhabits/models/Checkmark.java

@@ -26,9 +26,21 @@ import com.activeandroid.annotation.Table;
 @Table(name = "Checkmarks")
 public class Checkmark extends Model
 {
-
+    /**
+     * Indicates that there was no repetition at the timestamp, even though a repetition was
+     * expected.
+     */
     public static final int UNCHECKED = 0;
+
+    /**
+     * Indicates that there was no repetition at the timestamp, but one was not expected in any
+     * case, due to the frequency of the habit.
+     */
     public static final int CHECKED_IMPLICITLY = 1;
+
+    /**
+     * Indicates that there was a repetition at the timestamp.
+     */
     public static final int CHECKED_EXPLICITLY = 2;
 
     @Column(name = "habit")
@@ -38,10 +50,9 @@ public class Checkmark extends Model
     public Long timestamp;
 
     /**
-     * Indicates whether there is a checkmark at the given timestamp or not, and whether the
-     * checkmark is explicit or implicit. An explicit checkmark indicates that there is a
-     * repetition at that day. An implicit checkmark indicates that there is no repetition at that
-     * day, but a repetition was not needed, due to the frequency of the habit.
+     * Indicates whether there is a repetition at the given timestamp or not, and whether the
+     * repetition was expected. Assumes one of the values UNCHECKED, CHECKED_EXPLICITLY or
+     * CHECKED_IMPLICITLY.
      */
     @Column(name = "value")
     public Integer value;

app/src/main/java/org/isoron/uhabits/models/CheckmarkList.java

@@ -40,6 +40,12 @@ public class CheckmarkList
         this.habit = habit;
     }
 
+    /**
+     * Deletes every checkmark that has timestamp either equal or newer than a given timestamp.
+     * These checkmarks will be recomputed at the next time they are queried.
+     *
+     * @param timestamp the timestamp
+     */
     public void deleteNewerThan(long timestamp)
     {
         new Delete().from(Checkmark.class)
@@ -48,10 +54,21 @@ public class CheckmarkList
                 .execute();
     }
 
+    /**
+     * Returns the values of the checkmarks that fall inside a certain interval of time.
+     *
+     * The values are returned in an array containing one integer value for each day of the
+     * interval. The first entry corresponds to the most recent day in the interval. Each subsequent
+     * entry corresponds to one day older than the previous entry. The boundaries of the time
+     * interval are included.
+     *
+     * @param fromTimestamp timestamp for the oldest checkmark
+     * @param toTimestamp timestamp for the newest checkmark
+     * @return values for the checkmarks inside the given interval
+     */
     public int[] getValues(Long fromTimestamp, Long toTimestamp)
     {
-        rebuild();
-
+        buildCache(fromTimestamp, toTimestamp);
         if(fromTimestamp > toTimestamp) return new int[0];
 
         String query = "select value, timestamp from Checkmarks where " +
@@ -81,53 +98,59 @@ public class CheckmarkList
         return checks;
     }
 
+    /**
+     * Computes and returns the values for all the checkmarks, since the oldest repetition of the
+     * habit until today. If there are no repetitions at all, returns an empty array.
+     *
+     * The values are returned in an array containing one integer value for each day since the
+     * first repetition of the habit until today. The first entry corresponds to today, the second
+     * entry corresponds to yesterday, and so on.
+     *
+     * @return values for the checkmarks in the interval
+     */
     public int[] getAllValues()
     {
         Repetition oldestRep = habit.repetitions.getOldest();
         if(oldestRep == null) return new int[0];
 
-        Long toTimestamp = DateHelper.getStartOfToday();
         Long fromTimestamp = oldestRep.timestamp;
+        Long toTimestamp = DateHelper.getStartOfToday();
+
         return getValues(fromTimestamp, toTimestamp);
     }
 
-    public void rebuild()
+    /**
+     * Computes and stores one checkmark for each day that falls inside the specified interval of
+     * time. Days that already have a corresponding checkmark are skipped.
+     *
+     * @param from timestamp for the beginning of the interval
+     * @param to timestamp for the end of the interval
+     */
+    public void buildCache(long from, long to)
     {
-        long beginning;
-        long today = DateHelper.getStartOfToday();
         long day = DateHelper.millisecondsInOneDay;
 
-        Checkmark newestCheckmark = getNewest();
-        if (newestCheckmark == null)
-        {
-            Repetition oldestRep = habit.repetitions.getOldest();
-            if (oldestRep == null) return;
-
-            beginning = oldestRep.timestamp;
-        }
-        else
-        {
-            beginning = newestCheckmark.timestamp + day;
-        }
-
-        if (beginning > today) return;
+        Checkmark newestCheckmark = findNewest();
+        if(newestCheckmark != null)
+            from = Math.max(from, newestCheckmark.timestamp + day);
 
-        long beginningExtended = beginning - (long) (habit.freqDen) * day;
-        List<Repetition> reps = habit.repetitions.selectFromTo(beginningExtended, today).execute();
+        if(from > to) return;
 
-        int nDays = (int) ((today - beginning) / day) + 1;
-        int nDaysExtended = (int) ((today - beginningExtended) / day) + 1;
+        long fromExtended = from - (long) (habit.freqDen) * day;
+        List<Repetition> reps = habit.repetitions
+                .selectFromTo(fromExtended, to)
+                .execute();
 
+        int nDays = (int) ((to - from) / day) + 1;
+        int nDaysExtended = (int) ((to - fromExtended) / day) + 1;
         int checks[] = new int[nDaysExtended];
 
-        // explicit checks
         for (Repetition rep : reps)
         {
-            int offset = (int) ((rep.timestamp - beginningExtended) / day);
-            checks[nDaysExtended - offset - 1] = 2;
+            int offset = (int) ((rep.timestamp - fromExtended) / day);
+            checks[nDaysExtended - offset - 1] = Checkmark.CHECKED_EXPLICITLY;
         }
 
-        // implicit checks
         for (int i = 0; i < nDays; i++)
         {
             int counter = 0;
@@ -135,7 +158,9 @@ public class CheckmarkList
             for (int j = 0; j < habit.freqDen; j++)
                 if (checks[i + j] == 2) counter++;
 
-            if (counter >= habit.freqNum) checks[i] = Math.max(checks[i], 1);
+            if (counter >= habit.freqNum)
+                if(checks[i] != Checkmark.CHECKED_EXPLICITLY)
+                    checks[i] = Checkmark.CHECKED_IMPLICITLY;
         }
 
         ActiveAndroid.beginTransaction();
@@ -146,33 +171,48 @@ public class CheckmarkList
             {
                 Checkmark c = new Checkmark();
                 c.habit = habit;
-                c.timestamp = today - i * day;
+                c.timestamp = to - i * day;
                 c.value = checks[i];
                 c.save();
             }
 
             ActiveAndroid.setTransactionSuccessful();
-        } finally
+        }
+        finally
         {
             ActiveAndroid.endTransaction();
         }
     }
 
-    public Checkmark getNewest()
+    /**
+     * Returns newest checkmark that has already been computed. Ignores any checkmark that has
+     * timestamp in the future. This does not update the cache.
+     */
+    private Checkmark findNewest()
     {
         return new Select().from(Checkmark.class)
                 .where("habit = ?", habit.getId())
+                .and("timestamp <= ?", DateHelper.getStartOfToday())
                 .orderBy("timestamp desc")
                 .limit(1)
                 .executeSingle();
     }
 
-    public int getCurrentValue()
+    /**
+     * Returns the checkmark for today.
+     */
+    public Checkmark getToday()
     {
-        rebuild();
-        Checkmark c = getNewest();
+        long today = DateHelper.getStartOfToday();
+        buildCache(today, today);
+        return findNewest();
+    }
 
-        if(c != null) return c.value;
-        else return 0;
+    /**
+     * Returns the value of today's checkmark.
+     */
+    public int getTodayValue()
+    {
+        return getToday().value;
     }
 }

app/src/main/java/org/isoron/uhabits/models/RepetitionList.java

@@ -30,7 +30,6 @@ import com.activeandroid.query.Select;
 import org.isoron.helpers.DateHelper;
 
 import java.util.Arrays;
-import java.util.Calendar;
 import java.util.GregorianCalendar;
 import java.util.HashMap;
 

app/src/main/java/org/isoron/uhabits/views/CheckmarkView.java

@@ -114,7 +114,7 @@ public class CheckmarkView extends View
 
     public void setHabit(Habit habit)
     {
-        this.check_status = habit.checkmarks.getCurrentValue();
+        this.check_status = habit.checkmarks.getTodayValue();
         this.star_status = habit.scores.getCurrentStarStatus();
         this.primaryColor = Color.argb(230, Color.red(habit.color), Color.green(habit.color), Color.blue(habit.color));
         this.label = habit.name;