Remove GHRequest#hasXYZ and various clean-ups

Author: easbar

api/src/main/java/com/graphhopper/GHRequest.java

@@ -217,10 +217,6 @@ public List<String> getPointHints() {
         return pointHints;
     }
 
-    public boolean hasPointHints() {
-        return pointHints.size() == points.size() && !points.isEmpty();
-    }
-
     public GHRequest setCurbsides(List<String> curbsides) {
         this.curbsides = curbsides;
         return this;
@@ -230,19 +226,11 @@ public List<String> getCurbsides() {
         return curbsides;
     }
 
-    public boolean hasCurbsides() {
-        return curbsides.size() == points.size() && !points.isEmpty();
-    }
-
     public GHRequest setSnapPreventions(List<String> snapPreventions) {
         this.snapPreventions = snapPreventions;
         return this;
     }
 
-    public boolean hasSnapPreventions() {
-        return !snapPreventions.isEmpty();
-    }
-
     public List<String> getSnapPreventions() {
         return snapPreventions;
     }

api/src/main/java/com/graphhopper/util/Parameters.java

@@ -91,6 +91,7 @@ public static final class RoundTrip {
      * Parameters that can be passed as hints and influence routing per request.
      */
     public static final class Routing {
+        public static final String ALGORITHM = "algorithm";
         public static final String EDGE_BASED = "edge_based";
         public static final String TURN_COSTS = "turn_costs";
         public static final String U_TURN_COSTS = "u_turn_costs";

api/src/main/java/com/graphhopper/util/exceptions/PointNotFoundException.java

@@ -30,8 +30,8 @@ public class PointNotFoundException extends IllegalArgumentException implements
 
     protected final int pointIndex;
 
-    public PointNotFoundException(String var1, int pointIndex) {
-        super(var1);
+    public PointNotFoundException(String message, int pointIndex) {
+        super(message);
         this.pointIndex = pointIndex;
     }
 

client-hc/src/main/java/com/graphhopper/api/GHMRequest.java

@@ -120,12 +120,6 @@ public List<String> getPointHints() {
         throw new IllegalStateException("Use getFromPointHints or getToPointHints");
     }
 
-    @Override
-    public boolean hasPointHints() {
-        return this.fromPointHints.size() == this.fromPoints.size() && !fromPoints.isEmpty() &&
-                this.toPointHints.size() == this.toPoints.size() && !toPoints.isEmpty();
-    }
-
     public GHRequest addFromPointHint(String pointHint) {
         this.fromPointHints.add(pointHint);
         return this;
@@ -196,12 +190,6 @@ public List<String> getCurbsides() {
         throw new IllegalStateException("Use getFromCurbsides or getToCurbsides");
     }
 
-    @Override
-    public boolean hasCurbsides() {
-        return fromCurbsides.size() == fromPoints.size() && !fromPoints.isEmpty() &&
-                toCurbsides.size() == toPoints.size() && !toPoints.isEmpty();
-    }
-
     /**
      * @param failFast if false the matrix calculation will be continued even when some points are not connected
      */
@@ -215,7 +203,7 @@ public boolean getFailFast() {
     }
 
     /**
-     * This method makes it more likely that hasPointHints returns true as often point hints are added although the
+     * This method clears the point hint lists if they only contain empty values. Often point hints are added although the
      * strings are empty. But because they could be used as placeholder we do not know earlier if they are meaningless.
      */
     void compactPointHints() {

core/src/main/java/com/graphhopper/routing/DirectionResolverResult.java

@@ -25,8 +25,8 @@
 import static com.graphhopper.util.Parameters.Routing.CURBSIDE;
 
 public class DirectionResolverResult {
-    private static DirectionResolverResult UNRESTRICTED = new DirectionResolverResult(ANY_EDGE, ANY_EDGE, ANY_EDGE, ANY_EDGE);
-    private static DirectionResolverResult IMPOSSIBLE = new DirectionResolverResult(NO_EDGE, NO_EDGE, NO_EDGE, NO_EDGE);
+    private static final DirectionResolverResult UNRESTRICTED = new DirectionResolverResult(ANY_EDGE, ANY_EDGE, ANY_EDGE, ANY_EDGE);
+    private static final DirectionResolverResult IMPOSSIBLE = new DirectionResolverResult(NO_EDGE, NO_EDGE, NO_EDGE, NO_EDGE);
 
     private final int inEdgeRight;
     private final int outEdgeRight;

core/src/main/java/com/graphhopper/routing/Router.java

@@ -302,10 +302,9 @@ private void checkNonChMaxWaypointDistance(List<GHPoint> points) {
         GHPoint lastPoint = points.get(0);
         GHPoint point;
         double dist;
-        DistanceCalc calc = DIST_EARTH;
         for (int i = 1; i < points.size(); i++) {
             point = points.get(i);
-            dist = calc.calcDist(lastPoint.getLat(), lastPoint.getLon(), point.getLat(), point.getLon());
+            dist = DIST_EARTH.calcDist(lastPoint.getLat(), lastPoint.getLon(), point.getLat(), point.getLon());
             if (dist > routerConfig.getNonChMaxWaypointDistance()) {
                 Map<String, Object> detailMap = new HashMap<>(2);
                 detailMap.put("from", i - 1);

core/src/main/java/com/graphhopper/routing/RoutingAlgorithmFactorySimple.java

@@ -44,7 +44,7 @@ public RoutingAlgorithm createAlgo(Graph g, AlgorithmOptions opts) {
         } else if (DIJKSTRA.equalsIgnoreCase(algoStr)) {
             ra = new Dijkstra(g, weighting, opts.getTraversalMode());
 
-        } else if (ASTAR_BI.equalsIgnoreCase(algoStr) || Helper.isEmpty(opts.getAlgorithm())) {
+        } else if (ASTAR_BI.equalsIgnoreCase(algoStr) || Helper.isEmpty(algoStr)) {
             AStarBidirection aStarBi = new AStarBidirection(g, weighting,
                     opts.getTraversalMode());
             aStarBi.setApproximation(getApproximation(ASTAR_BI, opts, g.getNodeAccess()));

core/src/main/java/com/graphhopper/routing/lm/LMRoutingAlgorithmFactory.java

@@ -59,7 +59,7 @@ public RoutingAlgorithm createAlgo(Graph g, AlgorithmOptions opts) {
             algo.setApproximation(getApproximator(g, activeLM, epsilon));
             algo.setMaxVisitedNodes(opts.getMaxVisitedNodes());
             return algo;
-        } else if (ASTAR_BI.equalsIgnoreCase(algoStr) || Helper.isEmpty(opts.getAlgorithm())) {
+        } else if (ASTAR_BI.equalsIgnoreCase(algoStr) || Helper.isEmpty(algoStr)) {
             double epsilon = opts.getHints().getDouble(Parameters.Algorithms.AStarBi.EPSILON, 1);
             AStarBidirection algo = new AStarBidirection(g, weighting, opts.getTraversalMode());
             algo.setApproximation(getApproximator(g, activeLM, epsilon));

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

@@ -17,6 +17,7 @@
  */
 package com.graphhopper.routing.template;
 
+import com.carrotsearch.hppc.IntHashSet;
 import com.graphhopper.GHRequest;
 import com.graphhopper.GHResponse;
 import com.graphhopper.ResponsePath;
@@ -33,12 +34,9 @@
 import com.graphhopper.routing.weighting.Weighting;
 import com.graphhopper.storage.index.LocationIndex;
 import com.graphhopper.storage.index.QueryResult;
-import com.graphhopper.util.Helper;
-import com.graphhopper.util.Parameters;
+import com.graphhopper.util.*;
 import com.graphhopper.util.Parameters.Algorithms;
 import com.graphhopper.util.Parameters.Algorithms.RoundTrip;
-import com.graphhopper.util.PathMerger;
-import com.graphhopper.util.Translation;
 import com.graphhopper.util.exceptions.PointNotFoundException;
 import com.graphhopper.util.shapes.GHPoint;
 
@@ -119,6 +117,8 @@ public List<Path> calcPaths(QueryGraph queryGraph, RoutingAlgorithmFactory algoF
 
         long visitedNodesSum = 0L;
         QueryResult start = queryResults.get(0);
+        IntHashSet previousEdges = new IntHashSet();
+        avoidPathWeighting.setAvoidedEdges(previousEdges);
         for (int qrIndex = 1; qrIndex < queryResults.size(); qrIndex++) {
             RoutingAlgorithm algo = algoFactory.createAlgo(queryGraph, algoOpts);
             // instead getClosestNode (which might be a virtual one and introducing unnecessary tails of the route)
@@ -134,8 +134,10 @@ public List<Path> calcPaths(QueryGraph queryGraph, RoutingAlgorithmFactory algoF
 
             pathList.add(path);
 
-            // it is important to avoid previously visited nodes for future paths
-            avoidPathWeighting.addEdges(path.calcEdges());
+            // add the edges of this path to the set of previous edges so they will be avoided from now, otherwise
+            // we do not get a nice 'round trip'. note that for this reason we cannot use CH for round-trips currently
+            for (EdgeIteratorState e : path.calcEdges())
+                previousEdges.add(e.getEdge());
         }
 
         ghResponse.getHints().putObject("visited_nodes.sum", visitedNodesSum);

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

@@ -77,17 +77,17 @@ public List<QueryResult> lookup(List<GHPoint> points) {
         if (points.size() < 2)
             throw new IllegalArgumentException("At least 2 points have to be specified, but was:" + points.size());
 
-        EdgeFilter strictEdgeFilter = !ghRequest.hasSnapPreventions()
+        EdgeFilter strictEdgeFilter = ghRequest.getSnapPreventions().isEmpty()
                 ? edgeFilter
                 : new SnapPreventionEdgeFilter(edgeFilter, roadClassEnc, roadEnvEnc, ghRequest.getSnapPreventions());
         queryResults = new ArrayList<>(points.size());
         for (int placeIndex = 0; placeIndex < points.size(); placeIndex++) {
             GHPoint point = points.get(placeIndex);
             QueryResult qr = null;
-            if (ghRequest.hasPointHints())
+            if (!ghRequest.getPointHints().isEmpty())
                 qr = locationIndex.findClosest(point.lat, point.lon, new NameSimilarityEdgeFilter(strictEdgeFilter,
                         ghRequest.getPointHints().get(placeIndex), point, 100));
-            else if (ghRequest.hasSnapPreventions())
+            else if (!ghRequest.getSnapPreventions().isEmpty())
                 qr = locationIndex.findClosest(point.lat, point.lon, strictEdgeFilter);
             if (qr == null || !qr.isValid())
                 qr = locationIndex.findClosest(point.lat, point.lon, edgeFilter);

core/src/main/java/com/graphhopper/routing/weighting/AvoidEdgesWeighting.java

@@ -21,17 +21,15 @@
 import com.graphhopper.coll.GHIntHashSet;
 import com.graphhopper.util.EdgeIteratorState;
 
-import java.util.Collection;
-
 /**
- * Rates already used Paths worse.
+ * Increases the weight for a certain set of edges by a given factor and thus makes them less likely to be part of
+ * a shortest path
  *
  * @author Robin Boldt
  */
 public class AvoidEdgesWeighting extends AbstractAdjustedWeighting {
     // contains the edge IDs of the already visited edges
-    protected final IntSet visitedEdges = new GHIntHashSet();
-
+    protected IntSet avoidedEdges = new GHIntHashSet();
     private double edgePenaltyFactor = 5.0;
 
     public AvoidEdgesWeighting(Weighting superWeighting) {
@@ -43,20 +41,14 @@ public AvoidEdgesWeighting setEdgePenaltyFactor(double edgePenaltyFactor) {
         return this;
     }
 
-    /**
-     * This method adds the specified path to this weighting which should be penalized in the
-     * calcWeight method.
-     */
-    public void addEdges(Collection<EdgeIteratorState> edges) {
-        for (EdgeIteratorState edge : edges) {
-            visitedEdges.add(edge.getEdge());
-        }
+    public void setAvoidedEdges(IntSet avoidedEdges) {
+        this.avoidedEdges = avoidedEdges;
     }
 
     @Override
     public double calcEdgeWeight(EdgeIteratorState edgeState, boolean reverse) {
         double weight = superWeighting.calcEdgeWeight(edgeState, reverse);
-        if (visitedEdges.contains(edgeState.getEdge()))
+        if (avoidedEdges.contains(edgeState.getEdge()))
             return weight * edgePenaltyFactor;
 
         return weight;

core/src/main/java/com/graphhopper/routing/weighting/QueryGraphWeighting.java

@@ -109,6 +109,11 @@ public String getName() {
         return weighting.getName();
     }
 
+    @Override
+    public String toString() {
+        return getName();
+    }
+
     private int getOriginalEdge(int edge) {
         return closestEdges.get((edge - firstVirtualEdgeId) / 4);
     }

core/src/main/java/com/graphhopper/util/FetchMode.java

@@ -5,5 +5,5 @@
  * {@link EdgeIteratorState#fetchWayGeometry(FetchMode)}. See also docs/core/low-level-api.md
  */
 public enum FetchMode {
-    TOWER_ONLY, PILLAR_ONLY, BASE_AND_PILLAR, PILLAR_AND_ADJ, ALL;
+    TOWER_ONLY, PILLAR_ONLY, BASE_AND_PILLAR, PILLAR_AND_ADJ, ALL
 }

core/src/test/java/com/graphhopper/GraphHopperAPITest.java

@@ -67,7 +67,7 @@ public void testLoad() {
                 loadGraph(graph);
         // 3 -> 0
         GHResponse rsp = instance.route(new GHRequest(42, 10.4, 42, 10).setProfile(profile));
-        assertFalse(rsp.hasErrors());
+        assertFalse(rsp.getErrors().toString(), rsp.hasErrors());
         ResponsePath responsePath = rsp.getBest();
         assertEquals(80, responsePath.getDistance(), 1e-6);
 

core/src/test/java/com/graphhopper/routing/DirectedBidirectionalDijkstraTest.java

@@ -1,6 +1,7 @@
 package com.graphhopper.routing;
 
 import com.carrotsearch.hppc.IntArrayList;
+import com.carrotsearch.hppc.IntHashSet;
 import com.graphhopper.Repeat;
 import com.graphhopper.RepeatRule;
 import com.graphhopper.routing.ev.DecimalEncodedValue;
@@ -22,7 +23,6 @@
 import org.junit.Test;
 
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.Random;
 
@@ -455,7 +455,7 @@ public void blockArea() {
     private AvoidEdgesWeighting createAvoidEdgeWeighting(EdgeIteratorState edgeOut) {
         AvoidEdgesWeighting avoidEdgesWeighting = new AvoidEdgesWeighting(weighting);
         avoidEdgesWeighting.setEdgePenaltyFactor(Double.POSITIVE_INFINITY);
-        avoidEdgesWeighting.addEdges(Collections.singletonList(edgeOut));
+        avoidEdgesWeighting.setAvoidedEdges(IntHashSet.from(edgeOut.getEdge()));
         return avoidEdgesWeighting;
     }