Some minor cleanups in GraphHopper&RoutingTemplates

Author: easbar

core/src/main/java/com/graphhopper/GraphHopper.java

@@ -29,12 +29,12 @@
 import com.graphhopper.routing.RoutingAlgorithmFactorySimple;
 import com.graphhopper.routing.ch.CHPreparationHandler;
 import com.graphhopper.routing.ch.CHRoutingAlgorithmFactory;
-import com.graphhopper.routing.lm.LMConfig;
-import com.graphhopper.routing.lm.LMPreparationHandler;
 import com.graphhopper.routing.ev.DefaultEncodedValueFactory;
 import com.graphhopper.routing.ev.EncodedValueFactory;
 import com.graphhopper.routing.ev.EnumEncodedValue;
 import com.graphhopper.routing.ev.RoadEnvironment;
+import com.graphhopper.routing.lm.LMConfig;
+import com.graphhopper.routing.lm.LMPreparationHandler;
 import com.graphhopper.routing.querygraph.QueryGraph;
 import com.graphhopper.routing.subnetwork.PrepareRoutingSubnetworks;
 import com.graphhopper.routing.template.AlternativeRoutingTemplate;
@@ -1045,68 +1045,57 @@ public List<Path> calcPaths(GHRequest request, GHResponse ghRsp) {
 
         try {
             validateRequest(request);
-            PMap hints = request.getHints();
             final boolean disableCH = getDisableCH(request.getHints());
             final boolean disableLM = getDisableLM(request.getHints());
-            String algoStr = request.getAlgorithm();
-            if (algoStr.isEmpty())
-                algoStr = chPreparationHandler.isEnabled() && !disableCH ? DIJKSTRA_BI : ASTAR_BI;
-
             Profile profile = profilesByName.get(request.getProfile());
-            if (profile == null) {
+            if (profile == null)
                 throw new IllegalArgumentException("The requested profile '" + request.getProfile() + "' does not exist.\nAvailable profiles: " + profilesByName.keySet());
-            }
-            if (!profile.isTurnCosts() && !request.getCurbsides().isEmpty()) {
-                List<String> turnCostProfiles = new ArrayList<>();
-                for (Profile p : profilesByName.values()) {
-                    if (p.isTurnCosts()) {
-                        turnCostProfiles.add(p.getName());
-                    }
-                }
+            if (!profile.isTurnCosts() && !request.getCurbsides().isEmpty())
                 throw new IllegalArgumentException("To make use of the " + CURBSIDE + " parameter you need to use a profile that supports turn costs" +
-                        "\nThe following profiles do support turn costs: " + turnCostProfiles);
-            }
+                        "\nThe following profiles do support turn costs: " + getTurnCostProfiles());
 
             // todo later: should we be able to control this using the edge_based parameter?
             TraversalMode tMode = profile.isTurnCosts() ? TraversalMode.EDGE_BASED : TraversalMode.NODE_BASED;
-            List<GHPoint> points = request.getPoints();
             RoutingAlgorithmFactory algorithmFactory = getAlgorithmFactory(profile.getName(), disableCH, disableLM);
             Weighting weighting;
             Graph graph = ghStorage;
             if (chPreparationHandler.isEnabled() && !disableCH) {
                 if (!(algorithmFactory instanceof CHRoutingAlgorithmFactory))
                     throw new IllegalStateException("Although CH was enabled a non-CH algorithm factory was returned " + algorithmFactory);
 
-                if (hints.has(Routing.BLOCK_AREA))
+                if (request.getHints().has(Routing.BLOCK_AREA))
                     throw new IllegalArgumentException("When CH is enabled the " + Parameters.Routing.BLOCK_AREA + " cannot be specified");
 
                 CHConfig chConfig = ((CHRoutingAlgorithmFactory) algorithmFactory).getCHConfig();
                 weighting = chConfig.getWeighting();
                 graph = ghStorage.getCHGraph(chConfig);
             } else {
-                checkNonChMaxWaypointDistance(points);
-                final int uTurnCostsInt = hints.getInt(Routing.U_TURN_COSTS, INFINITE_U_TURN_COSTS);
+                checkNonChMaxWaypointDistance(request.getPoints());
+                final int uTurnCostsInt = request.getHints().getInt(Routing.U_TURN_COSTS, INFINITE_U_TURN_COSTS);
                 if (uTurnCostsInt != INFINITE_U_TURN_COSTS && !tMode.isEdgeBased()) {
                     throw new IllegalArgumentException("Finite u-turn costs can only be used for edge-based routing, use `" + Routing.EDGE_BASED + "=true'");
                 }
                 FlagEncoder encoder = encodingManager.getEncoder(profile.getVehicle());
-                weighting = createWeighting(profile, hints);
-                if (hints.has(Routing.BLOCK_AREA))
+                weighting = createWeighting(profile, request.getHints());
+                if (request.getHints().has(Routing.BLOCK_AREA))
                     weighting = new BlockAreaWeighting(weighting, GraphEdgeIdFinder.createBlockArea(ghStorage, locationIndex,
-                            points, hints, DefaultEdgeFilter.allEdges(encoder)));
+                            request.getPoints(), request.getHints(), DefaultEdgeFilter.allEdges(encoder)));
             }
             ghRsp.addDebugInfo("tmode:" + tMode.toString());
 
