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

feat(map): create the viewmodel for the list of hike routes #13

Merged
merged 2 commits into from
Oct 7, 2024

Conversation

rBergCode
Copy link
Contributor

Implemented the ListOfHikeRoutesViewModel to later retrieve the hike routes from the repository.

Summary

The objective was to habe a basic view model with hardcoded values in order to create the map screen.
The following were done:

  • Created a ListOfHikeRoutesViewModel with the said view model
  • Created a ListOfHikeRoutesViewModelTest to test it (coverage achieved on the viewmodel: 100%)

For now, the view model will return hike routes as hardcoded Strings ("Route 1", "Route 2", "Route 3"). This will be changed when the HikeRoute model will be created.

@rBergCode rBergCode linked an issue Oct 5, 2024 that may be closed by this pull request
@rBergCode rBergCode self-assigned this Oct 5, 2024
@rBergCode rBergCode added the enhancement New feature or request label Oct 5, 2024
@rBergCode rBergCode requested a review from MaelImhof October 5, 2024 10:50
@rBergCode
Copy link
Contributor Author

The code coverage looks like is not correct. I'm trying to correct it on #14 .

@rBergCode rBergCode force-pushed the feat/map-view-model branch from 174b7c6 to 2d7feac Compare October 6, 2024 07:49
@rBergCode
Copy link
Contributor Author

I rebased the branch from main to get the fix from #14. I then force pushed it to re-run the tests.

Copy link
Contributor

@MaelImhof MaelImhof left a comment

Choose a reason for hiding this comment

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

Nicely done, the 100% code coverage is very satisfying.

Two things that I think still need to be discussed :

  1. It was my understanding that when the user zooms in or out on the map, the routes are filtered based on the areas displayed on the map (to avoid loading all of them at the same time, which is probably not viable). This will be implemented in the model, but the viewmodel should provide a method to set the current displayed area so that the model can know about it and update the list of routes consequently. A parameter type for this could be org.osmdroid.util.BoundingBox, which defines the minimum/maximum latitude/longitude displayed on the map view. For now, the function could not really be tested I guess, there are no assertions to be made without a repository.
  2. We might want to use coroutines to make calls to the model, since it will be an API. Starting the coroutine in the view model instead of the model itself could have the significant advantage of being life-cycle aware, see official docs. You could keep your current methods as-is and make them call private suspend functions that will then call the repository on an IO dispatcher.

Let me know about your thoughts on those subjects or if you need any help !

Implemented the ListOfHikeRoutesViewModel to later retrieve the hike routes from the repository.
Added the setArea method to get the displayed portion of the map. Started to use co-routines for future API calls.
@rBergCode rBergCode force-pushed the feat/map-view-model branch from 2d7feac to 28d6727 Compare October 6, 2024 15:54
Copy link

sonarqubecloud bot commented Oct 6, 2024

@rBergCode rBergCode requested a review from MaelImhof October 6, 2024 16:03
Copy link
Contributor

@MaelImhof MaelImhof left a comment

Choose a reason for hiding this comment

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

Good work, this looks good to me.

We might want to avoid hard-coded dispatchers later on but I'm not sure why this is necessary, hence I would postpone it to when we have to actually test coroutines, aka when the actual model gets implemented.

@MaelImhof
Copy link
Contributor

I mentioned hard-coded dispatchers in the appropriate task so that the assigned person can have a look at it when the time comes.

@rBergCode rBergCode merged commit 50eb6f6 into main Oct 7, 2024
2 checks passed
@rBergCode rBergCode deleted the feat/map-view-model branch October 7, 2024 11:34
@MaelImhof MaelImhof added the sprint 1 Active during the first sprint label Nov 15, 2024
@MaelImhof MaelImhof added this to the Milestone 1 milestone Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request sprint 1 Active during the first sprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement map viewmodel
2 participants