Remove RouteMatcher#4821
Conversation
6c18450 to
83957ac
Compare
| private final AllowTransitModeFilter transportModeFilter; | ||
| private final List<FeedScopedId> agencies; | ||
| private final RouteMatcher routes; | ||
| private final List<FeedScopedId> routes; |
There was a problem hiding this comment.
What are the performance implications of using a List rather than a Set, which the original RouteMatcher used?
I think eventually we want to use the same indexing technique that unpreferred routes use, isn't it?
That's out of scope for this PR though.
There was a problem hiding this comment.
With a low number of entries (<~5), the List actually has better performance, since no hashCode needs to be calculated, and the FeedScopedId is easy to compare. We use List here for other things, I'll do some profiling and crate a PR if there is a difference.
leonardehrenfried
left a comment
leonardehrenfried
left a comment
There was a problem hiding this comment.
I'm not sure it will make a big difference but I think we should move to a Set for the collection of route ids.
2541503
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #4821 +/- ##
==========================================
Coverage ? 62.95%
Complexity ? 13248
==========================================
Files ? 1659
Lines ? 66246
Branches ? 7206
==========================================
Hits ? 41705
Misses ? 22161
Partials ? 2380
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
3 participants
Summary
We have previously discussed removal of
RouteMatcher. This does that. This branch is based on #4820Breaking change
This is a breaking change for the users of the REST API and legacy GraphQL API, as we remove the possibility of banning/whitelisting of routes by their name, and only allow filtering by ID.