Merge pull request #179 from conveyal/names-and-stop_time-validator-t…

…weaks

Speed up name validation and fixes shape_dist_traveled check

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<String, Route> 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?
     }

src/main/java/com/conveyal/gtfs/validator/SpeedTripValidator.java

@@ -86,14 +86,19 @@ public void validateTrip(Trip trip, Route route, List<StopTime> 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. 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 (
-            (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);
         }