+            String algoStr = request.getAlgorithm();
+            if (algoStr.isEmpty())
+                algoStr = chPreparationHandler.isEnabled() && !disableCH ? DIJKSTRA_BI : ASTAR_BI;
             RoutingTemplate routingTemplate = createRoutingTemplate(request, ghRsp, algoStr, weighting);
 
             StopWatch sw = new StopWatch().start();
-            List<QueryResult> qResults = routingTemplate.lookup(points);
+            List<QueryResult> qResults = routingTemplate.lookup(request.getPoints());
             ghRsp.addDebugInfo("idLookup:" + sw.stop().getSeconds() + "s");
             if (ghRsp.hasErrors())
                 return Collections.emptyList();
 
             QueryGraph queryGraph = QueryGraph.create(graph, qResults);
-            int maxVisitedNodesForRequest = hints.getInt(Routing.MAX_VISITED_NODES, routingConfig.getMaxVisitedNodes());
+            int maxVisitedNodesForRequest = request.getHints().getInt(Routing.MAX_VISITED_NODES, routingConfig.getMaxVisitedNodes());
             if (maxVisitedNodesForRequest > routingConfig.getMaxVisitedNodes())
                 throw new IllegalArgumentException("The max_visited_nodes parameter has to be below or equal to:" + routingConfig.getMaxVisitedNodes());
 
@@ -1115,15 +1104,15 @@ public List<Path> calcPaths(GHRequest request, GHResponse ghRsp) {
                     traversalMode(tMode).
                     weighting(weighting).
                     maxVisitedNodes(maxVisitedNodesForRequest).
-                    hints(hints).
+                    hints(request.getHints()).
                     build();
 
             // do the actual route calculation !
             List<Path> altPaths = routingTemplate.calcPaths(queryGraph, algorithmFactory, algoOpts);
 
-            boolean tmpEnableInstructions = hints.getBool(Routing.INSTRUCTIONS, encodingManager.isEnableInstructions());
-            boolean tmpCalcPoints = hints.getBool(Routing.CALC_POINTS, routingConfig.isCalcPoints());
-            double wayPointMaxDistance = hints.getDouble(Routing.WAY_POINT_MAX_DISTANCE, 1d);
+            boolean tmpEnableInstructions = request.getHints().getBool(Routing.INSTRUCTIONS, encodingManager.isEnableInstructions());
+            boolean tmpCalcPoints = request.getHints().getBool(Routing.CALC_POINTS, routingConfig.isCalcPoints());
+            double wayPointMaxDistance = request.getHints().getDouble(Routing.WAY_POINT_MAX_DISTANCE, 1d);
 
             DouglasPeucker peucker = new DouglasPeucker().setMaxDistance(wayPointMaxDistance);
             PathMerger pathMerger = new PathMerger(queryGraph.getBaseGraph(), weighting).
@@ -1144,6 +1133,16 @@ public List<Path> calcPaths(GHRequest request, GHResponse ghRsp) {
         }
     }
 
