From 5229d6c2a95553c7cdaa2b301fb43e988c75f0f4 Mon Sep 17 00:00:00 2001 From: Landon Reed Date: Fri, 4 Jan 2019 15:33:22 -0500 Subject: [PATCH 1/2] fix(validation): speeds up name validation and fixes shape_dist check fixes #178 --- .../gtfs/validator/NamesValidator.java | 31 ++++++++++++------- .../gtfs/validator/SpeedTripValidator.java | 13 +++++--- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/validator/NamesValidator.java b/src/main/java/com/conveyal/gtfs/validator/NamesValidator.java index 889a3679b..b417854ff 100644 --- a/src/main/java/com/conveyal/gtfs/validator/NamesValidator.java +++ b/src/main/java/com/conveyal/gtfs/validator/NamesValidator.java @@ -6,6 +6,9 @@ import com.conveyal.gtfs.model.Stop; import com.conveyal.gtfs.model.Trip; +import java.util.HashMap; +import java.util.Map; + import static com.conveyal.gtfs.error.NewGTFSErrorType.*; public class NamesValidator extends FeedValidator { @@ -56,26 +59,30 @@ public void validate() { registerError(stop, STOP_DESCRIPTION_SAME_AS_NAME, desc); } } - // Check trips + // Place routes into a map for quick access while validating trip names. + Map routesForId = new HashMap<>(); + for (Route route : feed.routes) { + routesForId.put(route.route_id, route); + } + // Check trip names (headsigns and TODO short names) for (Trip trip : feed.trips) { String headsign = normalize(trip.trip_headsign); + // Trip headsign should not begin with "to" or "towards" (note: headsign normalized to lowercase). Headsigns + // should follow one of the patterns defined in the best practices: http://gtfs.org/best-practices#tripstxt + if (headsign.startsWith("to ") || headsign.startsWith("towards ")) { + registerError(trip, TRIP_HEADSIGN_SHOULD_DESCRIBE_DESTINATION_OR_WAYPOINTS, headsign); + } // TODO: check trip short name? // String shortName = normalize(trip.trip_short_name); - Route route = feed.routes.get(trip.route_id); - String routeShortName = "", routeLongName = ""; - if (route != null) { - routeShortName = normalize(route.route_short_name); - routeLongName = normalize(route.route_long_name); - } + Route route = routesForId.get(trip.route_id); + // Skip route name/headsign check if the trip has a bad reference to its route. + if (route == null) continue; + String routeShortName = normalize(route.route_short_name); + String routeLongName = normalize(route.route_long_name); // Trip headsign should not duplicate route name. if (!headsign.isEmpty() && (headsign.contains(routeShortName) || headsign.contains(routeLongName))) { registerError(trip, TRIP_HEADSIGN_CONTAINS_ROUTE_NAME, headsign); } - // Trip headsign should not begin with "to" or "towards" (note: headsign normalized to lowercase). Headsigns - // should follow one of the patterns defined in the best practices: http://gtfs.org/best-practices#tripstxt - if (headsign.startsWith("to ") || headsign.startsWith("towards ")) { - registerError(trip, TRIP_HEADSIGN_SHOULD_DESCRIBE_DESTINATION_OR_WAYPOINTS, headsign); - } } // TODO Are there other tables we're not checking? } diff --git a/src/main/java/com/conveyal/gtfs/validator/SpeedTripValidator.java b/src/main/java/com/conveyal/gtfs/validator/SpeedTripValidator.java index 6a89163ae..c87e81f00 100644 --- a/src/main/java/com/conveyal/gtfs/validator/SpeedTripValidator.java +++ b/src/main/java/com/conveyal/gtfs/validator/SpeedTripValidator.java @@ -86,14 +86,17 @@ public void validateTrip(Trip trip, Route route, List stopTimes, List< } /** - * Register shape dist traveled error if previous stop time's value was missing and the current value is - * not (if at least one stop time has a value, all stop times for the trip should) OR if current value - * is not greater than previous value. + * Register shape dist traveled error if current stop time has a value AND either and the previous value is + * missing (if at least one stop time has a value, all stop times for the trip should) OR if current value + * is less than or equal to the previous value. */ private void checkShapeDistTraveled(StopTime previous, StopTime current) { if ( - (previous.shape_dist_traveled == Entity.DOUBLE_MISSING && current.shape_dist_traveled != Entity.DOUBLE_MISSING) || - current.shape_dist_traveled <= previous.shape_dist_traveled + current.shape_dist_traveled != Entity.DOUBLE_MISSING && + ( + previous.shape_dist_traveled == Entity.DOUBLE_MISSING || + current.shape_dist_traveled <= previous.shape_dist_traveled + ) ) { registerError(current, SHAPE_DIST_TRAVELED_NOT_INCREASING, current.shape_dist_traveled); } From 2fbbda033e2fd851a171ff85134a1819d4aca1dd Mon Sep 17 00:00:00 2001 From: Landon Reed Date: Fri, 4 Jan 2019 15:39:33 -0500 Subject: [PATCH 2/2] style(validator): improve javadoc --- .../java/com/conveyal/gtfs/validator/SpeedTripValidator.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/gtfs/validator/SpeedTripValidator.java b/src/main/java/com/conveyal/gtfs/validator/SpeedTripValidator.java index c87e81f00..96170bbff 100644 --- a/src/main/java/com/conveyal/gtfs/validator/SpeedTripValidator.java +++ b/src/main/java/com/conveyal/gtfs/validator/SpeedTripValidator.java @@ -88,7 +88,9 @@ public void validateTrip(Trip trip, Route route, List stopTimes, List< /** * Register shape dist traveled error if current stop time has a value AND either and the previous value is * missing (if at least one stop time has a value, all stop times for the trip should) OR if current value - * is less than or equal to the previous value. + * is less than or equal to the previous value. Note: if the previous shape_dist_traveled value is present and the + * current value is missing, the previous value will be greater than the current stop time's value because + * {@link Entity#DOUBLE_MISSING} is the lowest possible double value. This in turn will register an error. */ private void checkShapeDistTraveled(StopTime previous, StopTime current) { if (