From 8fc7bb576b1c498916c569e8d9ab4ac41399350e Mon Sep 17 00:00:00 2001
From: Stephen Michel <git@smichel.me>
Date: Fri, 22 Oct 2021 09:17:07 -0400
Subject: [PATCH 01/14] Use smart item selection for building levels
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

…but keep the same sort order
---
 .../streetcomplete/quests/LastPickedValuesStore.kt   | 12 +++++++++++-
 .../quests/building_levels/AddBuildingLevelsForm.kt  |  7 ++++---
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
index 2f756606e5..12ee312554 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
@@ -49,7 +49,17 @@ fun <T> LastPickedValuesStore<T>.getWeighted(
     itemPool: List<GroupableDisplayItem<T>>
 ): List<GroupableDisplayItem<T>> {
     val stringToItem = itemPool.associateBy { it.value.toString() }
-    val lastPickedItems = get(key).map { stringToItem.get(it) }
+    return getWeighted(key, count, historyCount, defaultItems, stringToItem::get)
+}
+
+fun <T, I> LastPickedValuesStore<T>.getWeighted(
+    key: String,
+    count: Int,
+    historyCount: Int,
+    defaultItems: List<I>,
+    deserialize: (String) -> I?
+): List<I> {
+    val lastPickedItems = get(key).map(deserialize)
     val counts = lastPickedItems.countUniqueNonNull(historyCount, count)
     val topRecent = counts.keys.sortedByDescending { counts.get(it) }
     val latest = lastPickedItems.take(1).filterNotNull()
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
index 3c199e206e..685e3a7fea 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
@@ -16,6 +16,7 @@ import de.westnordost.streetcomplete.databinding.QuestBuildingLevelsLastPickedBu
 import de.westnordost.streetcomplete.quests.AbstractQuestFormAnswerFragment
 import de.westnordost.streetcomplete.quests.LastPickedValuesStore
 import de.westnordost.streetcomplete.quests.AnswerItem
+import de.westnordost.streetcomplete.quests.getWeighted
 import de.westnordost.streetcomplete.util.TextChangedWatcher
 
 class AddBuildingLevelsForm : AbstractQuestFormAnswerFragment<BuildingLevelsAnswer>() {
@@ -31,9 +32,9 @@ 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.getWeighted(javaClass.simpleName, 5, 30, listOf()) { it.toBuildingLevelAnswer() }
+            .sortedWith(compareBy<BuildingLevelsAnswer> { it.levels }.thenBy { it.roofLevels })
+            .toList()
     }
 
     @Inject internal lateinit var favs: LastPickedValuesStore<String>

From 7affd151b21209b61b5e0ac507b5f4e84bf267cf Mon Sep 17 00:00:00 2001
From: Stephen Michel <git@smichel.me>
Date: Fri, 22 Oct 2021 11:15:39 -0400
Subject: [PATCH 02/14] Store more last-used building levels

Counting the recent values doesn't do much good if you're still only
saving 5
---
 .../quests/building_levels/AddBuildingLevelsForm.kt             | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
index 685e3a7fea..9c44f487a6 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
@@ -65,7 +65,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(javaClass.simpleName, answer.toSerializedString(), allowDuplicates = true)
         applyAnswer(answer)
     }
 

From d6aaba424d999109a486f4efb0cfde90cf527355 Mon Sep 17 00:00:00 2001
From: Stephen Michel <git@smichel.me>
Date: Fri, 22 Oct 2021 11:42:56 -0400
Subject: [PATCH 03/14] Allow duplicates in all `LastUsedValuesStore`s
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We *already* de-duplicate when retrieving values, in all cases where
that's desired, so also de-duplicating when storing values has no impact
on functionality— it's essentially a performance improvement, to reduce
the number of values we store. At 100 values maximum, I don't think this
makes a meaningful difference.

By removing the optimization, we now store history for all values, so if
we choose to enable "smart sort" for other quests in the future, they'll
already have history to pull from.

