EntryList: simplify groupBy

android/uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/show/views/BarCard.kt

@@ -28,6 +28,7 @@ import org.isoron.uhabits.activities.habits.show.views.ScoreCardPresenter.Compan
 import org.isoron.uhabits.core.models.Entry
 import org.isoron.uhabits.core.models.Habit
 import org.isoron.uhabits.core.models.PaletteColor
+import org.isoron.uhabits.core.models.groupedSum
 import org.isoron.uhabits.core.utils.DateUtils
 import org.isoron.uhabits.databinding.ShowHabitBarBinding
 import org.isoron.uhabits.utils.toThemedAndroidColor
@@ -61,20 +62,33 @@ class BarCard(context: Context, attrs: AttributeSet) : LinearLayout(context, att
 
         binding.numericalSpinner.onItemSelectedListener = null
         binding.numericalSpinner.setSelection(data.numericalSpinnerPosition)
-        binding.numericalSpinner.onItemSelectedListener = object : AdapterView.OnItemSelectedListener {
+        binding.numericalSpinner.onItemSelectedListener =
-            override fun onItemSelected(parent: AdapterView<*>?, view: View?, position: Int, id: Long) {
+            object : AdapterView.OnItemSelectedListener {
-                onNumericalSpinnerPosition(position)
+                override fun onItemSelected(
-            }
+                    parent: AdapterView<*>?,
-            override fun onNothingSelected(parent: AdapterView<*>?) {
+                    view: View?,
+                    position: Int,
+                    id: Long
+                ) {
+                    onNumericalSpinnerPosition(position)
+                }
+
+                override fun onNothingSelected(parent: AdapterView<*>?) {
+                }
             }
-        }
 
         binding.boolSpinner.onItemSelectedListener = null
         binding.boolSpinner.setSelection(data.boolSpinnerPosition)
         binding.boolSpinner.onItemSelectedListener = object : AdapterView.OnItemSelectedListener {
-            override fun onItemSelected(parent: AdapterView<*>?, view: View?, position: Int, id: Long) {
+            override fun onItemSelected(
+                parent: AdapterView<*>?,
+                view: View?,
+                position: Int,
+                id: Long
+            ) {
                 onBoolSpinnerPosition(position)
             }
+
             override fun onNothingSelected(parent: AdapterView<*>?) {
             }
         }
@@ -99,18 +113,11 @@ class BarCardPresenter(
         }
         val today = DateUtils.getToday()
         val oldest = habit.computedEntries.getKnown().lastOrNull()?.timestamp ?: today
-        val entries = if (bucketSize == 1) {
+        val entries = habit.computedEntries.getByInterval(oldest, today).groupedSum(
-            habit.computedEntries.getByInterval(oldest, today).map {
+            truncateField = getTruncateField(bucketSize),
-                if (it.value < 0) Entry(it.timestamp, 0) else it
+            firstWeekday = firstWeekday,
-            }
+            isNumerical = habit.isNumerical,
-        } else {
+        )
-            habit.computedEntries.groupBy(
-                original = habit.computedEntries.getByInterval(oldest, today),
-                field = getTruncateField(bucketSize),
-                firstWeekday = firstWeekday,
-                isNumerical = habit.isNumerical,
-            )
-        }
         return BarCardViewModel(
             entries = entries,
             bucketSize = bucketSize,

android/uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/show/views/TargetCard.kt

@@ -28,7 +28,13 @@ import kotlinx.coroutines.invoke
 import org.isoron.uhabits.R
 import org.isoron.uhabits.core.models.Habit
 import org.isoron.uhabits.core.models.PaletteColor
+import org.isoron.uhabits.core.models.groupedSum
 import org.isoron.uhabits.core.utils.DateUtils
+import org.isoron.uhabits.core.utils.DateUtils.TruncateField.DAY
+import org.isoron.uhabits.core.utils.DateUtils.TruncateField.MONTH
+import org.isoron.uhabits.core.utils.DateUtils.TruncateField.QUARTER
+import org.isoron.uhabits.core.utils.DateUtils.TruncateField.WEEK_NUMBER
+import org.isoron.uhabits.core.utils.DateUtils.TruncateField.YEAR
 import org.isoron.uhabits.databinding.ShowHabitTargetBinding
 import org.isoron.uhabits.utils.toThemedAndroidColor
 import java.util.ArrayList
@@ -61,12 +67,34 @@ class TargetCardPresenter(
 ) {
     suspend fun present(): TargetCardViewModel = Dispatchers.IO {
         val today = DateUtils.getTodayWithOffset()
-        val entries = habit.computedEntries
+        val oldest = habit.computedEntries.getKnown().lastOrNull()?.timestamp ?: today
-        val valueToday = entries.get(today).value / 1e3
+        val entries = habit.computedEntries.getByInterval(oldest, today)
-        val valueThisWeek = entries.getThisWeekValue(firstWeekday, habit.isNumerical) / 1e3
+
-        val valueThisMonth = entries.getThisMonthValue(habit.isNumerical) / 1e3
+        val valueToday = entries.groupedSum(
-        val valueThisQuarter = entries.getThisQuarterValue(habit.isNumerical) / 1e3
+            truncateField = DAY,
-        val valueThisYear = entries.getThisYearValue(habit.isNumerical) / 1e3
+            isNumerical = habit.isNumerical
+        ).firstOrNull()?.value ?: 0
+
+        val valueThisWeek = entries.groupedSum(
+            truncateField = WEEK_NUMBER,
+            firstWeekday = firstWeekday,
+            isNumerical = habit.isNumerical
+        ).firstOrNull()?.value ?: 0
+
+        val valueThisMonth = entries.groupedSum(
+            truncateField = MONTH,
+            isNumerical = habit.isNumerical
+        ).firstOrNull()?.value ?: 0
+
+        val valueThisQuarter = entries.groupedSum(
+            truncateField = QUARTER,
+            isNumerical = habit.isNumerical
+        ).firstOrNull()?.value ?: 0
+
+        val valueThisYear = entries.groupedSum(
+            truncateField = YEAR,
+            isNumerical = habit.isNumerical
+        ).firstOrNull()?.value ?: 0
 
         val cal = DateUtils.getStartOfTodayCalendarWithOffset()
         val daysInMonth = cal.getActualMaximum(Calendar.DAY_OF_MONTH)
@@ -80,11 +108,11 @@ class TargetCardPresenter(
         val targetThisYear = targetToday * daysInYear
 
         val values = ArrayList<Double>()
-        if (habit.frequency.denominator <= 1) values.add(valueToday)
+        if (habit.frequency.denominator <= 1) values.add(valueToday / 1e3)
-        if (habit.frequency.denominator <= 7) values.add(valueThisWeek)
+        if (habit.frequency.denominator <= 7) values.add(valueThisWeek / 1e3)
-        values.add(valueThisMonth)
+        values.add(valueThisMonth / 1e3)
-        values.add(valueThisQuarter)
+        values.add(valueThisQuarter / 1e3)
-        values.add(valueThisYear)
+        values.add(valueThisYear / 1e3)
 
         val targets = ArrayList<Double>()
         if (habit.frequency.denominator <= 1) targets.add(targetToday)

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

@@ -28,6 +28,7 @@ import java.util.ArrayList
 import java.util.Calendar
 import javax.annotation.concurrent.ThreadSafe
 import kotlin.collections.set
+import kotlin.math.max
 import kotlin.math.min
 
 @ThreadSafe
@@ -79,45 +80,6 @@ open class EntryList {
         return entriesByTimestamp.values.sortedBy { it.timestamp }.reversed()
     }
 
-    /**
-     * Truncates the timestamps of all known entries, then aggregates their values. This function
-     * is used to generate bar plots where each bar shows the number of repetitions in a given week,
-     * month or year.
-     *
-     * For boolean habits, the value of the aggregated entry equals to the number of YES_MANUAL
-     * 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,
-        firstWeekday: Int,
-        isNumerical: Boolean,
-    ): List<Entry> {
-        val truncated = original.map {
-            Entry(it.timestamp.truncate(field, firstWeekday), it.value)
-        }
-        val timestamps = mutableListOf<Timestamp>()
-        val values = mutableListOf<Int>()
-        for (i in truncated.indices) {
-            if (i == 0 || timestamps.last() != truncated[i].timestamp) {
-                timestamps.add(truncated[i].timestamp)
-                values.add(0)
-            }
-            if (isNumerical) {
-                if (truncated[i].value > 0) {
-                    values[values.lastIndex] += truncated[i].value
-                }
-            } else {
-                if (truncated[i].value == YES_MANUAL) {
-                    values[values.lastIndex] += 1000
-                }
-            }
-        }
-        return timestamps.indices.map { Entry(timestamps[it], values[it]) }
-    }
-
     /**
      * Replaces all entries in this list by entries computed automatically from another list.
      *
@@ -189,55 +151,6 @@ open class EntryList {
         return map
     }
 
-    @Deprecated("")
-    @Synchronized
-    open fun getThisWeekValue(firstWeekday: Int, isNumerical: Boolean): Int {
-        return getThisIntervalValue(
-            truncateField = DateUtils.TruncateField.WEEK_NUMBER,
-            firstWeekday = firstWeekday,
-            isNumerical = isNumerical
-        )
-    }
-
-    @Deprecated("")
-    @Synchronized
-    open fun getThisMonthValue(isNumerical: Boolean): Int {
-        return getThisIntervalValue(
-            truncateField = DateUtils.TruncateField.MONTH,
-            firstWeekday = Calendar.SATURDAY,
-            isNumerical = isNumerical
-        )
-    }
-
-    @Deprecated("")
-    @Synchronized
-    open fun getThisQuarterValue(isNumerical: Boolean): Int {
-        return getThisIntervalValue(
-            truncateField = DateUtils.TruncateField.QUARTER,
-            firstWeekday = Calendar.SATURDAY,
-            isNumerical = isNumerical
-        )
-    }
-
-    @Deprecated("")
-    @Synchronized
-    open fun getThisYearValue(isNumerical: Boolean): Int {
-        return getThisIntervalValue(
-            truncateField = DateUtils.TruncateField.YEAR,
-            firstWeekday = Calendar.SATURDAY,
-            isNumerical = isNumerical
-        )
-    }
-
-    private fun getThisIntervalValue(
-        truncateField: DateUtils.TruncateField,
-        firstWeekday: Int,
-        isNumerical: Boolean,
-    ): Int {
-        val groups: List<Entry> = groupBy(getKnown(), truncateField, firstWeekday, isNumerical)
-        return if (groups.isEmpty()) 0 else groups[0].value
-    }
-
     data class Interval(val begin: Timestamp, val center: Timestamp, val end: Timestamp) {
         val length: Int
             get() = begin.daysUntil(end) + 1
@@ -344,3 +257,41 @@ open class EntryList {
         }
     }
 }
+
+/**
+ * Given a list of entries, truncates the timestamp of each entry (according to the field given),
+ * groups the entries according to this truncated timestamp, then creates a new entry (t,v) for
+ * each group, where t is the truncated timestamp and v is the sum of the values of all entries in
+ * the group.
+ *
+ * For numerical habits, non-positive entry values are converted to zero. For boolean habits, each
+ * YES_MANUAL value is converted to 1000 and all other values are converted to zero.
+ *
+ * The returned list is sorted by timestamp, with the newest entry coming first and the oldest entry
+ * coming last. If the original list has gaps in it (for example, weeks or months without any
+ * entries), then the list produced by this method will also have gaps.
+ *
+ * The argument [firstWeekday] is only relevant when truncating by week.
+ */
+fun List<Entry>.groupedSum(
+    truncateField: DateUtils.TruncateField,
+    firstWeekday: Int = Calendar.SATURDAY,
+    isNumerical: Boolean,
+): List<Entry> {
+    return this.map { (timestamp, value) ->
+        if (isNumerical) {
+            Entry(timestamp, max(0, value))
+        } else {
+            Entry(timestamp, if (value == YES_MANUAL) 1000 else 0)
+        }
+    }.groupBy { entry ->
+        entry.timestamp.truncate(
+            truncateField,
+            firstWeekday,
+        )
+    }.entries.map { (timestamp, entries) ->
+        Entry(timestamp, entries.sumOf { it.value })
+    }.sortedBy { (timestamp, _) ->
+        - timestamp.unixTime
+    }
+}

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

@@ -21,9 +21,12 @@ package org.isoron.uhabits.core.models;
 
 import org.apache.commons.lang3.builder.*;
 import org.isoron.uhabits.core.utils.*;
+import org.jetbrains.annotations.*;
 
 import java.util.*;
 
+import kotlin.*;
+
 import static java.util.Calendar.*;
 
 public final class Timestamp implements Comparable<Timestamp>

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

@@ -26,7 +26,6 @@ import org.isoron.uhabits.core.models.EntryList
 import org.isoron.uhabits.core.models.Frequency
 import org.isoron.uhabits.core.models.Timestamp
 import org.isoron.uhabits.core.models.sqlite.records.EntryRecord
-import org.isoron.uhabits.core.utils.DateUtils
 
 class SQLiteEntryList(database: Database) : EntryList() {
     val repository = Repository(EntryRecord::class.java, database)
@@ -79,16 +78,6 @@ class SQLiteEntryList(database: Database) : EntryList() {
         return super.getKnown()
     }
 
-    override fun groupBy(
-        original: List<Entry>,
-        field: DateUtils.TruncateField,
-        firstWeekday: Int,
-        isNumerical: Boolean
-    ): List<Entry> {
-        loadRecords()
-        return super.groupBy(original, field, firstWeekday, isNumerical)
-    }
-
     override fun recomputeFrom(originalEntries: EntryList, frequency: Frequency, isNumerical: Boolean) {
         throw UnsupportedOperationException()
     }

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

@@ -142,10 +142,8 @@ class EntryListTest {
             entries.add(Entry(reference.minus(offsets[it]), values[it]))
         }
 
-        val byMonth = entries.groupBy(
+        val byMonth = entries.getKnown().groupedSum(
-            original = entries.getKnown(),
+            truncateField = DateUtils.TruncateField.MONTH,
-            field = DateUtils.TruncateField.MONTH,
-            firstWeekday = Calendar.SATURDAY,
             isNumerical = true,
         )
         assertThat(byMonth.size, equalTo(17))
@@ -153,10 +151,8 @@ class EntryListTest {
         assertThat(byMonth[6], equalTo(Entry(Timestamp.from(2013, Calendar.DECEMBER, 1), 1988)))
         assertThat(byMonth[12], equalTo(Entry(Timestamp.from(2013, Calendar.MAY, 1), 1271)))
 
-        val byQuarter = entries.groupBy(
+        val byQuarter = entries.getKnown().groupedSum(
-            original = entries.getKnown(),
+            truncateField = DateUtils.TruncateField.QUARTER,
-            field = DateUtils.TruncateField.QUARTER,
-            firstWeekday = Calendar.SATURDAY,
             isNumerical = true,
         )
         assertThat(byQuarter.size, equalTo(6))
@@ -164,10 +160,8 @@ class EntryListTest {
         assertThat(byQuarter[3], equalTo(Entry(Timestamp.from(2013, Calendar.JULY, 1), 3838)))
         assertThat(byQuarter[5], equalTo(Entry(Timestamp.from(2013, Calendar.JANUARY, 1), 4975)))
 
-        val byYear = entries.groupBy(
+        val byYear = entries.getKnown().groupedSum(
-            original = entries.getKnown(),
+            truncateField = DateUtils.TruncateField.YEAR,
-            field = DateUtils.TruncateField.YEAR,
-            firstWeekday = Calendar.SATURDAY,
             isNumerical = true,
         )
         assertThat(byYear.size, equalTo(2))
@@ -192,10 +186,8 @@ class EntryListTest {
             entries.add(Entry(reference.minus(offsets[it]), YES_MANUAL))
         }
 
-        val byMonth = entries.groupBy(
+        val byMonth = entries.getKnown().groupedSum(
-            original = entries.getKnown(),
+            truncateField = DateUtils.TruncateField.MONTH,
-            field = DateUtils.TruncateField.MONTH,
-            firstWeekday = Calendar.SATURDAY,
             isNumerical = false,
         )
         assertThat(byMonth.size, equalTo(17))
@@ -203,10 +195,8 @@ class EntryListTest {
         assertThat(byMonth[6], equalTo(Entry(Timestamp.from(2013, Calendar.DECEMBER, 1), 7_000)))
         assertThat(byMonth[12], equalTo(Entry(Timestamp.from(2013, Calendar.MAY, 1), 6_000)))
 
-        val byQuarter = entries.groupBy(
+        val byQuarter = entries.getKnown().groupedSum(
-            original = entries.getKnown(),
+            truncateField = DateUtils.TruncateField.QUARTER,
-            field = DateUtils.TruncateField.QUARTER,
-            firstWeekday = Calendar.SATURDAY,
             isNumerical = false,
         )
         assertThat(byQuarter.size, equalTo(6))
@@ -214,10 +204,8 @@ class EntryListTest {
         assertThat(byQuarter[3], equalTo(Entry(Timestamp.from(2013, Calendar.JULY, 1), 17_000)))
         assertThat(byQuarter[5], equalTo(Entry(Timestamp.from(2013, Calendar.JANUARY, 1), 20_000)))
 
-        val byYear = entries.groupBy(
+        val byYear = entries.getKnown().groupedSum(
-            original = entries.getKnown(),
+            truncateField = DateUtils.TruncateField.YEAR,
-            field = DateUtils.TruncateField.YEAR,
-            firstWeekday = Calendar.SATURDAY,
             isNumerical = false,
         )
         assertThat(byYear.size, equalTo(2))