Skip to content

feat(mobile): sort places by distance #17740

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

YarosMallorca
Copy link
Collaborator

@YarosMallorca YarosMallorca commented Apr 21, 2025

Currently blocked by #17882

Description

Added the ability to sort the places by distance (from nearest to furthest) from the user's location.

How Has This Been Tested?

  • When Distance is selected, the places are sorted by distance from the current location (obtained at app start) from nearest to furthest
  • When clicking search, the sorting algorithm is ignored
  • If location is not available at the time of loading the screen, the dropdown doesn't show up
  • The sorting order (A-Z) works in both "Name" and "Distance" sorting modes, but not during search

Screenshot

@YarosMallorca YarosMallorca marked this pull request as ready for review April 21, 2025 16:44
@YarosMallorca
Copy link
Collaborator Author

Not sure if this is an enhancement or a feature, feel free to change it :)

Comment on lines +156 to +173
places.sort((a, b) {
final double distanceA = calculateDistance(
currentLocation!.latitude,
currentLocation!.longitude,
a.latitude,
a.longitude,
);
final double distanceB = calculateDistance(
currentLocation!.latitude,
currentLocation!.longitude,
b.latitude,
b.longitude,
);

return isAscending.value
? distanceA.compareTo(distanceB)
: distanceB.compareTo(distanceA);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind of thing is usually indexed. Can you test how long it takes to brute force it like this with maybe 5000 places?

Comment on lines +170 to +172
return isAscending.value
? distanceA.compareTo(distanceB)
: distanceB.compareTo(distanceA);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's generally better to move this outside of the inner sort function and have separate variants to keep the sort function branchless.

} else {
// Sort places by name
places.sort(
(a, b) => isAscending.value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Comment on lines +47 to +48
latitude: data.exifInfo!.latitude!.toDouble(),
longitude: data.exifInfo!.longitude!.toDouble(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually safe? These values can be null.

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

Successfully merging this pull request may close these issues.

2 participants