Also, it simplifies the code, just a little.
---
 .../quests/AGroupedImageListQuestAnswerFragment.kt    |  4 ++--
 .../streetcomplete/quests/LastPickedValuesStore.kt    | 11 ++++-------
 .../quests/building_levels/AddBuildingLevelsForm.kt   |  2 +-
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
index 652b850e9e..53e290276b 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
@@ -114,14 +114,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(javaClass.simpleName, itemValue)
                             onClickOk(itemValue)
                         }
                         .show()
                 }
             }
             else {
-                favs.add(javaClass.simpleName, itemValue, allowDuplicates = true)
+                favs.add(javaClass.simpleName, itemValue)
                 onClickOk(itemValue)
             }
         }
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
index 12ee312554..dbd4dd84fa 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
@@ -13,17 +13,14 @@ 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()
+    fun add(key: String, newValues: Iterable<T>) {
+        val lastValues = newValues.asSequence().map { it.toString() } + get(key)
         prefs.edit {
-            putString(getKey(key), lastValues.take(max).joinToString(","))
+            putString(getKey(key), 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(key: String, value: T) = add(key, listOf(value))
 
     fun get(key: String): Sequence<String> = prefs.getString(getKey(key), "")!!.splitToSequence(",")
 
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
index 9c44f487a6..29e0866815 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
@@ -65,7 +65,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(), allowDuplicates = true)
+        favs.add(javaClass.simpleName, answer.toSerializedString())
         applyAnswer(answer)
     }
 

From 1000a6ac5133ce8a3c4e0cbb56461ed5e5ea8f3f Mon Sep 17 00:00:00 2001
From: Stephen Michel <git@smichel.me>
Date: Sat, 23 Oct 2021 10:20:33 -0400
Subject: [PATCH 04/14] Clean up LastPickedValuesStore interface, part 1

This time: remove the need for the BuildingLevels form to pass in an
empty list for defaults
---
 .../quests/LastPickedValuesStore.kt               | 15 +++++++++------
 .../building_levels/AddBuildingLevelsForm.kt      |  2 +-
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
index dbd4dd84fa..73ac781da9 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
@@ -46,22 +46,22 @@ fun <T> LastPickedValuesStore<T>.getWeighted(
     itemPool: List<GroupableDisplayItem<T>>
 ): List<GroupableDisplayItem<T>> {
     val stringToItem = itemPool.associateBy { it.value.toString() }
-    return getWeighted(key, count, historyCount, defaultItems, stringToItem::get)
+    val weightedItems = getWeighted(key, count, historyCount, stringToItem::get)
+    return weightedItems.padWith(defaultItems).toList()
 }
 
 fun <T, I> LastPickedValuesStore<T>.getWeighted(
     key: String,
     count: Int,
     historyCount: Int,
-    defaultItems: List<I>,
     deserialize: (String) -> I?
-): List<I> {
+): Sequence<I> {
     val lastPickedItems = get(key).map(deserialize)
     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()
+    val items = (latest + topRecent).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
@@ -82,5 +82,8 @@ fun <T> LastPickedValuesStore<T>.moveLastPickedDisplayItemsToFront(
 ): List<DisplayItem<T>> {
     val stringToItem = itemPool.associateBy { it.value.toString() }
     val lastPickedItems = get(key).mapNotNull { stringToItem.get(it) }
-    return (lastPickedItems + defaultItems).distinct().toList()
+    return lastPickedItems.padWith(defaultItems).toList()
 }
+
+private fun <T> Sequence<T>.padWith(defaults: List<T>, count: Int = defaults.size) =
+    (this + defaults).distinct().take(count)
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
index 29e0866815..9d7529b9b6 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
@@ -32,7 +32,7 @@ class AddBuildingLevelsForm : AbstractQuestFormAnswerFragment<BuildingLevelsAnsw
     private val roofLevels get() = binding.roofLevelsInput.text?.toString().orEmpty().trim()
 
     private val lastPickedAnswers by lazy {
-        favs.getWeighted(javaClass.simpleName, 5, 30, listOf()) { it.toBuildingLevelAnswer() }
+        favs.getWeighted(javaClass.simpleName, 5, 30) { it.toBuildingLevelAnswer() }
             .sortedWith(compareBy<BuildingLevelsAnswer> { it.levels }.thenBy { it.roofLevels })
             .toList()
     }

From 900b86ffd0503023a3038a61b0f90462200af0fb Mon Sep 17 00:00:00 2001
From: Stephen Michel <git@smichel.me>
Date: Sat, 23 Oct 2021 10:53:39 -0400
Subject: [PATCH 05/14] Clean up LastPickedValueStore interface, part 2

Make "smart" sorting of recently-picked elements work on any sequence.
It's more flexible now, and has a simpler type signature.
---
 .../quests/LastPickedValuesStore.kt           | 22 +++++++------------
 .../building_levels/AddBuildingLevelsForm.kt  |  5 +++--
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
index 73ac781da9..74252140ec 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
@@ -46,26 +46,20 @@ fun <T> LastPickedValuesStore<T>.getWeighted(
     itemPool: List<GroupableDisplayItem<T>>
 ): List<GroupableDisplayItem<T>> {
     val stringToItem = itemPool.associateBy { it.value.toString() }
-    val weightedItems = getWeighted(key, count, historyCount, stringToItem::get)
-    return weightedItems.padWith(defaultItems).toList()
+    val lastPickedItems = get(key).map { stringToItem.get(it) }
+    return lastPickedItems.mostCommonWithin(count, historyCount).padWith(defaultItems).toList()
 }
 
-fun <T, I> LastPickedValuesStore<T>.getWeighted(
-    key: String,
-    count: Int,
-    historyCount: Int,
-    deserialize: (String) -> I?
-): Sequence<I> {
-    val lastPickedItems = get(key).map(deserialize)
-    val counts = lastPickedItems.countUniqueNonNull(historyCount, count)
-    val topRecent = counts.keys.sortedByDescending { counts.get(it) }
-    val latest = lastPickedItems.take(1).filterNotNull()
-    val items = (latest + topRecent).distinct().take(count)
+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)
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
index 9d7529b9b6..55226bb9d2 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
@@ -16,7 +16,7 @@ import de.westnordost.streetcomplete.databinding.QuestBuildingLevelsLastPickedBu
 import de.westnordost.streetcomplete.quests.AbstractQuestFormAnswerFragment
 import de.westnordost.streetcomplete.quests.LastPickedValuesStore
 import de.westnordost.streetcomplete.quests.AnswerItem
-import de.westnordost.streetcomplete.quests.getWeighted
+import de.westnordost.streetcomplete.quests.mostCommonWithin
 import de.westnordost.streetcomplete.util.TextChangedWatcher
 
 class AddBuildingLevelsForm : AbstractQuestFormAnswerFragment<BuildingLevelsAnswer>() {
@@ -32,7 +32,8 @@ class AddBuildingLevelsForm : AbstractQuestFormAnswerFragment<BuildingLevelsAnsw
     private val roofLevels get() = binding.roofLevelsInput.text?.toString().orEmpty().trim()
 
     private val lastPickedAnswers by lazy {
-        favs.getWeighted(javaClass.simpleName, 5, 30) { it.toBuildingLevelAnswer() }
+        favs.get(javaClass.simpleName).map { it.toBuildingLevelAnswer() }
+            .mostCommonWithin(5, 30)
             .sortedWith(compareBy<BuildingLevelsAnswer> { it.levels }.thenBy { it.roofLevels })
             .toList()
     }

From 07e0d265f358c652dc3c6471be4aa5ed5c4aa9f0 Mon Sep 17 00:00:00 2001
From: Stephen Michel <git@smichel.me>
Date: Mon, 25 Oct 2021 19:59:36 -0400
Subject: [PATCH 06/14] Make LastUsedValuesStore tests more specific

`mostCommonWithin` is the complicated function, that we really care
about testing
---
 .../quests/LastPickedValuesStoreTest.kt       | 69 +++++++------------
 1 file changed, 25 insertions(+), 44 deletions(-)

diff --git a/app/src/test/java/de/westnordost/streetcomplete/quests/LastPickedValuesStoreTest.kt b/app/src/test/java/de/westnordost/streetcomplete/quests/LastPickedValuesStoreTest.kt
index 827e166916..8e8e95140b 100644
--- a/app/src/test/java/de/westnordost/streetcomplete/quests/LastPickedValuesStoreTest.kt
+++ b/app/src/test/java/de/westnordost/streetcomplete/quests/LastPickedValuesStoreTest.kt
@@ -5,70 +5,51 @@ import de.westnordost.streetcomplete.testutils.mock
 import de.westnordost.streetcomplete.testutils.on
 import de.westnordost.streetcomplete.view.image_select.Item
 import org.junit.Assert.assertEquals
-import org.junit.Before
 import org.junit.Test
 
 
 class LastPickedValuesStoreTest {
 
-    private lateinit var favs: LastPickedValuesStore<Letter>
-    private val key: String = javaClass.simpleName
-
-    private val allItems = Letter.values().toList().toItems()
-    private val defaultItems = listOf(X, Y, Z).toItems()
+    @Test fun `weighted sort returns the default items when there is no history`() {
+        val allItems = Letter.values().toList().map { Item(it) }
+        val defaultItems = listOf(A, B, C).map { Item(it) }
 
-    @Before fun setUp() {
-        favs = mock()
-    }
+        val favs = mock<LastPickedValuesStore<Letter>>()
+        on(favs.get(javaClass.simpleName)).thenReturn(sequenceOf())
 
-    @Test fun `weighted sort returns the default items when there is no history`() {
-        on(favs.get(key)).thenReturn(sequenceOf())
-        val returnedItems = favs.getWeighted(key, 4, 99, defaultItems, allItems)
+        val returnedItems = favs.getWeighted(javaClass.simpleName, 4, 99, defaultItems, allItems)
         assertEquals(defaultItems, returnedItems)
     }
 
-    @Test fun `weighted sort considers frequency first, then recency, then defaults`() {
-        on(favs.get(key)).thenReturn(sequenceOf("A", "C", "B", "B", "C", "D"))
-        val returnedItems = favs.getWeighted(key, 6, 99, defaultItems, allItems)
-        val expectedItems = listOf(C, B, A, D, X, Y).toItems()
-        assertEquals(expectedItems, returnedItems)
+    @Test fun `mostCommonWithin sorts by frequency first, then recency`() {
+        val items = sequenceOf(A, C, B, B, C, D)
+        assertEquals(items.mostCommonWithin(4, 99).toList(), listOf(C, B, A, D))
     }
 
-    @Test fun `weighted sort returns most recent item even if it is not the most picked`() {
-        on(favs.get(key)).thenReturn(sequenceOf("A", "B", "B", "B", "C", "C"))
-        val returnedItems = favs.getWeighted(key, 2, 99, defaultItems, allItems)
-        val expectedItems = listOf(B, A).toItems()
-        assertEquals(expectedItems, returnedItems)
+    @Test fun `mostCommonWithin includes the most recent item even if it is not the most common`() {
+        val items = sequenceOf(A, B, B, B, C, C)
+        assertEquals(items.mostCommonWithin(2, 99).toList(), listOf(B, A))
     }
 
-    @Test fun `weighted sort doesn't return duplicates`() {
-        on(favs.get(key)).thenReturn(sequenceOf("X", "Y", "X", "A"))
-        val returnedItems = favs.getWeighted(key, 4, 99, defaultItems, allItems)
-        val expectedItems = listOf(X, Y, A, Z).toItems()
-        assertEquals(expectedItems, returnedItems)
+    @Test fun `mostCommonWithin doesn't return duplicates`() {
+        val items = sequenceOf(A, B, A, B)
+        assertEquals(items.mostCommonWithin(4, 99).toList(), listOf(A, B))
     }
 
-    @Test fun `weighted sort only returns items in itemPool (the most recent is not exempt)`() {
-        on(favs.get(key)).thenReturn(sequenceOf("p", "B", "q", "C"))
-        val returnedItems = favs.getWeighted(key, 2, 99, defaultItems, allItems)
-        val expectedItems = listOf(B, C).toItems()
-        assertEquals(expectedItems, returnedItems)
+    @Test fun `mostCommonWithin doesn't include the first item if it's null`() {
+        val items = sequenceOf(null, B, null, C)
+        assertEquals(items.mostCommonWithin(2, 99).toList(), listOf(B, C))
     }
 
-    @Test fun `weighted sort still counts non-itemPool values in the history window`() {
-        on(favs.get(key)).thenReturn(sequenceOf("A", "p", "q", "B", /**/ "B", "C", "D"))
-        val returnedItems = favs.getWeighted(key, 2, 4, defaultItems, allItems)
-        val expectedItems = listOf(A, B).toItems()
-        assertEquals(expectedItems, returnedItems)
+    @Test fun `mostCommonWithin includes nulls in the number of items to count`() {
+        val items = sequenceOf(A, null, null, B, /* stops here */ B, C, D)
+        assertEquals(items.mostCommonWithin(2, 4).toList(), listOf(A, B))
     }
 
-    @Test fun `weighted sort extends the history window (only) as needed to find enough items`() {
-        on(favs.get(key)).thenReturn(sequenceOf("A", "p", "q", "B", "B", "C", /**/ "D"))
-        val returnedItems = favs.getWeighted(key, 3, 4, defaultItems, allItems)
-        val expectedItems = listOf(B, A, C).toItems()
-        assertEquals(expectedItems, returnedItems)
+    @Test fun `mostCommonWithin keeps counting until enough non-null items have been found`() {
+        val items = sequenceOf(A, null, null, B, B, C, /* stops here */ D)
+        assertEquals(items.mostCommonWithin(3, 4).toList(), listOf(B, A, C))
     }
 }
 
-private enum class Letter { A, B, C, D, X, Y, Z }
-private fun List<Letter>.toItems(): List<Item<Letter>> = this.map(::Item)
+private enum class Letter { A, B, C, D }

From c4dfa40e67bd6098007ae22cc1b68ced9ef24c28 Mon Sep 17 00:00:00 2001
From: Stephen Michel <git@smichel.me>
Date: Tue, 26 Oct 2021 09:07:09 -0400
Subject: [PATCH 07/14] Remove type from LastUsedValuesStore

Makes (de)serialization the responsibility of the caller, not the store.
---
 .../quests/AGroupedImageListQuestAnswerFragment.kt   |  6 +++---
 .../quests/AImageListQuestAnswerFragment.kt          |  4 ++--
 .../streetcomplete/quests/LastPickedValuesStore.kt   | 12 ++++++------
 .../quests/building_levels/AddBuildingLevelsForm.kt  |  2 +-
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
index 53e290276b..2b494cbe00 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
@@ -35,7 +35,7 @@ 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>
+    @Inject internal lateinit var favs: LastPickedValuesStore
 
     private val selectedItem get() = imageSelector.selectedItem
 
@@ -114,14 +114,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)
+                            favs.add(javaClass.simpleName, itemValue.toString())
                             onClickOk(itemValue)
                         }
                         .show()
                 }
             }
             else {
-                favs.add(javaClass.simpleName, itemValue)
+                favs.add(javaClass.simpleName, itemValue.toString())
                 onClickOk(itemValue)
             }
         }
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt
index 13f9f01b23..1a17ecaa0e 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt
@@ -34,7 +34,7 @@ abstract class AImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnswerFragm
 
     protected lateinit var imageSelector: ImageSelectAdapter<I>
 
-    private lateinit var favs: LastPickedValuesStore<I>
+    private lateinit var favs: LastPickedValuesStore
 
     protected open val itemsPerRow = 4
     /** return -1 for any number. Default: 1  */
@@ -89,7 +89,7 @@ abstract class AImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnswerFragm
     override fun onClickOk() {
         val values = imageSelector.selectedItems
         if (values.isNotEmpty()) {
-            favs.add(javaClass.simpleName, values)
+            favs.add(javaClass.simpleName, values.map { it.toString() })
             onClickOk(values)
         }
     }
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
index 1a81e97dc4..3b330ccbad 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
@@ -11,16 +11,16 @@ 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) {
+class LastPickedValuesStore @Inject constructor(private val prefs: SharedPreferences) {
 
-    fun add(key: String, newValues: Iterable<T>) {
-        val lastValues = newValues.asSequence().map { it.toString() } + get(key)
+    fun add(key: String, newValues: Iterable<String>) {
+        val lastValues = newValues.asSequence() + get(key)
         prefs.edit {
             putString(getKey(key), lastValues.take(MAX_ENTRIES).joinToString(","))
         }
     }
 
-    fun add(key: String, value: T) = add(key, listOf(value))
+    fun add(key: String, value: String) = add(key, listOf(value))
 
     fun get(key: String): Sequence<String> =
         prefs.getString(getKey(key), null)?.splitToSequence(",") ?: sequenceOf()
@@ -39,7 +39,7 @@ private const val MAX_ENTRIES = 100
  *
  * impl: null represents items not in the item pool
  */
-fun <T> LastPickedValuesStore<T>.getWeighted(
+fun <T> LastPickedValuesStore.getWeighted(
     key: String,
     count: Int,
     historyCount: Int,
@@ -70,7 +70,7 @@ private fun <T : Any> Sequence<T?>.countUniqueNonNull(minItems: Int, target: Int
 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(
+fun <T> LastPickedValuesStore.moveLastPickedDisplayItemsToFront(
     key: String,
     defaultItems: List<DisplayItem<T>>,
     itemPool: List<DisplayItem<T>>
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
index 55226bb9d2..82e61bde11 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
@@ -38,7 +38,7 @@ class AddBuildingLevelsForm : AbstractQuestFormAnswerFragment<BuildingLevelsAnsw
             .toList()
     }
 
-    @Inject internal lateinit var favs: LastPickedValuesStore<String>
+    @Inject internal lateinit var favs: LastPickedValuesStore
 
     init {
         Injector.applicationComponent.inject(this)

From 207e3ce3949fddf27b2f99ba90c6b6f3c95afe05 Mon Sep 17 00:00:00 2001
From: Stephen Michel <git@smichel.me>
Date: Tue, 26 Oct 2021 11:03:15 -0400
Subject: [PATCH 08/14] Re-type the LastPickedValuesStore
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

1. With the store being untyped, the compiler does not guarantee that
   the (de)serialization functions match— or that they both use the same
   key to store/retrieve the serialized values.

2. Serialization and deserialization are done relatively far apart.

Together, these make it hard for a human to notice if there's a problem.

Creating a 'Factory' (better name pending) forces the function types to
match, to be declared close together, and to be associated with the same
key. This should make it easy for humans to notice any issues… while
being easier for humans to spot issues *and* more flexible than the
previous typed implementation, where changing the (de)serialization
method required changes to the store.
---
 .../AGroupedImageListQuestAnswerFragment.kt   | 19 ++++--
 .../quests/AImageListQuestAnswerFragment.kt   | 16 +++--
 .../quests/LastPickedValuesStore.kt           | 59 ++++++-------------
 .../building_levels/AddBuildingLevelsForm.kt  | 22 ++++---
 .../view/image_select/ImageSelectAdapter.kt   |  2 +-
 5 files changed, 59 insertions(+), 59 deletions(-)

diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
index 2b494cbe00..e3d2a92e51 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
@@ -35,7 +35,17 @@ 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
+    @Inject internal lateinit var favs: LastPickedValuesStore<GroupableDisplayItem<I>>
+
+    private val favsFactory = object : LastPickedValuesStore.Factory<GroupableDisplayItem<I>> {
+        private val stringToItem by lazy {
+            allItems.mapNotNull { it.items }.flatten().associateBy { serialize(it) }
+        }
+
+        override val key = javaClass.simpleName
+        override fun serialize(item: GroupableDisplayItem<I>) = item.value.toString()
+        override fun deserialize(value: String) = stringToItem[value]
+    }
 
     private val selectedItem get() = imageSelector.selectedItem
 
@@ -92,8 +102,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(favsFactory).mostCommonWithin(6, 30).padWith(topItems).toList()
     }
 
     override fun onClickOk() {
@@ -114,14 +123,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.toString())
+                            favs.add(favsFactory, item)
                             onClickOk(itemValue)
                         }
                         .show()
                 }
             }
             else {
-                favs.add(javaClass.simpleName, itemValue.toString())
+                favs.add(favsFactory, item)
                 onClickOk(itemValue)
             }
         }
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt
index 1a17ecaa0e..0031b0e44b 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt
@@ -34,7 +34,7 @@ abstract class AImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnswerFragm
 
     protected lateinit var imageSelector: ImageSelectAdapter<I>
 
-    private lateinit var favs: LastPickedValuesStore
+    private lateinit var favs: LastPickedValuesStore<DisplayItem<I>>
 
     protected open val itemsPerRow = 4
     /** return -1 for any number. Default: 1  */
@@ -45,6 +45,14 @@ abstract class AImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnswerFragm
 
     protected abstract val items: List<DisplayItem<I>>
 
+    private val favsFactory = object : LastPickedValuesStore.Factory<DisplayItem<I>> {
+        private val stringToItem by lazy { items.associateBy { serialize(it) } }
+
+        override val key = javaClass.simpleName
+        override fun serialize(item: DisplayItem<I>) = item.value.toString()
+        override fun deserialize(value: String) = stringToItem[value]
+    }
+
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
         imageSelector = ImageSelectAdapter(maxSelectableItems)
@@ -89,8 +97,8 @@ abstract class AImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnswerFragm
     override fun onClickOk() {
         val values = imageSelector.selectedItems
         if (values.isNotEmpty()) {
-            favs.add(javaClass.simpleName, values.map { it.toString() })
-            onClickOk(values)
+            favs.add(favsFactory, values)
+            onClickOk(values.map { it.value!! })
         }
     }
 
@@ -106,7 +114,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(favsFactory).filterNotNull().padWith(originalList).toList()
         } else {
             originalList
         }
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
index 3b330ccbad..ccd439ad77 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
@@ -6,51 +6,40 @@ 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 @Inject constructor(private val prefs: SharedPreferences) {
+class LastPickedValuesStore<T : Any> @Inject constructor(private val prefs: SharedPreferences) {
 
-    fun add(key: String, newValues: Iterable<String>) {
-        val lastValues = newValues.asSequence() + get(key)
+    fun add(factory: Factory<T>, newValues: Iterable<T>) {
+        val lastValues = newValues.asSequence().map(factory::serialize) + getRaw(factory.key)
         prefs.edit {
-            putString(getKey(key), lastValues.take(MAX_ENTRIES).joinToString(","))
+            putString(getKey(factory.key), lastValues.take(MAX_ENTRIES).joinToString(","))
         }
     }
 
-    fun add(key: String, value: String) = add(key, listOf(value))
+    fun add(factory: Factory<T>, value: T) = add(factory, listOf(value))
 
-    fun get(key: String): Sequence<String> =
+    fun get(factory: Factory<T>): Sequence<T?> = getRaw(factory.key).map(factory::deserialize)
+
+    private fun getRaw(key: String): Sequence<String> =
         prefs.getString(getKey(key), null)?.splitToSequence(",") ?: sequenceOf()
 
     private fun getKey(key: String) = Prefs.LAST_PICKED_PREFIX + key
+
+    interface Factory<T : Any> {
+        val key: String
+        fun serialize(item: T): String
+        fun deserialize(value: String): T? // null = invalid value
+    }
 }
 
 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.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) }
-    return lastPickedItems.mostCommonWithin(count, historyCount).padWith(defaultItems).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) }
@@ -70,15 +59,5 @@ private fun <T : Any> Sequence<T?>.countUniqueNonNull(minItems: Int, target: Int
 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.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.padWith(defaultItems).toList()
-}
-
-private fun <T> Sequence<T>.padWith(defaults: List<T>, count: Int = defaults.size) =
+fun <T> Sequence<T>.padWith(defaults: List<T>, count: Int = defaults.size) =
     (this + defaults).distinct().take(count)
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
index 82e61bde11..b4881e3a61 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
@@ -32,13 +32,23 @@ 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() }
+        favs.get(favsFactory)
             .mostCommonWithin(5, 30)
             .sortedWith(compareBy<BuildingLevelsAnswer> { it.levels }.thenBy { it.roofLevels })
             .toList()
     }
 
-    @Inject internal lateinit var favs: LastPickedValuesStore
+    @Inject internal lateinit var favs: LastPickedValuesStore<BuildingLevelsAnswer>
+
+    val favsFactory = object : LastPickedValuesStore.Factory<BuildingLevelsAnswer> {
+        override val key = javaClass.simpleName
+
+        override fun serialize(item: BuildingLevelsAnswer): String =
+            listOfNotNull(item.levels, item.roofLevels).joinToString("#")
+
+        override fun deserialize(value: String) =
+            value.split("#").let { BuildingLevelsAnswer(it[0].toInt(), it.getOrNull(1)?.toInt()) }
+    }
 
     init {
         Injector.applicationComponent.inject(this)
@@ -66,7 +76,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())
+        favs.add(favsFactory, answer)
         applyAnswer(answer)
     }
 
@@ -113,9 +123,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()) }
diff --git a/app/src/main/java/de/westnordost/streetcomplete/view/image_select/ImageSelectAdapter.kt b/app/src/main/java/de/westnordost/streetcomplete/view/image_select/ImageSelectAdapter.kt
index 551a8c94f1..e33f9b7979 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/view/image_select/ImageSelectAdapter.kt
+++ b/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)

