Refactor property 'stopClusterMode' by making it an Enum and not a St…

…ring. #2558

src/main/java/org/opentripplanner/profile/StopClusterMode.java

@@ -0,0 +1,8 @@
+package org.opentripplanner.profile;
+
+public enum StopClusterMode {
+    /** Group stops by their declared parent station in the GTFS data. */
+    parentStation,
+    /** Cluster stops by proximity and name. */
+    proximity
+}

src/main/java/org/opentripplanner/routing/graph/Graph.java

@@ -39,6 +39,7 @@ the License, or (at your option) any later version.
 import org.opentripplanner.graph_builder.annotation.GraphBuilderAnnotation;
 import org.opentripplanner.graph_builder.annotation.NoFutureDates;
 import org.opentripplanner.model.GraphBundle;
+import org.opentripplanner.profile.StopClusterMode;
 import org.opentripplanner.routing.alertpatch.AlertPatch;
 import org.opentripplanner.routing.core.MortonVertexComparatorFactory;
 import org.opentripplanner.routing.core.TransferTable;
@@ -52,7 +53,6 @@ the License, or (at your option) any later version.
 import org.opentripplanner.routing.services.notes.StreetNotesService;
 import org.opentripplanner.routing.trippattern.Deduplicator;
 import org.opentripplanner.routing.vertextype.PatternArriveVertex;
-import org.opentripplanner.routing.vertextype.TransitStation;
 import org.opentripplanner.routing.vertextype.TransitStop;
 import org.opentripplanner.traffic.StreetSpeedSnapshotSource;
 import org.opentripplanner.updater.GraphUpdaterConfigurator;
