Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Smart sort order for recent houses quest #1771

Closed
smichel17 opened this issue Apr 8, 2020 · 10 comments · Fixed by #3373
Closed

Smart sort order for recent houses quest #1771

smichel17 opened this issue Apr 8, 2020 · 10 comments · Fixed by #3373
Assignees

Comments

@smichel17
Copy link
Member

smichel17 commented Apr 8, 2020

Proposed Solution

In #1692 (comment), @westnordost proposed sorting like this:

Per session, count the number of times a certain option has been selected. Then, each time the dialog is opened, the first row of items are sorted by the previous selection count.

Then, for persisting, the collected data is reduced like this: Let's say the user selected 27x gabled, 10x hipped, 3x dome and 1xround. Only the order is saved, so gabled, hipped, dome, round. When the data is reloaded into memory (=new app start), it is interpreted like this: 4x gabled, 3x hipped, 2x dome, 1x round. So, the ordering is still the same, but will be changed easier if the user selects different roof sizes where he is now.

I'd love to see that applied to house type.

Use case

I've been mapping a lot of residential areas lately. There is a very common ratio of building types (detached house > shed > garage), so the sort order would remain stable, which would make selection much faster since I wouldn't have to verify the order before tapping. At the same time, I wouldn't want this to carry over between sessions, if I'm doing a different area later, so the storage method also works well.

Currently my workaround to guarantee a stable sort order as often as possible is:

  • While walking slowly down a street…
  • Select the house type for 4 detached houses (two on either side of the street, in front of me).
    • For 3 of these selections, detached house is always first, I don't have to look.
  • Select the number of levels for all 4 houses. This also allows me to re-use the last-picked height when possible (in lieu of Make it faster to answer the building levels quest #1772).
  • Select the roof type for the houses, if applicable.
  • Select the building type for any sheds or garages next to the houses

This works pretty well, but is not always possible depending on the layout of the street. It's also barely fast enough to keep up with my (intentionally slow) walking pace, so occasionally I need to stop and finish filling out quests. So, this issue would be a good QoL improvement.

@westnordost
Copy link
Member

Well the feature would be implemented for any quest type of course. I started implementation a few months ago but realized it is not as trivial as it sounded, so I discarded it for sometime later.

@peternewman
Copy link
Collaborator

* Select the number of levels for all 4 houses. This also allows me to re-use the last-picked height when possible (in lieu of #1772).

How does the re-using the last-picked height work?

This idea might go slightly awry for streets with a mix of semi-detached and terrace, or detached and semi, as the order might keep swapping.

@smichel17
Copy link
Member Author

How does the re-using the last-picked height work?

If there are 3 single-story buildings and one two-story building, I do the two story building either first or last.

This idea might go slightly awry for streets with a mix […] the order might keep swapping.

Currently the order swaps every time you pick a new building type, so this is still an improvement on the status quo, even if it's not a perfect solution.

@peternewman
Copy link
Collaborator

How does the re-using the last-picked height work?

If there are 3 single-story buildings and one two-story building, I do the two story building either first or last.

Does it remember the last values you used then? I've never seen or noticed that?

@matkoniecz
Copy link
Member

matkoniecz commented Oct 10, 2021

so the sort order would remain stable, which would make selection much faster since I wouldn't have to verify the order before tapping

Would it be enough to still display N-last selected items, but in stable way?

For example if last-selected-building has space for 6 entries and user selected

  • detached house
  • shed
  • garage
  • carport
  • detached house
  • detached house
  • shop building
  • building constructed to be a hotel (filling 6 items)
  • church building

Now something needs to be dropped. Instead of putting church on the first position, it would be placed in the second replacing shed.

Not tried implementing it, but maybe it would make things easier.

@smichel17
Copy link
Member Author

smichel17 commented Oct 10, 2021

I like the original idea better, but this idea should be easier to implement — by sorting alphabetically if nothing else. So, perhaps worth trying. I agree the unstable sort order is annoying.

Separately:

I think an improvement on the original idea would be: instead of keeping data for the whole session and resetting it when the session ends, just keep data for the last N (say, 20) answers, and show the most common 6 from that set. I think that makes more sense, since it's possible you might walk between two neighborhoods and the common building types change. From what I remember in #3117, this should be easier to implement, as well.

@smichel17
Copy link
Member Author

I started to look into this. It looks like some of the extra effort comes from having several layers of abstraction to dance through.

As far as I can tell, AGroupedImageListQuestAnswerFragment is only implemented by AddBuildingTypeForm. Did it previously have other classes implementing it, or has it always been this way (planned to add more later)? @westnordost How would you feel about getting rid of the abstract class?

Also, this quest seems to be the only place that calls favs::moveLastPickedDisplayItemsToFront, so I think it would be better to move it (an extension function) into the file where it's used.

@westnordost
Copy link
Member

The surface form used to be grouped, too.

@westnordost How would you feel about getting rid of the abstract class?

Also, this quest seems to be the only place that calls favs::moveLastPickedDisplayItemsToFront, so I think it would be better to move it (an extension function) into the file where it's used.

No and no.

@smichel17

This comment has been minimized.

@smichel17
Copy link
Member Author

Sorry, I did a real silly, and missed that I should have been looking at moveLastPickedGroupableDisplayItemToFront, not moveLastPickedDisplayItemsToFront (which are identical, except that the groupable variant has different helper functions, which do indeed filter out the groups. Filtering to all of the more specific values is a bit of a roundabout way to filter out all of the categories, but I suppose it's more reliable in case the answers ever change, so you don't have to scrub the history when the app is updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants