Sort building levels recents by regular, then roof #3117
Conversation
app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/de/westnordost/streetcomplete/quests/building_levels/AddBuildingLevelsForm.kt
Show resolved
Hide resolved
|
Did a little mapping this morning. Unfortunately, the neighborhood I walked through had only 1, 1|1, and 2 story houses. So while this did feel nicer, it also didn't feel like much of a stress test. Anyone in a more varied area want to take it for a spin? I can share a debug apk if needed. |
|
@matkoniecz 👀 They are still added and removed in the same order, just the display order is sorted instead of chronological. So it has the disadvantage that you can't be precisely sure which one will disappear next time, when you enter a new answer. In practice, I think it doesn't matter that much. |
smichel17
force-pushed
the
sort-building-levels-recents
branch
from
a41a842
to
f3f7551
Compare
The new building levels recent values is great. However, I've found that the vast majority of the buildings in the areas I map are 4 values (1|0, 1|1, 2|0, and 2|1), so my recent values seldom change. As a result, a stabler sort order is much easier for me to scan and find the entry I'm looking for than the current FIFO queue, where they constantly re-order. They are still added and removed in the same order; only the display order is changed. So it has the drawback that you can't be precisely sure which entry will disappear, after you add a new answer. In practice, I think it doesn't matter that much. Originally I tried sorting by total levels first, but when testing, I found it harder to parse "1|1, 2|0, 1|2, 2|1" than "1|1, 1|2, 2|0, 2|1". Unfortunately, the compiler can't figure out the type of `thenBy` without the annotation.
smichel17
force-pushed
the
sort-building-levels-recents
branch
from
f3f7551
to
c51ffd8
Compare
|
Updated formatting and added all the commentary here to the commit message. So aside from maybe waiting on testing from people in a different area, I consider this ready for merge. |
|
I also think the same. I'll merge this before the release of the next version but I'll wait because you said that you wanted to do some field testing. |
|
I've done enough field testing now that I'm confident it works the way I'd like. But merge conflicts are extremely unlikely, so I'm happy to wait to merge. |
The new building levels recent values is great. However, I've found that the vast majority of the buildings in the areas I map are 5 values: 1/0, 1/1, 2/0, 2/1, and -- probably to the surprise of no one -- 3/0. With that in mind, a stabler sort order would be much easier for me to scan and find the entry I'm looking for than the current FIFO queue.
I haven't tried this yet, going to do it this morning.Tried it, works as intended.