Merge pull request streetcomplete#3425 from smichel17/building-levels…

…-sort

Use smart item selection for building levels

app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt

@@ -9,14 +9,11 @@ import android.view.View
 import androidx.core.view.postDelayed
 import androidx.preference.PreferenceManager
 
-import javax.inject.Inject
-
 import de.westnordost.streetcomplete.R
 import de.westnordost.streetcomplete.databinding.QuestGenericListBinding
 import de.westnordost.streetcomplete.view.image_select.GroupableDisplayItem
 import de.westnordost.streetcomplete.view.image_select.GroupedImageSelectAdapter
 import kotlin.math.max
-import kotlin.math.min
 
 /**
  * Abstract class for quests with a grouped list of images and one to select.
@@ -35,15 +32,22 @@ abstract class AGroupedImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnsw
     protected abstract val allItems: List<GroupableDisplayItem<I>>
     protected abstract val topItems: List<GroupableDisplayItem<I>>
 
-    @Inject internal lateinit var favs: LastPickedValuesStore<I>
+    internal lateinit var favs: LastPickedValuesStore<GroupableDisplayItem<I>>
 
     private val selectedItem get() = imageSelector.selectedItem
 
     protected open val itemsPerRow = 3
 
     override fun onAttach(ctx: Context) {
         super.onAttach(ctx)
-        favs = LastPickedValuesStore(PreferenceManager.getDefaultSharedPreferences(ctx.applicationContext))
+        val validSuggestions = allItems.mapNotNull { it.items }.flatten()
+        val stringToItem = validSuggestions.associateBy { it.value.toString() }
+        favs = LastPickedValuesStore(
+            PreferenceManager.getDefaultSharedPreferences(ctx.applicationContext),
+            key = javaClass.simpleName,
+            serialize = { item -> item.value.toString() },
+            deserialize = { value -> stringToItem[value] }
+        )
     }
 
     override fun onCreate(savedInstanceState: Bundle?) {
@@ -92,8 +96,7 @@ abstract class AGroupedImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnsw
     }
 
     private fun getInitialItems(): List<GroupableDisplayItem<I>> {
-        val validSuggestions = allItems.mapNotNull { it.items }.flatten()
-        return favs.getWeighted(javaClass.simpleName, 6, 30, topItems, validSuggestions)
+        return favs.get().mostCommonWithin(6, 30).padWith(topItems).toList()
     }
 
     override fun onClickOk() {
@@ -114,14 +117,14 @@ abstract class AGroupedImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnsw
                         .setMessage(R.string.quest_generic_item_confirmation)
                         .setNegativeButton(R.string.quest_generic_confirmation_no, null)
                         .setPositiveButton(R.string.quest_generic_confirmation_yes) { _, _ ->
-                            favs.add(javaClass.simpleName, itemValue, allowDuplicates = true)
+                            favs.add(item)
                             onClickOk(itemValue)
                         }
                         .show()
                 }
             }
             else {
-                favs.add(javaClass.simpleName, itemValue, allowDuplicates = true)
+                favs.add(item)
                 onClickOk(itemValue)
             }
         }

app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt

@@ -11,7 +11,6 @@ import de.westnordost.streetcomplete.R
 import de.westnordost.streetcomplete.databinding.QuestGenericListBinding
 import de.westnordost.streetcomplete.view.image_select.DisplayItem
 import de.westnordost.streetcomplete.view.image_select.ImageSelectAdapter
-import java.util.*
 import kotlin.collections.ArrayList
 
 /**
@@ -34,7 +33,7 @@ abstract class AImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnswerFragm
 
     protected lateinit var imageSelector: ImageSelectAdapter<I>
 
-    private lateinit var favs: LastPickedValuesStore<I>
+    private lateinit var favs: LastPickedValuesStore<DisplayItem<I>>
 
     protected open val itemsPerRow = 4
     /** return -1 for any number. Default: 1  */
