Simplify StreakList

android/uhabits-core/src/main/java/org/isoron/uhabits/core/models/EntryList.kt

@@ -25,10 +25,12 @@ import org.isoron.uhabits.core.models.Entry.Companion.YES_AUTO
 import org.isoron.uhabits.core.models.Entry.Companion.YES_MANUAL
 import org.isoron.uhabits.core.utils.*
 import java.util.*
+import javax.annotation.concurrent.*
 import kotlin.collections.HashMap
 import kotlin.collections.set
 import kotlin.math.*
 
+@ThreadSafe
 open class EntryList {
 
     private val entriesByTimestamp: HashMap<Timestamp, Entry> = HashMap()
@@ -37,6 +39,7 @@ open class EntryList {
      * Returns the entry corresponding to the given timestamp. If no entry with such timestamp
      * has been previously added, returns Entry(timestamp, UNKNOWN).
      */
+    @Synchronized
     open fun get(timestamp: Timestamp): Entry {
         return entriesByTimestamp[timestamp] ?: Entry(timestamp, UNKNOWN)
     }
@@ -46,6 +49,7 @@ open class EntryList {
      * newest entry, and the last element corresponds to the oldest. The interval endpoints are
      * included.
      */
+    @Synchronized
     open fun getByInterval(from: Timestamp, to: Timestamp): List<Entry> {
         val result = mutableListOf<Entry>()
         if (from.isNewerThan(to)) return result
@@ -61,6 +65,7 @@ open class EntryList {
      * Adds the given entry to the list. If another entry with the same timestamp already exists,
      * replaces it.
      */
+    @Synchronized
     open fun add(entry: Entry) {
         entriesByTimestamp[entry.timestamp] = entry
     }
@@ -69,6 +74,7 @@ open class EntryList {
      * Returns all entries whose values are known, sorted by timestamp. The first element
      * corresponds to the newest entry, and the last element corresponds to the oldest.
      */
+    @Synchronized
     open fun getKnown(): List<Entry> {
         return entriesByTimestamp.values.sortedBy { it.timestamp }.reversed()
     }
@@ -82,6 +88,7 @@ open class EntryList {
      * entries. For numerical habits, the value is the total sum. The field [firstWeekday] is only
      * relevant when grouping by week.
      */
+    @Synchronized
     open fun groupBy(
             original: List<Entry>,
             field: DateUtils.TruncateField,
@@ -117,6 +124,7 @@ open class EntryList {
      * For boolean habits, this function creates additional entries (with value YES_AUTO) according
      * to the frequency of the habit. For numerical habits, this function simply copies all entries.
      */
+    @Synchronized
     open fun recomputeFrom(
             originalEntries: EntryList,
             frequency: Frequency,
@@ -137,6 +145,7 @@ open class EntryList {
     /**
      * Removes all known entries.
      */
+    @Synchronized
     open fun clear() {
         entriesByTimestamp.clear()
     }
@@ -153,6 +162,7 @@ open class EntryList {
      *
      * @return total number of checkmarks by month versus day of week
      */
+    @Synchronized
     fun computeWeekdayFrequency(isNumerical: Boolean): HashMap<Timestamp, Array<Int>> {
         val entries = getKnown()
         val map = hashMapOf<Timestamp, Array<Int>>()
@@ -185,6 +195,7 @@ open class EntryList {
      * are included.
      */
     @Deprecated("")
+    @Synchronized
     fun getValues(from: Timestamp, to: Timestamp): IntArray {
         if (from.isNewerThan(to)) throw IllegalArgumentException()
         val nDays = from.daysUntil(to) + 1
@@ -199,6 +210,7 @@ open class EntryList {
     }
 
     @Deprecated("")
+    @Synchronized
     fun getAllValues(): IntArray {
         val entries = getKnown()
         if (entries.isEmpty()) return IntArray(0)
@@ -209,6 +221,7 @@ open class EntryList {
     }
 
     @Deprecated("")
+    @Synchronized
     open fun getThisWeekValue(firstWeekday: Int, isNumerical: Boolean): Int {
         return getThisIntervalValue(
                 truncateField = DateUtils.TruncateField.WEEK_NUMBER,
@@ -218,6 +231,7 @@ open class EntryList {
     }
 
     @Deprecated("")
+    @Synchronized
     open fun getThisMonthValue(isNumerical: Boolean): Int {
         return getThisIntervalValue(
                 truncateField = DateUtils.TruncateField.MONTH,
@@ -227,6 +241,7 @@ open class EntryList {
     }
 
     @Deprecated("")
+    @Synchronized
     open fun getThisQuarterValue(isNumerical: Boolean): Int {
         return getThisIntervalValue(
                 truncateField = DateUtils.TruncateField.QUARTER,
@@ -236,6 +251,7 @@ open class EntryList {
     }
 
     @Deprecated("")
+    @Synchronized
     open fun getThisYearValue(isNumerical: Boolean): Int {
         return getThisIntervalValue(
                 truncateField = DateUtils.TruncateField.YEAR,

android/uhabits-core/src/main/java/org/isoron/uhabits/core/models/Habit.kt

@@ -70,7 +70,6 @@ data class Habit(
     }
 
     fun recompute() {
-        streaks.recompute()
         computedEntries.recomputeFrom(
                 originalEntries = originalEntries,
                 frequency = frequency,
@@ -90,6 +89,12 @@ data class Habit(
                 from = from,
                 to = to,
         )
+
+        streaks.recompute(
+                computedEntries,
+                from,
+                to,
+        )
     }
 
     fun copyFrom(other: Habit) {

android/uhabits-core/src/main/java/org/isoron/uhabits/core/models/HabitList.java

@@ -176,10 +176,6 @@ public abstract class HabitList implements Iterable<Habit>
 
     public void repair()
     {
-        for (Habit h : this)
-        {
-            h.getStreaks().recompute();
-        }
     }
 
     /**

android/uhabits-core/src/main/java/org/isoron/uhabits/core/models/ModelFactory.kt

@@ -36,7 +36,6 @@ interface ModelFactory {
                 originalEntries = buildOriginalEntries(),
                 computedEntries = buildComputedEntries(),
         )
-        streaks.setHabit(habit)
         return habit
     }
     fun buildComputedEntries(): EntryList

android/uhabits-core/src/main/java/org/isoron/uhabits/core/models/ScoreList.kt

@@ -20,8 +20,10 @@ package org.isoron.uhabits.core.models
 
 import org.isoron.uhabits.core.models.Score.Companion.compute
 import java.util.*
+import javax.annotation.concurrent.*
 import kotlin.math.*
 
+@ThreadSafe
 class ScoreList {
 
     private val map = HashMap<Timestamp, Score>()
@@ -30,6 +32,7 @@ class ScoreList {
      * Returns the score for a given day. If the timestamp given happens before the first
      * repetition of the habit or after the last computed score, returns a score with value zero.
      */
+    @Synchronized
     operator fun get(timestamp: Timestamp): Score {
         return map[timestamp] ?: Score(timestamp, 0.0)
     }
@@ -41,6 +44,7 @@ class ScoreList {
      * included. The list is ordered by timestamp (decreasing). That is, the first score
      * corresponds to the newest timestamp, and the last score corresponds to the oldest timestamp.
      */
+    @Synchronized
     fun getByInterval(
             fromTimestamp: Timestamp,
             toTimestamp: Timestamp,
@@ -58,6 +62,7 @@ class ScoreList {
     /**
      * Recomputes all scores between the provided [from] and [to] timestamps.
      */
+    @Synchronized
     fun recompute(
             frequency: Frequency,
             isNumerical: Boolean,

android/uhabits-core/src/main/java/org/isoron/uhabits/core/models/StreakList.java

@@ -1,183 +0,0 @@
-/*
- * 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.core.models;
-
-import androidx.annotation.*;
-
-import org.isoron.uhabits.core.utils.*;
-
-import java.util.*;
-
-/**
- * The collection of {@link Streak}s that belong to a habit.
- * <p>
- * This list is populated automatically from the list of repetitions.
- */
-public class StreakList
-{
-    protected Habit habit;
-
-    protected ModelObservable observable = new ModelObservable();
-
-    ArrayList<Streak> list = new ArrayList<>();
-
-    public void setHabit(Habit habit)
-    {
-        this.habit = habit;
-    }
-
-    public List<Streak> getAll()
-    {
-        rebuild();
-        return new LinkedList<>(list);
-    }
-
-    @NonNull
-    public List<Streak> getBest(int limit)
-    {
-        List<Streak> streaks = getAll();
-        Collections.sort(streaks, (s1, s2) -> s2.compareLonger(s1));
-        streaks = streaks.subList(0, Math.min(streaks.size(), limit));
-        Collections.sort(streaks, (s1, s2) -> s2.compareNewer(s1));
-        return streaks;
-    }
-
-    @Nullable
-    public Streak getNewestComputed()
-    {
-        Streak newest = null;
-
-        for (Streak s : list)
-            if (newest == null || s.getEnd().isNewerThan(newest.getEnd()))
-                newest = s;
-
-        return newest;
-
-    }
-
-    @NonNull
-    public ModelObservable getObservable()
-    {
-        return observable;
-    }
-
-    public void recompute()
-    {
-        list.clear();
-        observable.notifyListeners();
-    }
-
-    public synchronized void rebuild()
-    {
-        Timestamp today = DateUtils.getTodayWithOffset();
-        Timestamp beginning = findBeginning();
-        if (beginning == null || beginning.isNewerThan(today)) return;
-
-        int checks[] = habit.getComputedEntries().getValues(beginning, today);
-        List<Streak> streaks = checkmarksToStreaks(beginning, checks);
-
-        removeNewestComputed();
-        add(streaks);
-    }
-
-    /**
-     * Converts a list of checkmark values to a list of streaks.
-     *
-     * @param beginning the timestamp corresponding to the first checkmark
-     *                  value.
-     * @param checks    the checkmarks values, ordered by decreasing timestamp.
-     * @return the list of streaks.
-     */
-    @NonNull
-    protected List<Streak> checkmarksToStreaks(Timestamp beginning, int[] checks)
-    {
-        ArrayList<Timestamp> transitions = getTransitions(beginning, checks);
-
-        List<Streak> streaks = new LinkedList<>();
-        for (int i = 0; i < transitions.size(); i += 2)
-        {
-            Timestamp start = transitions.get(i);
-            Timestamp end = transitions.get(i + 1);
-            streaks.add(new Streak(start, end));
-        }
-
-        return streaks;
-    }
-
-    /**
-     * Finds the place where we should start when recomputing the streaks.
-     *
-     * @return
-     */
-    @Nullable
-    protected Timestamp findBeginning()
-    {
-        Streak newestStreak = getNewestComputed();
-        if (newestStreak != null) return newestStreak.getStart();
-
-        List<Entry> entries = habit.getOriginalEntries().getKnown();
-        if(entries.isEmpty()) return null;
-        return entries.get(entries.size() - 1).getTimestamp();
-    }
-
-    /**
-     * Returns the timestamps where there was a transition from performing a
-     * habit to not performing a habit, and vice-versa.
-     *
-     * @param beginning the timestamp for the first checkmark
-     * @param checks    the checkmarks, ordered by decreasing timestamp
-     * @return the list of transitions
-     */
-    @NonNull
-    protected ArrayList<Timestamp> getTransitions(Timestamp beginning, int[] checks)
-    {
-        ArrayList<Timestamp> list = new ArrayList<>();
-        Timestamp current = beginning;
-        list.add(current);
-
-        for (int i = 1; i < checks.length; i++)
-        {
-            current = current.plus(1);
-            int j = checks.length - i - 1;
-
-            if ((checks[j + 1] <= 0 && checks[j] > 0)) list.add(current);
-            if ((checks[j + 1] > 0 && checks[j] <= 0)) list.add(current.minus(1));
-        }
-
-        if (list.size() % 2 == 1) list.add(current);
-
-        return list;
-    }
-
-    protected void add(@NonNull List<Streak> streaks)
-    {
-        list.addAll(streaks);
-        Collections.sort(list, (s1, s2) -> s2.compareNewer(s1));
-        observable.notifyListeners();
-
-    }
-
-    protected void removeNewestComputed()
-    {
-        Streak newest = getNewestComputed();
-        if (newest != null) list.remove(newest);
-
-    }
-}

android/uhabits-core/src/main/java/org/isoron/uhabits/core/models/StreakList.kt

@@ -0,0 +1,65 @@
+/*
+ * 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.core.models
+
+import javax.annotation.concurrent.*
+import kotlin.math.*
+
+@ThreadSafe
+class StreakList {
+    private val list = ArrayList<Streak>()
+
+    @Synchronized
+    fun getBest(limit: Int): List<Streak> {
+        list.sortWith { s1: Streak, s2: Streak -> s2.compareLonger(s1) }
+        return list.subList(0, min(list.size, limit)).apply {
+            sortWith { s1: Streak, s2: Streak -> s2.compareNewer(s1) }
+        }
+    }
+
+    @Synchronized
+    fun recompute(
+            computedEntries: EntryList,
+            from: Timestamp,
+            to: Timestamp,
+    ) {
+        list.clear()
+        val timestamps = computedEntries
+                .getByInterval(from, to)
+                .filter { it.value > 0 }
+                .map { it.timestamp }
+                .toTypedArray()
+
+        if (timestamps.isEmpty()) return
+
+        var begin = timestamps[0]
+        var end = timestamps[0]
+        for (i in 1 until timestamps.size) {
+            val current = timestamps[i]
+            if (current == begin.minus(1)) {
+                begin = current
+            } else {
+                list.add(Streak(begin, end))
+                begin = current
+                end = current
+            }
+        }
+        list.add(Streak(begin, end))
+    }
+}

android/uhabits-core/src/test/java/org/isoron/uhabits/core/models/StreakListTest.java

@@ -36,12 +36,8 @@ public class StreakListTest extends BaseUnitTest
 
     private StreakList streaks;
 
-    private long day;
-
     private Timestamp today;
 
-    private ModelObservable.Listener listener;
-
     @Override
     public void setUp() throws Exception
     {
@@ -51,49 +47,14 @@ public class StreakListTest extends BaseUnitTest
         habit.recompute();
 
         streaks = habit.getStreaks();
-        streaks.rebuild();
-
-        listener = mock(ModelObservable.Listener.class);
-        streaks.getObservable().addListener(listener);
         today = DateUtils.getToday();
     }
 
-    @Test
-    public void testFindBeginning_withEmptyHistory()
-    {
-        Habit habit2 = fixtures.createEmptyHabit();
-        Timestamp beginning = habit2.getStreaks().findBeginning();
-        assertNull(beginning);
-    }
-
-    @Test
-    public void testFindBeginning_withLongHistory()
-    {
-        streaks.rebuild();
-        streaks.recompute();
-        assertThat(streaks.findBeginning(), equalTo(today.minus(120)));
-    }
-
-    @Test
-    public void testGetAll() throws Exception
-    {
-        List<Streak> all = streaks.getAll();
-
-        assertThat(all.size(), equalTo(22));
-
-        assertThat(all.get(3).getEnd(), equalTo(today.minus(7)));
-        assertThat(all.get(3).getStart(), equalTo(today.minus(10)));
-
-        assertThat(all.get(17).getEnd(), equalTo(today.minus(89)));
-        assertThat(all.get(17).getStart(), equalTo(today.minus(91)));
-    }
-
     @Test
     public void testGetBest() throws Exception
     {
         List<Streak> best = streaks.getBest(4);
         assertThat(best.size(), equalTo(4));
-
         assertThat(best.get(0).getLength(), equalTo(4));
         assertThat(best.get(1).getLength(), equalTo(3));
         assertThat(best.get(2).getLength(), equalTo(5));
@@ -101,30 +62,20 @@ public class StreakListTest extends BaseUnitTest
 
         best = streaks.getBest(2);
         assertThat(best.size(), equalTo(2));
-
         assertThat(best.get(0).getLength(), equalTo(5));
         assertThat(best.get(1).getLength(), equalTo(6));
     }
 
     @Test
-    public void testInvalidateNewer()
+    public void testGetBest_withUnknowns()
     {
-        Streak s = streaks.getNewestComputed();
+        habit.getOriginalEntries().clear();
-        assertThat(s.getEnd(), equalTo(today));
+        habit.getOriginalEntries().add(new Entry(today, Entry.YES_MANUAL));
-
+        habit.getOriginalEntries().add(new Entry(today.minus(5), Entry.NO));
-        streaks.recompute();
+        habit.recompute();
-        verify(listener).onModelChange();
-
-        s = streaks.getNewestComputed();
-        assertNull(s);
-    }
 
-    @Test
+        List<Streak> best = streaks.getBest(5);
-    public void testToString() throws Exception
+        assertThat(best.size(), equalTo(1));
-    {
+        assertThat(best.get(0).getLength(), equalTo(1));
-        Timestamp time = Timestamp.ZERO.plus(100);
-        Streak streak = new Streak(time, time.plus(10));
-        assertThat(streak.toString(), equalTo(
-            "{start: 1970-04-11, end: 1970-04-21}"));
     }
 }