@@ -204,7 +204,7 @@ public class Graph implements Serializable {
     public transient StreetSpeedSnapshotSource streetSpeedSource;
 
     /** How should we cluster stops? By 'proximity' or 'ParentStation' */
-    public String stopClusterMode = "proximity";
+    public StopClusterMode stopClusterMode = StopClusterMode.proximity;
 
     /** The difference in meters between the WGS84 ellipsoid height and geoid height at the graph's center */
     public Double ellipsoidToGeoidDifference = 0.0;

src/main/java/org/opentripplanner/routing/graph/GraphIndex.java

@@ -39,6 +39,7 @@
 import org.opentripplanner.index.model.TripTimeShort;
 import org.opentripplanner.profile.ProfileTransfer;
 import org.opentripplanner.profile.StopCluster;
+import org.opentripplanner.profile.StopClusterMode;
 import org.opentripplanner.profile.StopNameNormalizer;
 import org.opentripplanner.profile.StopTreeCache;
 import org.opentripplanner.routing.algorithm.AStar;
@@ -557,18 +558,9 @@ public Timetable currentUpdatedTimetableForTripPattern (TripPattern tripPattern)
      * Stop clusters can be built in one of two ways, either by geographical proximity and name, or
      * according to a parent/child station topology, if it exists.
      */
-    public void clusterStops() {
-    	if (graph.stopClusterMode != null) {
-            switch (graph.stopClusterMode) {
-                case "parentStation":
-                    clusterByParentStation();
-                    break;
-                case "proximity":
-                    clusterByProximityAndName();
-                    break;
-                default:
-                    clusterByProximityAndName();
-            }
+    private void clusterStops() {
+    	if (graph.stopClusterMode == StopClusterMode.parentStation) {
+            clusterByParentStation();
         } else {
             clusterByProximityAndName();
         }

src/main/java/org/opentripplanner/standalone/GraphBuilderParameters.java

@@ -2,17 +2,16 @@
 
 import org.opentripplanner.graph_builder.module.osm.WayPropertySetSource;
 import org.opentripplanner.graph_builder.services.osm.CustomNamer;
- import org.opentripplanner.routing.graph.GraphIndex;
+import org.opentripplanner.profile.StopClusterMode;
+import org.opentripplanner.routing.graph.GraphIndex;
 import org.opentripplanner.routing.impl.DefaultFareServiceFactory;
 import org.opentripplanner.routing.services.FareServiceFactory;
 
 import com.fasterxml.jackson.databind.JsonNode;
 
-import java.util.Collection;
+import java.util.Arrays;
 import java.util.List;
 
-import static java.util.Arrays.asList;
-
 /**
  * These are parameters that when changed, necessitate a Graph rebuild.
  * They are distinct from the RouterParameters which can be applied to a pre-built graph or on the fly at runtime.
@@ -28,9 +27,6 @@
 public class GraphBuilderParameters {
 
     private static double DEFAULT_SUBWAY_ACCESS_TIME = 2.0; // minutes
-    private static final String CULUSTER_MODE_PARENT_STATION = "parentStation";
-    private static final String CLUSTER_MODE_PROXIMITY = "proximity";
-    private static final List<String> CLUSTER_MODES = asList(CULUSTER_MODE_PARENT_STATION, CLUSTER_MODE_PROXIMITY);
 
     /**
      * Generates nice HTML report of Graph errors/warnings (annotations). They are stored in the same location as the graph.
@@ -71,7 +67,7 @@ public class GraphBuilderParameters {
      * <li>"proximity" See {@link GraphIndex#clusterByProximityAndName()}. This is the default value.</li>
      * </ul>
      */
-    public final String stopClusterMode;
+    public final StopClusterMode stopClusterMode;
 
     /**
      * Minutes necessary to reach stops served by trips on routes of route_type=1 (subway) from the street.
@@ -188,7 +184,7 @@ public GraphBuilderParameters(JsonNode config) {
         useTransfersTxt = config.path("useTransfersTxt").asBoolean(false);
         parentStopLinking = config.path("parentStopLinking").asBoolean(false);
         stationTransfers = config.path("stationTransfers").asBoolean(false);
-        stopClusterMode = valueOf(config, "stopClusterMode", CLUSTER_MODE_PROXIMITY, CLUSTER_MODES);
+        stopClusterMode = enumValueOf(config, "stopClusterMode", StopClusterMode.proximity);
         subwayAccessTime = config.path("subwayAccessTime").asDouble(DEFAULT_SUBWAY_ACCESS_TIME);
         streets = config.path("streets").asBoolean(true);
         embedRouterConfig = config.path("embedRouterConfig").asBoolean(true);
@@ -213,12 +209,16 @@ public GraphBuilderParameters(JsonNode config) {
     }
 
 
-    static String valueOf(JsonNode config, String propertyName, String defaultValue, Collection<String> legalValues) {
-        String value = config.path(propertyName).asText(defaultValue);
-        for (String legalValue : legalValues) {
-            if (value.equals(legalValue)) return value;
+    @SuppressWarnings("unchecked")
+    static <T extends Enum<T>> T enumValueOf(JsonNode config, String propertyName, T defaultValue) {
+        String valueAsString = config.path(propertyName).asText(defaultValue.name());
+        try {
+            return Enum.valueOf((Class<T>) defaultValue.getClass(), valueAsString);
+        }
+        catch (IllegalArgumentException ignore) {
+            List<? extends Enum> legalValues = Arrays.asList(defaultValue.getClass().getEnumConstants());
+            throw new IllegalArgumentException("The graph build parameter " + propertyName
+                    + " value '" + valueAsString + "' is not in legal. Expected one of " + legalValues + ".");
         }
-        throw new IllegalArgumentException("The graph build parameter " + propertyName
-                + " value '" + value + "' is not in legal. Expected one of " + legalValues + ".");
     }
 }

src/test/java/org/opentripplanner/standalone/GraphBuilderParametersTest.java

@@ -5,21 +5,17 @@
 import org.junit.Test;
 
 
-import java.util.List;
-
-import static java.util.Arrays.asList;
-import static org.opentripplanner.standalone.GraphBuilderParameters.valueOf;
+import static org.opentripplanner.standalone.GraphBuilderParameters.enumValueOf;
 import static org.junit.Assert.*;
+import static org.opentripplanner.standalone.GraphBuilderParametersTest.AnEnum.*;
+
 
 public class GraphBuilderParametersTest {
+    enum AnEnum { A, B }
 
     private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
     private static final String KEY = "key";
-    private static final String A = "A";
-    private static final String B = "B";
-    private static final String DEFAULT = B;
-    private static final List<String> A_B = asList(A, B);
-    private static final List<String> B_C = asList(B, "C");
+    private static final AnEnum DEFAULT = B;
 
 
     @Test
@@ -28,8 +24,8 @@ public void testValueOf() throws Exception {
         JsonNode config = readConfig("{ 'key' : 'A' }");
 
         // Then
-        assertEquals("Get existing property", A, valueOf(config, KEY, DEFAULT, A_B));
-        assertEquals("Get default value", DEFAULT, valueOf(config, "missing-key", DEFAULT, B_C));
+        assertEquals("Get existing property", A, enumValueOf(config, KEY, DEFAULT));
+        assertEquals("Get default value", DEFAULT, enumValueOf(config, "missing-key", DEFAULT));
     }
 
     @Test(expected = IllegalArgumentException.class)
@@ -38,7 +34,7 @@ public void testValueOfWithIllegalPropertySet() throws Exception {
         JsonNode config = readConfig("{ 'key' : 'X' }");
 
         // Then expect an error when value 'X' is not in the set of legal values: ['A', 'B']
-        valueOf(config, "key", DEFAULT, A_B);
+        enumValueOf(config, "key", DEFAULT);
     }
 
     private static JsonNode readConfig(String text) throws Exception {