@@ -52,7 +51,13 @@ abstract class AImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnswerFragm
 
     override fun onAttach(ctx: Context) {
         super.onAttach(ctx)
-        favs = LastPickedValuesStore(PreferenceManager.getDefaultSharedPreferences(ctx.applicationContext))
+        val stringToItem = items.associateBy { it.value.toString() }
+        favs = LastPickedValuesStore(
+            PreferenceManager.getDefaultSharedPreferences(ctx.applicationContext),
+            key = javaClass.simpleName,
+            serialize = { item -> item.value.toString() },
+            deserialize = { value -> stringToItem[value] }
+        )
     }
 
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
@@ -89,8 +94,8 @@ abstract class AImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnswerFragm
     override fun onClickOk() {
         val values = imageSelector.selectedItems
         if (values.isNotEmpty()) {
-            favs.add(javaClass.simpleName, values)
-            onClickOk(values)
+            favs.add(values)
+            onClickOk(values.map { it.value!! })
         }
     }
 
@@ -106,7 +111,7 @@ abstract class AImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnswerFragm
 
     private fun moveFavouritesToFront(originalList: List<DisplayItem<I>>): List<DisplayItem<I>> {
         return if (originalList.size > itemsPerRow && moveFavoritesToFront) {
-            favs.moveLastPickedDisplayItemsToFront(javaClass.simpleName, originalList, originalList)
+            favs.get().filterNotNull().padWith(originalList).toList()
         } else {
             originalList
         }

app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt

@@ -3,63 +3,48 @@ package de.westnordost.streetcomplete.quests
 import android.content.SharedPreferences
 import androidx.core.content.edit
 
-import javax.inject.Inject
-
 import de.westnordost.streetcomplete.Prefs
-import de.westnordost.streetcomplete.view.image_select.DisplayItem
-import de.westnordost.streetcomplete.view.image_select.GroupableDisplayItem
-import kotlin.math.min
-
-/** T must be a string or enum - something that distinctly converts toString. */
-class LastPickedValuesStore<T> @Inject constructor(private val prefs: SharedPreferences) {
 
-    fun add(key: String, newValues: Iterable<T>, max: Int = MAX_ENTRIES, allowDuplicates: Boolean = false) {
-        val values = newValues.asSequence().map { it.toString() } + get(key)
-        val lastValues = if (allowDuplicates) values else values.distinct()
+class LastPickedValuesStore<T : Any>(
+    private val prefs: SharedPreferences,
+    private val key: String,
+    private val serialize: (T) -> String,
+    private val deserialize: (String) -> T? // null = unwanted value, see mostCommonWithin
+) {
+    fun add(newValues: Iterable<T>) {
+        val lastValues = newValues.asSequence().map(serialize) + getRaw()
         prefs.edit {
-            putString(getKey(key), lastValues.take(max).joinToString(","))
+            putString(getKey(), lastValues.take(MAX_ENTRIES).joinToString(","))
         }
     }
 
-    fun add(key: String, value: T, max: Int = MAX_ENTRIES, allowDuplicates: Boolean = false) {
-        add(key, listOf(value), max, allowDuplicates)
-    }
+    fun add(value: T) = add(listOf(value))
 
-    fun get(key: String): Sequence<String> =
-        prefs.getString(getKey(key), null)?.splitToSequence(",") ?: sequenceOf()
+    fun get(): Sequence<T?> = getRaw().map(deserialize)
 
-    private fun getKey(key: String) = Prefs.LAST_PICKED_PREFIX + key
+    private fun getRaw(): Sequence<String> =
+        prefs.getString(getKey(), null)?.splitToSequence(",") ?: sequenceOf()
+
+    private fun getKey() = Prefs.LAST_PICKED_PREFIX + key
 }
 
 private const val MAX_ENTRIES = 100
 
-/* Returns `count` unique items, sorted by how often they appear in the last `historyCount` answers.
- * If fewer than `count` unique items are found, look farther back in the history.
- * Only returns items in `itemPool` ("valid"), although other answers count towards `historyCount`.
- * If there are not enough unique items in the whole history, add unique `defaultItems` as needed.
- * Always include the most recent answer, if it is in `itemPool`, but still sorted normally. So, if
- * it is not one of the `count` most frequent items, it will replace the last of those.
- *
- * impl: null represents items not in the item pool
+/* In the first `historyCount` items, return the `count` most-common non-null items, in order.
+ * If the first item is not included (and is not null), it replaces the last of the common items.
+ * If fewer than `count` unique items are found, continue counting items until that many are found,
+ * or the end of the sequence is reached.
  */
-fun <T> LastPickedValuesStore<T>.getWeighted(
-    key: String,
-    count: Int,
-    historyCount: Int,
-    defaultItems: List<GroupableDisplayItem<T>>,
-    itemPool: List<GroupableDisplayItem<T>>
-): List<GroupableDisplayItem<T>> {
-    val stringToItem = itemPool.associateBy { it.value.toString() }
-    val lastPickedItems = get(key).map { stringToItem.get(it) }
-    val counts = lastPickedItems.countUniqueNonNull(historyCount, count)
-    val topRecent = counts.keys.sortedByDescending { counts.get(it) }
-    val latest = lastPickedItems.take(1).filterNotNull()
-    val items = (latest + topRecent + defaultItems).distinct().take(count)
-    return items.sortedByDescending { counts.get(it) }.toList()
+fun <T : Any> Sequence<T?>.mostCommonWithin(count: Int, historyCount: Int): Sequence<T> {
+    val counts = this.countUniqueNonNull(historyCount, count)
+    val top = counts.keys.sortedByDescending { counts.get(it) }
+    val latest = this.take(1).filterNotNull()
+    val items = (latest + top).distinct().take(count)
+    return items.sortedByDescending { counts.get(it) }
 }
 
 // Counts at least the first `minItems`, keeps going until it finds at least `target` unique values
-private fun <T> Sequence<T?>.countUniqueNonNull(minItems: Int, target: Int): Map<T, Int> {
+private fun <T : Any> Sequence<T?>.countUniqueNonNull(minItems: Int, target: Int): Map<T, Int> {
     val counts = mutableMapOf<T, Int>()
     val items = takeAtLeastWhile(minItems) { counts.size < target }.filterNotNull()
     return items.groupingBy { it }.eachCountTo(counts)
@@ -69,12 +54,5 @@ private fun <T> Sequence<T?>.countUniqueNonNull(minItems: Int, target: Int): Map
 private fun <T> Sequence<T>.takeAtLeastWhile(count: Int, predicate: (T) -> Boolean): Sequence<T> =
     withIndex().takeWhile{ (i, t) -> i < count || predicate(t) }.map { it.value }
 
-fun <T> LastPickedValuesStore<T>.moveLastPickedDisplayItemsToFront(
-    key: String,
-    defaultItems: List<DisplayItem<T>>,
-    itemPool: List<DisplayItem<T>>
-): List<DisplayItem<T>> {
-    val stringToItem = itemPool.associateBy { it.value.toString() }
-    val lastPickedItems = get(key).mapNotNull { stringToItem.get(it) }
-    return (lastPickedItems + defaultItems).distinct().toList()
-}
+fun <T> Sequence<T>.padWith(defaults: List<T>, count: Int = defaults.size) =
+    (this + defaults).distinct().take(count)

app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt

@@ -1,21 +1,21 @@
 package de.westnordost.streetcomplete.quests.building_levels
 
+import android.content.Context
 import android.os.Bundle
 import android.view.LayoutInflater
 import androidx.appcompat.app.AlertDialog
 import android.view.View
 import android.view.ViewGroup
+import androidx.preference.PreferenceManager
 import androidx.recyclerview.widget.RecyclerView
 
-import javax.inject.Inject
-
-import de.westnordost.streetcomplete.Injector
 import de.westnordost.streetcomplete.R
 import de.westnordost.streetcomplete.databinding.QuestBuildingLevelsBinding
 import de.westnordost.streetcomplete.databinding.QuestBuildingLevelsLastPickedButtonBinding
 import de.westnordost.streetcomplete.quests.AbstractQuestFormAnswerFragment
 import de.westnordost.streetcomplete.quests.LastPickedValuesStore
 import de.westnordost.streetcomplete.quests.AnswerItem
+import de.westnordost.streetcomplete.quests.mostCommonWithin
 import de.westnordost.streetcomplete.util.TextChangedWatcher
 
 class AddBuildingLevelsForm : AbstractQuestFormAnswerFragment<BuildingLevelsAnswer>() {
@@ -31,15 +31,24 @@ class AddBuildingLevelsForm : AbstractQuestFormAnswerFragment<BuildingLevelsAnsw
     private val roofLevels get() = binding.roofLevelsInput.text?.toString().orEmpty().trim()
 
     private val lastPickedAnswers by lazy {
-        favs.get(javaClass.simpleName).map { it.toBuildingLevelAnswer() }.sortedWith(
-            compareBy<BuildingLevelsAnswer> { it.levels }.thenBy { it.roofLevels }
-        ).toList()
+        favs.get()
+            .mostCommonWithin(5, 30)
+            .sortedWith(compareBy<BuildingLevelsAnswer> { it.levels }.thenBy { it.roofLevels })
+            .toList()
     }
 
-    @Inject internal lateinit var favs: LastPickedValuesStore<String>
-
-    init {
-        Injector.applicationComponent.inject(this)
+    internal lateinit var favs: LastPickedValuesStore<BuildingLevelsAnswer>
+
+    override fun onAttach(ctx: Context) {
+        super.onAttach(ctx)
+        favs = LastPickedValuesStore(
+            PreferenceManager.getDefaultSharedPreferences(ctx.applicationContext),
+            key = javaClass.simpleName,
+            serialize = { item -> listOfNotNull(item.levels, item.roofLevels).joinToString("#") },
+            deserialize = { value ->
+                value.split("#").let { BuildingLevelsAnswer(it[0].toInt(), it.getOrNull(1)?.toInt()) }
+            }
+        )
     }
 
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
@@ -64,7 +73,7 @@ class AddBuildingLevelsForm : AbstractQuestFormAnswerFragment<BuildingLevelsAnsw
     override fun onClickOk() {
         val roofLevelsNumber = if (roofLevels.isEmpty()) null else roofLevels.toInt()
         val answer = BuildingLevelsAnswer(levels.toInt(), roofLevelsNumber)
-        favs.add(javaClass.simpleName, answer.toSerializedString(), max = 5)
+        favs.add(answer)
         applyAnswer(answer)
     }
 
@@ -111,9 +120,3 @@ private class LastPickedAdapter(
 
     override fun getItemCount() = lastPickedAnswers.size
 }
-
-private fun BuildingLevelsAnswer.toSerializedString() =
-    listOfNotNull(levels, roofLevels).joinToString("#")
-
-private fun String.toBuildingLevelAnswer() =
-    this.split("#").let { BuildingLevelsAnswer(it[0].toInt(), it.getOrNull(1)?.toInt()) }

app/src/main/java/de/westnordost/streetcomplete/view/image_select/ImageSelectAdapter.kt

@@ -24,7 +24,7 @@ class ImageSelectAdapter<T>(private val maxSelectableIndices: Int = -1) :
 
     val listeners: MutableList<OnItemSelectionListener> = CopyOnWriteArrayList()
 
-    val selectedItems get() = _selectedIndices.map { i -> items[i].value!! }
+    val selectedItems get() = _selectedIndices.map { i -> items[i] }
 
     interface OnItemSelectionListener {
         fun onIndexSelected(index: Int)