Sort building type by recently used frequency

[smichel17]

Fixes #1771

[smichel17]

Edit: all issues resolved. Previous contents inside.

This is fully mostly functional, but there's 3 parts that need to be addressed before it is ready for merge.

I was going to "request changes" on my own PR, but… 😡

[smichel17]

I have tested manually, but I think it might be good to add some unit tests for getWeighted before merge, since there are quite a few edge cases which would be easy to screw up by accident.

[smichel17]

Okay, nothing more I want to do here, ready for merge after review.

[matkoniecz]

Is there limit for number of remembered solutions? It would be useful for two reasons:

• if in latest 100 entries 50 was building=farm_auxiliary, 25 was building=garage and 25 was building=house then it would be silly to show building=apartments building=retail building=service building=kiosk building=office just because I normally edit mostly in city and it is in my top lifetime values.
• what happens once user solves 10 000 building quests? Is it going to be laggy and eating even more battery? Or run into some storage size limits

So I think that storing last 100 solutions would be sufficient.

[smichel17]

Is there limit for number of remembered solutions?

Currently 100, see #3373 (comment) and line 24 of the store.

what happens once user solves 10 000 building quests? Is it going to be laggy and eating even more battery?

There is a temptation to obsess over performance here, because it is a problem with lots of opportunities to optimize.
I have been fighting that temptation and focusing on code readability instead, because:

1. There is a limit. We only store 100 entries, and usually only count 30. Nothing here is worse than O(nlogn). Performance will not be an issue.
2. Once this is merged, I'm going to send another PR that switches to sequences instead of LinkedList (WIP here). The main motivations are cleaner code and dropping a java dependency (iOS version #1892), but it will also make the performance impact minimal in all but the most extreme situations.

Okay, but what if…

SharedPreferences are stored as XML files on disk. I wrote a file with RESIDENTIAL, repeated 10k times and it used 118KB of disk space. This might slow down retrieving other SharedPreferences values, but I doubt it. If it's really needed, we could use a different SharedPreferences (different file) for storing last-used values. Minimal impact.

We'd need to read that whole string into memory, so we'd need around 100KB of free RAM. This also seems pretty minimal.

Sequences are lazily evaluated, and splitToSequence uses substrings (based on the original string, no new memory), so there's no additional cost for simply storing this many entries, unless we actually process them for deciding which values to show.

If we actually used all 10k values, then we might want to switch to a more efficient counting implementation, like Kotlin does for eachCount (using boxed integers).

Plus, this is a situation where it's easy to swap out the implementation for something more efficient if needed. So, really, this not something we have to worry about unless we profile and find a problem.

[smichel17]

I'm heading out for a few hours, will make the mentioned changes when I return.

[westnordost]

Nothing to add, looks very good.

[westnordost]

In general, would it make sense to apply this sort order to all quests that have their sort order re-shuffled to have recently selected items first?

[smichel17]

I'm not sure. I think it may be different quest-by-quest, so I'd like to at least do some spot-checks. In any case, I'll go send the follow-up clean-up PR now :)

[westnordost]

Consistency is key

[smichel17]

I have thought most about applying this order to the building levels quest. For it:

• I would like to use this strategy for deciding which answers are shown.
  • Right now, when you encounter one-odd building level (in my neighborhood, that's usually a 3:0 or 1:2), it bumps some more-common value out of your recents. Then, when you go to answer it, it bumps a different more-common value out, instead of the uncommon one. GRR. With this strategy, the uncommon value would be dropped quickly, and I probably wouldn't need to open the keyboard at all.
• I am not sure if I would like to use this order for sorting the answers.
  • Since the levels have a natural sort order, that's pretty convenient for locating the right one to tap. I think with the recently used order, they might shift around too much.
  • But maybe it'll be more stable than I expect, and it'll be okay. I want to test with the building type quest (and possibly make behavior tweaks, like how far back in the history to look; 30 values is arbitrary) before deciding how it should work for the other quests.

[westnordost]

Yeah let's use it for the which but then sort it by how it is currently sorted.

[smichel17]

One thing I noticed while testing with the current implementation is that the history is not changed when you undo an answer. In testing, it's easy to rack up a bunch of answers in your history by doing and un-doing one quest. Not going to worry about it unless it turns out to be a problem.

If you want to include this in the changelog, I suggest something like:

the most-picked of your last 30 answers, always including your latest answer

or

your most-picked recent (30) answers, always including your latest answer

[pkoby]

I'm not sure if this is the right place to comment on this, but I didn't want to open an issue for it.

I like the idea of this change, but in practice I will admit it was confusing (coming from the expectations and experience with the old configuration, especially).

Yesterday, I went to an area with a lot of different building types (many industrial and shops). When I got to an area with just rowhouses (by far the most common type around here) I had to reselect it from the full menu (not a problem), but then on the second building, the recent selection of =terrace was not in the top three (which is all I can see on my phone without scrolling) and kept changing places as I kept mapping.

If the last selection was always at the top, then when you keep selecting the same type, it's always in the same spot (whereas otherwise it might change places as it gets more "popular"). When I'm mapping terraces, there's always going to be a bunch in a row.

So essentially, my preference would be latest in spot # 1 always, then # 2-6 sorted by popularity. Thus if there are a lot of the same type (even if it's a new type), you'll find it at the top, but if you select one of # 2-6, the previous # 1 drops to its spot in recent popularity.

[smichel17]

A new discussion would be ideal.