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

Improve the monitoring of step progress #20

Closed
cammace opened this issue Apr 25, 2017 · 5 comments
Closed

Improve the monitoring of step progress #20

cammace opened this issue Apr 25, 2017 · 5 comments
Assignees
Milestone

Comments

@cammace
Copy link
Contributor

cammace commented Apr 25, 2017

Breakup monitorStepProgress method into smaller, more testable, methods and easier readability. Currently it is a direct port from the Swift function found here. Tagging @bsudekum and @1ec5 since this could potentially break parity with the iOS SDK.

cc: @zugaldia @danesfeder

@1ec5
Copy link
Contributor

1ec5 commented Apr 25, 2017

Sounds great. It’s a sprawling method that should be refactored on the Swift side too.

@cammace cammace added this to the v0.2.0 milestone Apr 28, 2017
@cammace cammace self-assigned this May 1, 2017
@cammace
Copy link
Contributor Author

cammace commented May 1, 2017

I've been looking over best solutions for splitting up the monitorStepProgress, here are the steps I'm thinking of taking to refactor it.

  • Move logic into it's own class and get rid of LocationUpdateThread (see Background Thread #19).
  • Have routeProgress handle more logic, currently in monitorStepProgress() we are doing conversions and calculations that seem fit for handling inside the routeProgress model instead such as calculating the users location truePosition, secondsToEndOfStep, etc. This would require building a new routeProgress object before determining what alert level the users on and invoking the callbacks appropriately.
  • Each alert level check moved into separate methods. This can help remove unnecessary checks such as whether the user just departed or not, which only needs to be called once. This also makes testing of each method significantly easier.

@cammace
Copy link
Contributor Author

cammace commented May 15, 2017

Refactoring of this took place in #30. Closing.

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

No branches or pull requests

3 participants
@1ec5 @cammace and others