From f4b2f260cbce552e0876657cc78576eb9e76631d Mon Sep 17 00:00:00 2001
From: Stephen Michel <git@smichel.me>
Date: Tue, 26 Oct 2021 12:41:04 -0400
Subject: [PATCH 09/14] Clean up tests

Replace test for removed function with a test for a now-public function.
---
 .../quests/LastPickedValuesStoreTest.kt       | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/app/src/test/java/de/westnordost/streetcomplete/quests/LastPickedValuesStoreTest.kt b/app/src/test/java/de/westnordost/streetcomplete/quests/LastPickedValuesStoreTest.kt
index 8e8e95140b..cde8b89cb7 100644
--- a/app/src/test/java/de/westnordost/streetcomplete/quests/LastPickedValuesStoreTest.kt
+++ b/app/src/test/java/de/westnordost/streetcomplete/quests/LastPickedValuesStoreTest.kt
@@ -1,26 +1,12 @@
 package de.westnordost.streetcomplete.quests
 
 import de.westnordost.streetcomplete.quests.Letter.*
-import de.westnordost.streetcomplete.testutils.mock
-import de.westnordost.streetcomplete.testutils.on
-import de.westnordost.streetcomplete.view.image_select.Item
 import org.junit.Assert.assertEquals
 import org.junit.Test
 
 
 class LastPickedValuesStoreTest {
 
-    @Test fun `weighted sort returns the default items when there is no history`() {
-        val allItems = Letter.values().toList().map { Item(it) }
-        val defaultItems = listOf(A, B, C).map { Item(it) }
-
-        val favs = mock<LastPickedValuesStore<Letter>>()
-        on(favs.get(javaClass.simpleName)).thenReturn(sequenceOf())
-
-        val returnedItems = favs.getWeighted(javaClass.simpleName, 4, 99, defaultItems, allItems)
-        assertEquals(defaultItems, returnedItems)
-    }
-
     @Test fun `mostCommonWithin sorts by frequency first, then recency`() {
         val items = sequenceOf(A, C, B, B, C, D)
         assertEquals(items.mostCommonWithin(4, 99).toList(), listOf(C, B, A, D))
@@ -50,6 +36,11 @@ class LastPickedValuesStoreTest {
         val items = sequenceOf(A, null, null, B, B, C, /* stops here */ D)
         assertEquals(items.mostCommonWithin(3, 4).toList(), listOf(B, A, C))
     }
+
+    @Test fun `padWith doesn't include duplicates`() {
+        val items = sequenceOf(A, B).padWith(listOf(B, C, D, A))
+        assertEquals(items.toList(), listOf(A, B, C, D))
+    }
 }
 
 private enum class Letter { A, B, C, D }

From 6fd8d27981a80a12574739cef057a929958fe0ba Mon Sep 17 00:00:00 2001
From: Stephen Michel <git@smichel.me>
Date: Tue, 26 Oct 2021 12:53:40 -0400
Subject: [PATCH 10/14] Remove unused imports

---
 .../quests/AGroupedImageListQuestAnswerFragment.kt               | 1 -
 .../streetcomplete/quests/AImageListQuestAnswerFragment.kt       | 1 -
 .../westnordost/streetcomplete/quests/LastPickedValuesStore.kt   | 1 -
 3 files changed, 3 deletions(-)

diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
index e3d2a92e51..8649ec1c1f 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
@@ -16,7 +16,6 @@ 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.
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt
index 0031b0e44b..564dd941ab 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt
+++ b/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
 
 /**
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
index ccd439ad77..be07db8525 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
@@ -6,7 +6,6 @@ import androidx.core.content.edit
 import javax.inject.Inject
 
 import de.westnordost.streetcomplete.Prefs
-import kotlin.math.min
 
 class LastPickedValuesStore<T : Any> @Inject constructor(private val prefs: SharedPreferences) {
 

From ac2a881f618c91b52fe820efa59908126656f34e Mon Sep 17 00:00:00 2001
From: Stephen Michel <git@smichel.me>
Date: Tue, 26 Oct 2021 21:15:33 -0400
Subject: [PATCH 11/14] Make favs simpler to use

Set everything up via constructor instead of an ugly `Factory` object.
---
 .../AGroupedImageListQuestAnswerFragment.kt   | 28 ++++++++--------
 .../quests/AImageListQuestAnswerFragment.kt   | 20 ++++++------
 .../quests/LastPickedValuesStore.kt           | 30 ++++++++---------
 .../building_levels/AddBuildingLevelsForm.kt  | 32 +++++++++----------
 4 files changed, 52 insertions(+), 58 deletions(-)

diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
index 8649ec1c1f..3b032598a8 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
@@ -34,17 +34,7 @@ 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<GroupableDisplayItem<I>>
-
-    private val favsFactory = object : LastPickedValuesStore.Factory<GroupableDisplayItem<I>> {
-        private val stringToItem by lazy {
-            allItems.mapNotNull { it.items }.flatten().associateBy { serialize(it) }
-        }
-
-        override val key = javaClass.simpleName
-        override fun serialize(item: GroupableDisplayItem<I>) = item.value.toString()
-        override fun deserialize(value: String) = stringToItem[value]
-    }
+    internal lateinit var favs: LastPickedValuesStore<GroupableDisplayItem<I>>
 
     private val selectedItem get() = imageSelector.selectedItem
 
@@ -52,7 +42,15 @@ abstract class AGroupedImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnsw
 
     override fun onAttach(ctx: Context) {
         super.onAttach(ctx)
-        favs = LastPickedValuesStore(PreferenceManager.getDefaultSharedPreferences(ctx.applicationContext))
+        val stringToItem by lazy {
+            allItems.mapNotNull { it.items }.flatten().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?) {
@@ -101,7 +99,7 @@ abstract class AGroupedImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnsw
     }
 
     private fun getInitialItems(): List<GroupableDisplayItem<I>> {
-        return favs.get(favsFactory).mostCommonWithin(6, 30).padWith(topItems).toList()
+        return favs.get().mostCommonWithin(6, 30).padWith(topItems).toList()
     }
 
     override fun onClickOk() {
@@ -122,14 +120,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(favsFactory, item)
+                            favs.add(item)
                             onClickOk(itemValue)
                         }
                         .show()
                 }
             }
             else {
-                favs.add(favsFactory, item)
+                favs.add(item)
                 onClickOk(itemValue)
             }
         }
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt
index 564dd941ab..a4e8efa231 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt
@@ -44,14 +44,6 @@ abstract class AImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnswerFragm
 
     protected abstract val items: List<DisplayItem<I>>
 
-    private val favsFactory = object : LastPickedValuesStore.Factory<DisplayItem<I>> {
-        private val stringToItem by lazy { items.associateBy { serialize(it) } }
-
-        override val key = javaClass.simpleName
-        override fun serialize(item: DisplayItem<I>) = item.value.toString()
-        override fun deserialize(value: String) = stringToItem[value]
-    }
-
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
         imageSelector = ImageSelectAdapter(maxSelectableItems)
@@ -59,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 by lazy { 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?) {
@@ -96,7 +94,7 @@ abstract class AImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnswerFragm
     override fun onClickOk() {
         val values = imageSelector.selectedItems
         if (values.isNotEmpty()) {
-            favs.add(favsFactory, values)
+            favs.add(values)
             onClickOk(values.map { it.value!! })
         }
     }
@@ -113,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.get(favsFactory).filterNotNull().padWith(originalList).toList()
+            favs.get().filterNotNull().padWith(originalList).toList()
         } else {
             originalList
         }
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
index be07db8525..d343d78fd9 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
@@ -7,29 +7,27 @@ import javax.inject.Inject
 
 import de.westnordost.streetcomplete.Prefs
 
-class LastPickedValuesStore<T : Any> @Inject constructor(private val prefs: SharedPreferences) {
-
-    fun add(factory: Factory<T>, newValues: Iterable<T>) {
-        val lastValues = newValues.asSequence().map(factory::serialize) + getRaw(factory.key)
+class LastPickedValuesStore<T : Any>(
+    private val prefs: SharedPreferences,
+    private val key: String,
+    private val serialize: (T) -> String,
+    private val deserialize: (String) -> T? // null = invalid value
+) {
+    fun add(newValues: Iterable<T>) {
+        val lastValues = newValues.asSequence().map(serialize) + getRaw()
         prefs.edit {
-            putString(getKey(factory.key), lastValues.take(MAX_ENTRIES).joinToString(","))
+            putString(getKey(), lastValues.take(MAX_ENTRIES).joinToString(","))
         }
     }
 
-    fun add(factory: Factory<T>, value: T) = add(factory, listOf(value))
-
-    fun get(factory: Factory<T>): Sequence<T?> = getRaw(factory.key).map(factory::deserialize)
+    fun add(value: T) = add(listOf(value))
 
-    private fun getRaw(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()
 
-    interface Factory<T : Any> {
-        val key: String
-        fun serialize(item: T): String
-        fun deserialize(value: String): T? // null = invalid value
-    }
+    private fun getKey() = Prefs.LAST_PICKED_PREFIX + key
 }
 
 private const val MAX_ENTRIES = 100
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
index b4881e3a61..223116e52d 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
@@ -1,10 +1,12 @@
 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
@@ -32,26 +34,24 @@ class AddBuildingLevelsForm : AbstractQuestFormAnswerFragment<BuildingLevelsAnsw
     private val roofLevels get() = binding.roofLevelsInput.text?.toString().orEmpty().trim()
 
     private val lastPickedAnswers by lazy {
-        favs.get(favsFactory)
+        favs.get()
             .mostCommonWithin(5, 30)
             .sortedWith(compareBy<BuildingLevelsAnswer> { it.levels }.thenBy { it.roofLevels })
             .toList()
     }
 
-    @Inject internal lateinit var favs: LastPickedValuesStore<BuildingLevelsAnswer>
-
-    val favsFactory = object : LastPickedValuesStore.Factory<BuildingLevelsAnswer> {
-        override val key = javaClass.simpleName
-
-        override fun serialize(item: BuildingLevelsAnswer): String =
-            listOfNotNull(item.levels, item.roofLevels).joinToString("#")
-
-        override fun deserialize(value: String) =
-            value.split("#").let { BuildingLevelsAnswer(it[0].toInt(), it.getOrNull(1)?.toInt()) }
-    }
-
-    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?) {
@@ -76,7 +76,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(favsFactory, answer)
+        favs.add(answer)
         applyAnswer(answer)
     }
 

From c0574f4bd2a018c511c326dd2a73375d4e1bceef Mon Sep 17 00:00:00 2001
From: Stephen Michel <git@smichel.me>
Date: Tue, 26 Oct 2021 21:32:59 -0400
Subject: [PATCH 12/14] Less lazy

Using `lazy` here is premature optimization; it makes the code look more
complicated for no reason (unlike the building levels case, where it
delays some i/o until it's needed).
---
 .../quests/AGroupedImageListQuestAnswerFragment.kt           | 5 ++---
 .../streetcomplete/quests/AImageListQuestAnswerFragment.kt   | 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
index 3b032598a8..b54d40bce5 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
@@ -42,9 +42,8 @@ abstract class AGroupedImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnsw
 
     override fun onAttach(ctx: Context) {
         super.onAttach(ctx)
-        val stringToItem by lazy {
-            allItems.mapNotNull { it.items }.flatten().associateBy { it.value.toString() }
-        }
+        val validSuggestions = allItems.mapNotNull { it.items }.flatten()
+        val stringToItem = validSuggestions.associateBy { it.value.toString() }
         favs = LastPickedValuesStore(
             PreferenceManager.getDefaultSharedPreferences(ctx.applicationContext),
             key = javaClass.simpleName,
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt
index a4e8efa231..41b571475a 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AImageListQuestAnswerFragment.kt
@@ -51,7 +51,7 @@ abstract class AImageListQuestAnswerFragment<I,T> : AbstractQuestFormAnswerFragm
 
     override fun onAttach(ctx: Context) {
         super.onAttach(ctx)
-        val stringToItem by lazy { items.associateBy { it.value.toString() } }
+        val stringToItem = items.associateBy { it.value.toString() }
         favs = LastPickedValuesStore(
             PreferenceManager.getDefaultSharedPreferences(ctx.applicationContext),
             key = javaClass.simpleName,

From 5b0b55e9e91eed170d023b0b396e5814514fe632 Mon Sep 17 00:00:00 2001
From: Stephen Michel <git@smichel.me>
Date: Wed, 27 Oct 2021 20:19:41 -0400
Subject: [PATCH 13/14] Clarify purpose of null deserialization result

---
 .../westnordost/streetcomplete/quests/LastPickedValuesStore.kt  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
index d343d78fd9..b12085184a 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
@@ -11,7 +11,7 @@ class LastPickedValuesStore<T : Any>(
     private val prefs: SharedPreferences,
     private val key: String,
     private val serialize: (T) -> String,
-    private val deserialize: (String) -> T? // null = invalid value
+    private val deserialize: (String) -> T? // null = unwanted value, see mostCommonWithin
 ) {
     fun add(newValues: Iterable<T>) {
         val lastValues = newValues.asSequence().map(serialize) + getRaw()

From 777de5e6072791befed3c84888f422ee1b26ab3b Mon Sep 17 00:00:00 2001
From: Stephen Michel <git@smichel.me>
Date: Wed, 27 Oct 2021 20:19:55 -0400
Subject: [PATCH 14/14] Remove unused imports

---
 .../quests/AGroupedImageListQuestAnswerFragment.kt             | 2 --
 .../westnordost/streetcomplete/quests/LastPickedValuesStore.kt | 2 --
 .../quests/building_levels/AddBuildingLevelsForm.kt            | 3 ---
 3 files changed, 7 deletions(-)

diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
index b54d40bce5..59f4197373 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AGroupedImageListQuestAnswerFragment.kt
@@ -9,8 +9,6 @@ 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
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
index b12085184a..08ebcb993a 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/LastPickedValuesStore.kt
@@ -3,8 +3,6 @@ package de.westnordost.streetcomplete.quests
 import android.content.SharedPreferences
 import androidx.core.content.edit
 
-import javax.inject.Inject
-
 import de.westnordost.streetcomplete.Prefs
 
 class LastPickedValuesStore<T : Any>(
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
index 223116e52d..0a6cff2b7a 100644
--- a/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
+++ b/app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
@@ -9,9 +9,6 @@ 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