+    private List<String> getTurnCostProfiles() {
+        List<String> turnCostProfiles = new ArrayList<>();
+        for (Profile p : profilesByName.values()) {
+            if (p.isTurnCosts()) {
+                turnCostProfiles.add(p.getName());
+            }
+        }
+        return turnCostProfiles;
+    }
+
     protected void validateRequest(GHRequest request) {
         if (Helper.isEmpty(request.getProfile()))
             throw new IllegalArgumentException("You need to specify a profile to perform a routing request, see docs/core/profiles.md");

core/src/main/java/com/graphhopper/routing/template/AlternativeRoutingTemplate.java

@@ -74,9 +74,9 @@ public void finish(PathMerger pathMerger, Translation tr) {
 
         // if alternative route calculation was done then create the responses from single paths        
         PointList wpList = getWaypoints();
-        altResponse.setWaypoints(wpList);
-        ghResponse.add(altResponse);
-        pathMerger.doWork(altResponse, Collections.singletonList(pathList.get(0)), lookup, tr);
+        pathWrapper.setWaypoints(wpList);
+        ghResponse.add(pathWrapper);
+        pathMerger.doWork(pathWrapper, Collections.singletonList(pathList.get(0)), lookup, tr);
         for (int index = 1; index < pathList.size(); index++) {
             PathWrapper tmpAltRsp = new PathWrapper();
             tmpAltRsp.setWaypoints(wpList);

core/src/main/java/com/graphhopper/routing/template/ViaRoutingTemplate.java

@@ -56,7 +56,7 @@ public class ViaRoutingTemplate extends AbstractRoutingTemplate implements Routi
     protected final GHResponse ghResponse;
     // result from route
     protected List<Path> pathList;
-    protected final PathWrapper altResponse = new PathWrapper();
+    protected final PathWrapper pathWrapper = new PathWrapper();
     private final EnumEncodedValue<RoadClass> roadClassEnc;
     private final EnumEncodedValue<RoadEnvironment> roadEnvEnc;
 
@@ -113,7 +113,6 @@ public List<Path> calcPaths(QueryGraph queryGraph, RoutingAlgorithmFactory algoF
             }
         }
 
-        final boolean forceCurbsides = ghRequest.getHints().getBool(Routing.FORCE_CURBSIDE, true);
         QueryResult fromQResult = queryResults.get(0);
         StopWatch sw;
         for (int placeIndex = 1; placeIndex < pointsCount; placeIndex++) {
@@ -155,6 +154,7 @@ public List<Path> calcPaths(QueryGraph queryGraph, RoutingAlgorithmFactory algoF
                 final String toCurbside = ghRequest.getCurbsides().get(placeIndex);
                 int sourceOutEdge = DirectionResolverResult.getOutEdge(directions.get(placeIndex - 1), fromCurbside);
                 int targetInEdge = DirectionResolverResult.getInEdge(directions.get(placeIndex), toCurbside);
+                final boolean forceCurbsides = ghRequest.getHints().getBool(Routing.FORCE_CURBSIDE, true);
                 sourceOutEdge = ignoreThrowOrAcceptImpossibleCurbsides(sourceOutEdge, placeIndex - 1, forceCurbsides);
                 targetInEdge = ignoreThrowOrAcceptImpossibleCurbsides(targetInEdge, placeIndex, forceCurbsides);
 
@@ -183,17 +183,18 @@ public List<Path> calcPaths(QueryGraph queryGraph, RoutingAlgorithmFactory algoF
             if (tmpPathList.isEmpty())
                 throw new IllegalStateException("At least one path has to be returned for " + fromQResult + " -> " + toQResult);
 
-            int idx = 0;
-            for (Path path : tmpPathList) {
+            // todo: can tmpPathList ever have more than one path here? and would it even be correct to add them all
+            // to pathList then?
+            for (int i = 0; i < tmpPathList.size(); i++) {
+                Path path = tmpPathList.get(i);
                 if (path.getTime() < 0)
-                    throw new RuntimeException("Time was negative " + path.getTime() + " for index " + idx + ". Please report as bug and include:" + ghRequest);
+                    throw new RuntimeException("Time was negative " + path.getTime() + " for index " + i + ". Please report as bug and include:" + ghRequest);
 
                 pathList.add(path);
                 debug += ", " + path.getDebugInfo();
-                idx++;
             }
 
-            altResponse.addDebugInfo(debug);
+            pathWrapper.addDebugInfo(debug);
 
             // reset all direction enforcements in queryGraph to avoid influencing next path
             queryGraph.clearUnfavoredStatus();
@@ -202,7 +203,7 @@ public List<Path> calcPaths(QueryGraph queryGraph, RoutingAlgorithmFactory algoF
                 throw new IllegalArgumentException("No path found due to maximum nodes exceeded " + algoOpts.getMaxVisitedNodes());
 
             visitedNodesSum += algo.getVisitedNodes();
-            altResponse.addDebugInfo("visited nodes sum: " + visitedNodesSum);
+            pathWrapper.addDebugInfo("visited nodes sum: " + visitedNodesSum);
             fromQResult = toQResult;
         }
 
@@ -232,9 +233,9 @@ public void finish(PathMerger pathMerger, Translation tr) {
         if (ghRequest.getPoints().size() - 1 != pathList.size())
             throw new RuntimeException("There should be exactly one more points than paths. points:" + ghRequest.getPoints().size() + ", paths:" + pathList.size());
 
-        altResponse.setWaypoints(getWaypoints());
-        ghResponse.add(altResponse);
-        pathMerger.doWork(altResponse, pathList, lookup, tr);
+        pathWrapper.setWaypoints(getWaypoints());
+        ghResponse.add(pathWrapper);
+        pathMerger.doWork(pathWrapper, pathList, lookup, tr);
     }
 
 }