Skip to content

Commit 3c63803

Browse files
author
Peter
committed
refactoring of AltResponse due to inconsistencies discussion: https://discuss.graphhopper.com/t/463 
1 parent 2147745 commit 3c63803

File tree

16 files changed

+137
-144
lines changed

16 files changed

+137
-144
lines changed

‎android/app/src/main/java/com/graphhopper/android/MainActivity.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import android.widget.Spinner;
2424
import android.widget.TextView;
2525
import android.widget.Toast;
26-
import com.graphhopper.AltResponse;
26+
import com.graphhopper.PathWrapper;
2727

2828
import com.graphhopper.GHRequest;
2929
import com.graphhopper.GHResponse;
@@ -462,7 +462,7 @@ private void finishPrepare()
462462
prepareInProgress = false;
463463
}
464464

465-
private Polyline createPolyline( AltResponse response )
465+
private Polyline createPolyline( PathWrapper response )
466466
{
467467
Paint paintStroke = AndroidGraphicFactory.INSTANCE.createPaint();
468468
paintStroke.setStyle(Style.STROKE);
@@ -496,11 +496,11 @@ public void calcPath( final double fromLat, final double fromLon,
496496
{
497497

498498
log("calculating path ...");
499-
new AsyncTask<Void, Void, AltResponse>()
499+
new AsyncTask<Void, Void, PathWrapper>()
500500
{
501501
float time;
502502

503-
protected AltResponse doInBackground( Void... v )
503+
protected PathWrapper doInBackground( Void... v )
504504
{
505505
StopWatch sw = new StopWatch().start();
506506
GHRequest req = new GHRequest(fromLat, fromLon, toLat, toLon).
@@ -509,10 +509,10 @@ protected AltResponse doInBackground( Void... v )
509509
put("instructions", "false");
510510
GHResponse resp = hopper.route(req);
511511
time = sw.stop().getSeconds();
512-
return resp.getFirst();
512+
return resp.getBest();
513513
}
514514

515-
protected void onPostExecute( AltResponse resp )
515+
protected void onPostExecute( PathWrapper resp )
516516
{
517517
if (!resp.hasErrors())
518518
{

‎core/src/main/java/com/graphhopper/GHResponse.java

Lines changed: 31 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import java.util.List;
2424

2525
/**
26-
* Wrapper to simplify output of GraphHopper.
26+
* Wrapper containing path and error output of GraphHopper.
2727
* <p>
2828
* @author Peter Karich
2929
*/
@@ -32,36 +32,42 @@ public class GHResponse
3232
private String debugInfo = "";
3333
private final List<Throwable> errors = new ArrayList<Throwable>(4);
3434
private final PMap hintsMap = new PMap();
35-
private final List<AltResponse> alternatives = new ArrayList<AltResponse>(5);
35+
private final List<PathWrapper> pathWrappers = new ArrayList<PathWrapper>(5);
3636

3737
public GHResponse()
3838
{
3939
}
4040

41-
public void addAlternative( AltResponse altResponse )
41+
public void add( PathWrapper altResponse )
4242
{
43-
alternatives.add(altResponse);
43+
pathWrappers.add(altResponse);
4444
}
4545

4646
/**
47-
* Returns the first response.
47+
* Returns the best path.
4848
*/
49-
public AltResponse getFirst()
49+
public PathWrapper getBest()
5050
{
51-
if (alternatives.isEmpty())
52-
throw new RuntimeException("Cannot fetch first alternative if list is empty");
51+
if (pathWrappers.isEmpty())
52+
throw new RuntimeException("Cannot fetch best response if list is empty");
5353

54-
return alternatives.get(0);
54+
return pathWrappers.get(0);
5555
}
5656

57-
public List<AltResponse> getAlternatives()
57+
/**
58+
* This method returns the best path as well as all alternatives.
59+
*/
60+
public List<PathWrapper> getAll()
5861
{
59-
return alternatives;
62+
return pathWrappers;
6063
}
6164

65+
/**
66+
* This method returns true if there are alternative paths available besides the best.
67+
*/
6268
public boolean hasAlternatives()
6369
{
64-
return !alternatives.isEmpty();
70+
return pathWrappers.size() > 1;
6571
}
6672

6773
public void addDebugInfo( String debugInfo )
@@ -78,7 +84,7 @@ public void addDebugInfo( String debugInfo )
7884
public String getDebugInfo()
7985
{
8086
String str = debugInfo;
81-
for (AltResponse ar : alternatives)
87+
for (PathWrapper ar : pathWrappers)
8288
{
8389
if (!str.isEmpty())
8490
str += "; ";
@@ -89,27 +95,17 @@ public String getDebugInfo()
8995
}
9096

9197
/**
92-
* This method returns true only if the response itself is errornous.
93-
* <p/>
94-
* @see #hasErrors()
95-
*/
96-
public boolean hasRawErrors()
97-
{
98-
return !errors.isEmpty();
99-
}
100-
101-
/**
102-
* This method returns true if no alternative is available, if one of these has an error or if
103-
* the response itself is errornous.
98+
* This method returns true if one of the paths has an error or if the response itself is
99+
* errornous.
104100
* <p/>
105101
* @see #hasRawErrors()
106102
*/
107103
public boolean hasErrors()
108104
{
109-
if (hasRawErrors() || alternatives.isEmpty())
105+
if (!errors.isEmpty())
110106
return true;
111107

112-
for (AltResponse ar : alternatives)
108+
for (PathWrapper ar : pathWrappers)
113109
{
114110
if (ar.hasErrors())
115111
return true;
@@ -119,19 +115,16 @@ public boolean hasErrors()
119115
}
120116

121117
/**
122-
* This method returns all the explicitely added errors and the errors of all alternatives.
118+
* This method returns all the explicitely added errors and the errors of all paths.
123119
*/
124120
public List<Throwable> getErrors()
125121
{
126122
List<Throwable> list = new ArrayList<Throwable>();
127123
list.addAll(errors);
128-
if (alternatives.isEmpty())
129-
list.add(new IllegalStateException("No alternative existent"));
130-
else
131-
for (AltResponse ar : alternatives)
132-
{
133-
list.addAll(ar.getErrors());
134-
}
124+
for (PathWrapper ar : pathWrappers)
125+
{
126+
list.addAll(ar.getErrors());
127+
}
135128
return list;
136129
}
137130

@@ -151,13 +144,13 @@ public GHResponse addError( Throwable error )
151144
public String toString()
152145
{
153146
String str = "";
154-
for (AltResponse a : alternatives)
147+
for (PathWrapper a : pathWrappers)
155148
{
156149
str += "; " + a.toString();
157150
}
158151

159-
if (alternatives.isEmpty())
160-
str = "no alternatives";
152+
if (pathWrappers.isEmpty())
153+
str = "no paths";
161154

162155
if (!errors.isEmpty())
163156
str += ", main errors: " + errors.toString();

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,7 +1069,7 @@ protected List<Path> calcPaths( GHRequest request, GHResponse ghRsp )
10691069
StopWatch sw = new StopWatch().start();
10701070
List<QueryResult> qResults = lookup(points, encoder, ghRsp);
10711071
ghRsp.addDebugInfo("idLookup:" + sw.stop().getSeconds() + "s");
1072-
if (ghRsp.hasRawErrors())
1072+
if (ghRsp.hasErrors())
10731073
return Collections.emptyList();
10741074

10751075
Weighting weighting;
@@ -1116,8 +1116,8 @@ protected List<Path> calcPaths( GHRequest request, GHResponse ghRsp )
11161116
Translation tr = trMap.getWithFallBack(locale);
11171117

11181118
// Every alternative path makes one AltResponse BUT if via points exists then reuse the altResponse object
1119-
AltResponse altResponse = new AltResponse();
1120-
ghRsp.addAlternative(altResponse);
1119+
PathWrapper altResponse = new PathWrapper();
1120+
ghRsp.add(altResponse);
11211121
boolean isRoundTrip = AlgorithmOptions.ROUND_TRIP_ALT.equalsIgnoreCase(algoOpts.getAlgorithm());
11221122
boolean isAlternativeRoute = AlgorithmOptions.ALT_ROUTE.equalsIgnoreCase(algoOpts.getAlgorithm());
11231123

@@ -1187,14 +1187,14 @@ protected List<Path> calcPaths( GHRequest request, GHResponse ghRsp )
11871187
pathMerger.doWork(altResponse, Collections.singletonList(altPaths.get(0)), tr);
11881188
for (int index = 1; index < altPaths.size(); index++)
11891189
{
1190-
altResponse = new AltResponse();
1191-
ghRsp.addAlternative(altResponse);
1190+
altResponse = new PathWrapper();
1191+
ghRsp.add(altResponse);
11921192
pathMerger.doWork(altResponse, Collections.singletonList(altPaths.get(index)), tr);
11931193
}
11941194
} else if (isRoundTrip)
11951195
{
11961196
if (points.size() != altPaths.size())
1197-
throw new RuntimeException("There should be exactly one more points than paths. points:" + points.size() + ", paths:" + altPaths.size());
1197+
throw new RuntimeException("There should be the same number of points as paths. points:" + points.size() + ", paths:" + altPaths.size());
11981198

11991199
pathMerger.doWork(altResponse, altPaths, tr);
12001200
} else

‎core/src/main/java/com/graphhopper/AltResponse.java renamed to ‎core/src/main/java/com/graphhopper/PathWrapper.java

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@
2525
import java.util.List;
2626

2727
/**
28-
* This class holds one possibility of a route
29-
* <p/>
28+
* This class holds the data like points and instructions of a Path.
29+
* <p>
3030
* @author Peter Karich
3131
*/
32-
public class AltResponse
32+
public class PathWrapper
3333
{
3434
private List<String> description;
3535
private double distance;
@@ -53,13 +53,13 @@ public List<String> getDescription()
5353
return description;
5454
}
5555

56-
public AltResponse setDescription( List<String> names )
56+
public PathWrapper setDescription( List<String> names )
5757
{
5858
this.description = names;
5959
return this;
6060
}
6161

62-
public AltResponse addDebugInfo( String debugInfo )
62+
public PathWrapper addDebugInfo( String debugInfo )
6363
{
6464
if (debugInfo == null)
6565
throw new IllegalStateException("Debug information has to be none null");
@@ -76,7 +76,7 @@ public String getDebugInfo()
7676
return debugInfo;
7777
}
7878

79-
public AltResponse setPoints( PointList points )
79+
public PathWrapper setPoints( PointList points )
8080
{
8181
list = points;
8282
return this;
@@ -93,7 +93,7 @@ public PointList getPoints()
9393
return list;
9494
}
9595

96-
public AltResponse setDistance( double distance )
96+
public PathWrapper setDistance( double distance )
9797
{
9898
this.distance = distance;
9999
return this;
@@ -111,7 +111,7 @@ public double getDistance()
111111
return distance;
112112
}
113113

114-
public AltResponse setAscend( double ascend )
114+
public PathWrapper setAscend( double ascend )
115115
{
116116
if (ascend < 0)
117117
throw new IllegalArgumentException("ascend has to be strictly positive");
@@ -130,7 +130,7 @@ public double getAscend()
130130
return ascend;
131131
}
132132

133-
public AltResponse setDescend( double descend )
133+
public PathWrapper setDescend( double descend )
134134
{
135135
if (descend < 0)
136136
throw new IllegalArgumentException("descend has to be strictly positive");
@@ -149,7 +149,7 @@ public double getDescend()
149149
return descend;
150150
}
151151

152-
public AltResponse setTime( long timeInMillis )
152+
public PathWrapper setTime( long timeInMillis )
153153
{
154154
this.time = timeInMillis;
155155
return this;
@@ -164,7 +164,7 @@ public long getTime()
164164
return time;
165165
}
166166

167-
public AltResponse setRouteWeight( double weight )
167+
public PathWrapper setRouteWeight( double weight )
168168
{
169169
this.routeWeight = weight;
170170
return this;
@@ -257,13 +257,13 @@ public List<Throwable> getErrors()
257257
return errors;
258258
}
259259

260-
public AltResponse addError( Throwable error )
260+
public PathWrapper addError( Throwable error )
261261
{
262262
errors.add(error);
263263
return this;
264264
}
265265

266-
public AltResponse addErrors( List<Throwable> errors )
266+
public PathWrapper addErrors( List<Throwable> errors )
267267
{
268268
this.errors.addAll(errors);
269269
return this;

‎core/src/main/java/com/graphhopper/routing/util/TestAlgoCollector.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
*/
1818
package com.graphhopper.routing.util;
1919

20-
import com.graphhopper.AltResponse;
20+
import com.graphhopper.PathWrapper;
2121
import com.graphhopper.routing.*;
2222
import com.graphhopper.storage.Graph;
2323
import com.graphhopper.storage.CHGraph;
@@ -69,7 +69,7 @@ public TestAlgoCollector assertDistance( AlgoHelperEntry algoEntry, List<QueryRe
6969
setCalcPoints(true).
7070
setSimplifyResponse(false).
7171
setEnableInstructions(true);
72-
AltResponse rsp = new AltResponse();
72+
PathWrapper rsp = new PathWrapper();
7373
pathMerger.doWork(rsp, altPaths, trMap.getWithFallBack(Locale.US));
7474

7575
if (rsp.hasErrors())

‎core/src/main/java/com/graphhopper/util/PathMerger.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919
package com.graphhopper.util;
2020

21-
import com.graphhopper.AltResponse;
21+
import com.graphhopper.PathWrapper;
2222
import com.graphhopper.routing.Path;
2323
import java.util.ArrayList;
2424
import java.util.List;
@@ -61,7 +61,7 @@ public PathMerger setEnableInstructions( boolean enableInstructions )
6161
return this;
6262
}
6363

64-
public void doWork( AltResponse altRsp, List<Path> paths, Translation tr )
64+
public void doWork( PathWrapper altRsp, List<Path> paths, Translation tr )
6565
{
6666
int origPoints = 0;
6767
long fullTimeInMillis = 0;
@@ -150,7 +150,7 @@ public void doWork( AltResponse altRsp, List<Path> paths, Translation tr )
150150
setTime(fullTimeInMillis);
151151
}
152152

153-
private void calcAscendDescend( final AltResponse rsp, final PointList pointList )
153+
private void calcAscendDescend( final PathWrapper rsp, final PointList pointList )
154154
{
155155
double ascendMeters = 0;
156156
double descendMeters = 0;

‎core/src/test/java/com/graphhopper/GHResponseTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,15 @@ public class GHResponseTest
88
@Test
99
public void testToString() throws Exception
1010
{
11-
assertEquals("no alternatives", new GHResponse().toString());
11+
assertEquals("no paths", new GHResponse().toString());
1212
}
1313

1414
@Test
15-
public void testHasError() throws Exception
15+
public void testHasNoErrorIfEmpty() throws Exception
1616
{
17-
assertTrue(new GHResponse().hasErrors());
17+
assertFalse(new GHResponse().hasErrors());
1818
GHResponse rsp = new GHResponse();
19-
rsp.addAlternative(new AltResponse());
19+
rsp.add(new PathWrapper());
2020
assertFalse(rsp.hasErrors());
2121
}
2222
}

0 commit comments

Comments
 (0)