-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Refactor NearbyStopsFinder to follow strategy pattern #5906
Refactor NearbyStopsFinder to follow strategy pattern #5906
Conversation
This commit should only change structure. No behaviour should be affected.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #5906 +/- ##
=============================================
- Coverage 69.45% 69.45% -0.01%
- Complexity 17064 17072 +8
=============================================
Files 1927 1931 +4
Lines 73578 73587 +9
Branches 7549 7550 +1
=============================================
+ Hits 51106 51109 +3
- Misses 19847 19853 +6
Partials 2625 2625 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good refactoring. I look forward to the tests.
src/main/java/org/opentripplanner/graph_builder/module/nearbystops/NearbyStopFinder.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one JavaDoc need to be updated.
Co-authored-by: Thomas Gran <[email protected]>
Summary
This PR is a pure refactor and shouldn't change any behaviour.
The NearbyStopsFinder has three different behaviours depending on two different parameters:
ConsiderPatternsForDirectTransfers
feature is enabled.The different branches makes the code difficult to understand and difficult to test. This PR refactors this code to use the strategy pattern by extracting a common interface and having one class for each behaviour.
This work is in preparation of some work I need to do on access/egress search as part of #5848.
Unit tests
Unfortunately there is no tests for this piece of code. I would like to add some in a follow-up PR.
Documentation
I added and updated javadoc.
Bumping the serialization version id
Nope