-
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
Accept extended TPEG route types (modes) in API #2390
Conversation
Hi @mg-code, am I right in understanding that this is intended to be used with the extended TPEG route types as described in https://groups.google.com/forum/#!msg/gtfs-changes/keT5rTPS7Y0/Q6jtJ-gnHJ8J ? |
Hi @abyrd, if you mean "Extended GTFS Route Types" then yes. As I see they are same. |
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.
Please add tests.
@@ -786,6 +789,17 @@ public void setTriangleTimeFactor(double triangleTimeFactor) { | |||
bikeWalkingOptions.triangleTimeFactor = triangleTimeFactor; | |||
} | |||
|
|||
public void setRouteTypes(String s) { | |||
if (s == null || s.equals("")) |
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.
In that case shouldn't you clear the existing set?
public void setRouteTypes(String s) { | ||
if (s == null || s.equals("")) | ||
return; | ||
HashSet<String> types = new HashSet<String>(Arrays.asList(s.split(","))); |
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 doesn't need to be a hashset -- you can just use the list.
if (s == null || s.equals("")) | ||
return; | ||
HashSet<String> types = new HashSet<String>(Arrays.asList(s.split(","))); | ||
if (types.size() > 0) { |
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 check will always pass; remove it.
* Check if trip route type matches for this plan. | ||
*/ | ||
public boolean tripRouteTypeMatches(Trip trip) { | ||
return routeTypes == null || routeTypes.size() == 0 || routeTypes.contains(trip.getRoute().getType()); |
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.
I don't care for the double check. Let "null" mean "anything is allowed, and don't worry about the empty set (your code above refuses to allow it to be set).
@@ -294,6 +294,9 @@ public State traverse(State s0, long arrivalTimeAtStop) { | |||
// FIXME this should be done WHILE searching for a trip. | |||
if (options.tripIsBanned(trip)) return null; | |||
|
|||
/* check if route type matches (if requested). */ | |||
if(!options.tripRouteTypeMatches(trip)) return null; |
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.
nit: space after if.
@@ -576,6 +582,9 @@ protected RoutingRequest buildRequest() throws ParameterException { | |||
if (maxHours != null) | |||
request.maxHours = maxHours; | |||
|
|||
if(routeTypes != null) |
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.
nit: space after if
Closing, no response after more than one year. |
Sometimes it is necessary to filter trips by GTFS route type.
We are developing route planner service and we want to allow users to select specific transport types, because people buy monthly tickets for specific transport types.
It becomes problematic because trolleybus and bus services both are under BUS TraverseMode.
With my modifications you can specify GTFS route types using comma separated list: