Skip to content

Commit 48daef4

Browse files
t2grant2gran
authored
Merge pull request opentripplanner#7077 from entur/refactor-small
Cleanup SlackProvider and other misc refactorings
2 parents dd8052c + 86b5333 commit 48daef4

29 files changed

Lines changed: 135 additions & 117 deletions

‎application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/api/SlackProvider.java‎

Lines changed: 0 additions & 11 deletions
This file was deleted.

application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/SlackProvider.java renamed to application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/DefaultSlackProvider.java

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import org.opentripplanner.raptor.spi.RaptorSlackProvider;
55
import org.opentripplanner.routing.api.request.framework.DurationForEnum;
66
import org.opentripplanner.transit.model.basic.TransitMode;
7+
import org.opentripplanner.transit.model.network.RoutingTripPattern;
78

89
/**
910
* This class provides transferSlack, boardSlack and alightSlack for the Raptor algorithm.
@@ -12,7 +13,7 @@
1213
* ordinal, and not in a {@link Map}, because it is faster. The board-slack and alight-slack lookup
1314
* is done for every strop-arrival computation, and should be as fast as possible.
1415
*/
15-
public final class SlackProvider implements RaptorSlackProvider {
16+
public final class DefaultSlackProvider implements RaptorSlackProvider {
1617

1718
/**
1819
* Keep a list of board-slack values for each mode.
@@ -29,7 +30,7 @@ public final class SlackProvider implements RaptorSlackProvider {
2930
*/
3031
private final int transferSlack;
3132

32-
public SlackProvider(
33+
public DefaultSlackProvider(
3334
int transferSlack,
3435
DurationForEnum<TransitMode> boardSlack,
3536
DurationForEnum<TransitMode> alightSlack
@@ -59,12 +60,8 @@ public int alightSlack(int slackIndex) {
5960
private static int[] slackByMode(DurationForEnum<TransitMode> slack) {
6061
int[] result = new int[TransitMode.values().length];
6162
for (TransitMode mode : TransitMode.values()) {
62-
result[SlackProvider.slackIndex(mode)] = (int) slack.valueOf(mode).toSeconds();
63+
result[RoutingTripPattern.slackIndex(mode)] = (int) slack.valueOf(mode).toSeconds();
6364
}
6465
return result;
6566
}
66-
67-
public static int slackIndex(final TransitMode mode) {
68-
return mode.ordinal();
69-
}
7067
}

‎application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitData.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@
1818
import org.opentripplanner.raptor.spi.RaptorSlackProvider;
1919
import org.opentripplanner.raptor.spi.RaptorTransitDataProvider;
2020
import org.opentripplanner.raptor.util.BitSetIterator;
21+
import org.opentripplanner.routing.algorithm.raptoradapter.transit.DefaultSlackProvider;
2122
import org.opentripplanner.routing.algorithm.raptoradapter.transit.RaptorTransferIndex;
2223
import org.opentripplanner.routing.algorithm.raptoradapter.transit.RaptorTransitData;
23-
import org.opentripplanner.routing.algorithm.raptoradapter.transit.SlackProvider;
2424
import org.opentripplanner.routing.algorithm.raptoradapter.transit.TripSchedule;
2525
import org.opentripplanner.routing.algorithm.raptoradapter.transit.constrainedtransfer.ConstrainedBoardingSearch;
2626
import org.opentripplanner.routing.algorithm.raptoradapter.transit.constrainedtransfer.ConstrainedTransfersForPatterns;
@@ -107,7 +107,7 @@ public RaptorRoutingRequestTransitData(
107107
raptorTransitData.getStopBoardAlightTransferCosts()
108108
);
109109

110-
this.slackProvider = new SlackProvider(
110+
this.slackProvider = new DefaultSlackProvider(
111111
(int) request.preferences().transfer().slack().toSeconds(),
112112
request.preferences().transit().boardSlack(),
113113
request.preferences().transit().alightSlack()

‎application/src/main/java/org/opentripplanner/transit/model/network/RoutingTripPattern.java‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import java.util.concurrent.atomic.AtomicInteger;
66
import org.opentripplanner.raptor.api.model.RaptorTripPattern;
77
import org.opentripplanner.routing.algorithm.raptoradapter.api.DefaultTripPattern;
8+
import org.opentripplanner.transit.model.basic.TransitMode;
89
import org.opentripplanner.transit.model.site.StopLocation;
910

1011
/**
@@ -43,8 +44,11 @@ public class RoutingTripPattern implements DefaultTripPattern, Serializable {
4344
wheelchairAccessible.set(s, pattern.wheelchairAccessible(s));
4445
}
4546

46-
this.slackIndex = builder.slackIndex();
47-
this.transitReluctanceFactorIndex = builder.transitReluctanceFactorIndex();
47+
// This uses a static SlackProvider. Change it to be injectable if required
48+
this.slackIndex = slackIndex(pattern.getRoute().getMode());
49+
50+
// Change the calculation to be injectable if required
51+
this.transitReluctanceFactorIndex = pattern.getRoute().getMode().ordinal();
4852
}
4953

5054
/**
@@ -151,4 +155,12 @@ public static int indexCounter() {
151155
public static void initIndexCounter(int indexCounter) {
152156
INDEX_COUNTER.set(indexCounter);
153157
}
158+
159+
/**
160+
* Return an index used to look up the slack for a {@link RaptorTripPattern}. OTP support
161+
* setting a slack per mode or the same value for all modes.
162+
*/
163+
public static int slackIndex(final TransitMode mode) {
164+
return mode.ordinal();
165+
}
154166
}

‎application/src/main/java/org/opentripplanner/transit/model/network/TripPatternBuilder.java‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import org.opentripplanner.framework.geometry.GeometryUtils;
1111
import org.opentripplanner.model.Timetable;
1212
import org.opentripplanner.model.TimetableBuilder;
13-
import org.opentripplanner.routing.algorithm.raptoradapter.api.SlackProvider;
1413
import org.opentripplanner.transit.model.basic.SubMode;
1514
import org.opentripplanner.transit.model.basic.TransitMode;
1615
import org.opentripplanner.transit.model.framework.AbstractEntityBuilder;
@@ -126,16 +125,6 @@ public TripPatternBuilder withHopGeometries(List<LineString> hopGeometries) {
126125
return this;
127126
}
128127

129-
// TODO: This uses a static SlackProvider. Change it to be injectable if required
130-
public int slackIndex() {
131-
return SlackProvider.slackIndex(route.getMode());
132-
}
133-
134-
// TODO: Change the calculation to be injectable if required
135-
public int transitReluctanceFactorIndex() {
136-
return route.getMode().ordinal();
137-
}
138-
139128
public Direction getDirection() {
140129
if (scheduledTimetable != null) {
141130
return scheduledTimetable.getDirection();
@@ -160,31 +149,31 @@ public SubMode getNetexSubmode() {
160149
return netexSubMode != null ? netexSubMode : route.getNetexSubmode();
161150
}
162151

163-
public boolean getContainsMultipleModes() {
152+
boolean getContainsMultipleModes() {
164153
return containsMultipleModes;
165154
}
166155

167156
public StopPattern getStopPattern() {
168157
return stopPattern;
169158
}
170159

171-
public Timetable getScheduledTimetable() {
160+
Timetable getScheduledTimetable() {
172161
return scheduledTimetable;
173162
}
174163

175164
public TimetableBuilder getScheduledTimetableBuilder() {
176165
return scheduledTimetableBuilder;
177166
}
178167

179-
public String getName() {
168+
String getName() {
180169
return name;
181170
}
182171

183172
public TripPattern getOriginalTripPattern() {
184173
return originalTripPattern;
185174
}
186175

187-
public boolean isCreatedByRealtimeUpdate() {
176+
boolean isCreatedByRealtimeUpdate() {
188177
return createdByRealtimeUpdate;
189178
}
190179

‎application/src/test-fixtures/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/TestSlackProvider.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package org.opentripplanner.routing.algorithm.raptoradapter.transit;
2+
3+
import org.opentripplanner.raptor.spi.RaptorSlackProvider;
4+
5+
/**
6+
* This is a simpler version than the {@link DefaultSlackProvider}. Raptor is not under test, so
7+
* there is no need for the complex version in most unit-tests.
8+
*
9+
* FOR TESTS ONLY
10+
*/
11+
public class TestSlackProvider implements RaptorSlackProvider {
12+
13+
private final int boardSlack;
14+
private final int alightSlack;
15+
private final int transferSlack;
16+
17+
public TestSlackProvider(int boardSlack, int alightSlack, int transferSlack) {
18+
this.boardSlack = boardSlack;
19+
this.alightSlack = alightSlack;
20+
this.transferSlack = transferSlack;
21+
}
22+
23+
@Override
24+
public int transferSlack() {
25+
return transferSlack;
26+
}
27+
28+
@Override
29+
public int boardSlack(int slackIndex) {
30+
return boardSlack;
31+
}
32+
33+
@Override
34+
public int alightSlack(int slackIndex) {
35+
return alightSlack;
36+
}
37+
}

‎application/src/test/java/org/opentripplanner/raptorlegacy/_data/RaptorTestConstants.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
import static org.opentripplanner.utils.time.DurationUtils.durationInSeconds;
44
import static org.opentripplanner.utils.time.TimeUtils.hm2time;
55

6-
import org.opentripplanner.raptor.spi.DefaultSlackProvider;
76
import org.opentripplanner.raptor.spi.RaptorCostCalculator;
87
import org.opentripplanner.raptor.spi.RaptorSlackProvider;
98
import org.opentripplanner.raptorlegacy._data.transit.TestTripSchedule;
9+
import org.opentripplanner.routing.algorithm.raptoradapter.transit.TestSlackProvider;
1010
import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.DefaultCostCalculator;
1111

1212
/**
@@ -70,12 +70,6 @@ public interface RaptorTestConstants {
7070
int TRANSFER_COST = 120;
7171
double WAIT_RELUCTANCE = 0.8;
7272

73-
RaptorSlackProvider SLACK_PROVIDER = new DefaultSlackProvider(
74-
TRANSFER_SLACK,
75-
BOARD_SLACK,
76-
ALIGHT_SLACK
77-
);
78-
7973
RaptorCostCalculator<TestTripSchedule> COST_CALCULATOR = new DefaultCostCalculator<>(
8074
BOARD_COST,
8175
TRANSFER_COST,
@@ -84,6 +78,12 @@ public interface RaptorTestConstants {
8478
null
8579
);
8680

81+
RaptorSlackProvider SLACK_PROVIDER = new TestSlackProvider(
82+
BOARD_SLACK,
83+
ALIGHT_SLACK,
84+
TRANSFER_SLACK
85+
);
86+
8787
default String stopIndexToName(int index) {
8888
return Character.toString('A' + index - 1);
8989
}

‎application/src/test/java/org/opentripplanner/raptorlegacy/_data/api/TestPathBuilder.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import org.opentripplanner.raptor.api.model.RaptorStopNameResolver;
99
import org.opentripplanner.raptor.api.path.RaptorPath;
1010
import org.opentripplanner.raptor.path.PathBuilder;
11-
import org.opentripplanner.raptor.spi.DefaultSlackProvider;
1211
import org.opentripplanner.raptor.spi.RaptorCostCalculator;
1312
import org.opentripplanner.raptor.spi.RaptorSlackProvider;
1413
import org.opentripplanner.raptorlegacy._data.RaptorTestConstants;
@@ -52,7 +51,7 @@ public TestPathBuilder(
5251
* Uses the slacks in {@link RaptorTestConstants}.
5352
*/
5453
public TestPathBuilder(@Nullable RaptorCostCalculator<TestTripSchedule> costCalculator) {
55-
this(new DefaultSlackProvider(TRANSFER_SLACK, BOARD_SLACK, ALIGHT_SLACK), costCalculator);
54+
this(SLACK_PROVIDER, costCalculator);
5655
}
5756

5857
/** Assign c2 value for path. TODO: Add c2 value for each leg. */

‎application/src/test/java/org/opentripplanner/raptorlegacy/_data/transit/TestTransitData.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import org.opentripplanner.raptor.api.model.RaptorStopNameResolver;
1414
import org.opentripplanner.raptor.api.model.RaptorTransfer;
1515
import org.opentripplanner.raptor.api.model.RaptorTripPattern;
16-
import org.opentripplanner.raptor.spi.DefaultSlackProvider;
1716
import org.opentripplanner.raptor.spi.IntIterator;
1817
import org.opentripplanner.raptor.spi.RaptorConstrainedBoardingSearch;
1918
import org.opentripplanner.raptor.spi.RaptorCostCalculator;
@@ -51,9 +50,6 @@ public class TestTransitData
5150
.minTransferTime(3600)
5251
.build();
5352

54-
// Slack defaults: 1 minute for transfer-slack, 0 minutes for board- and alight-slack.
55-
public static final RaptorSlackProvider SLACK_PROVIDER = new DefaultSlackProvider(60, 0, 0);
56-
5753
private final List<List<RaptorTransfer>> transfersFromStop = new ArrayList<>();
5854
private final List<List<RaptorTransfer>> transfersToStop = new ArrayList<>();
5955
private final List<Set<Integer>> routeIndexesByStopIndex = new ArrayList<>();

‎application/src/test/java/org/opentripplanner/routing/algorithm/transferoptimization/services/OptimizePathDomainServiceTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@
88
import java.util.List;
99
import javax.annotation.Nullable;
1010
import org.junit.jupiter.api.Test;
11-
import org.opentripplanner.raptor.spi.DefaultSlackProvider;
1211
import org.opentripplanner.raptor.spi.RaptorCostCalculator;
1312
import org.opentripplanner.raptor.spi.RaptorSlackProvider;
1413
import org.opentripplanner.raptorlegacy._data.RaptorTestConstants;
1514
import org.opentripplanner.raptorlegacy._data.api.PathUtils;
1615
import org.opentripplanner.raptorlegacy._data.api.TestPathBuilder;
1716
import org.opentripplanner.raptorlegacy._data.transit.TestTripSchedule;
17+
import org.opentripplanner.routing.algorithm.raptoradapter.transit.TestSlackProvider;
1818
import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.DefaultCostCalculator;
1919
import org.opentripplanner.routing.algorithm.transferoptimization.model.TransferWaitTimeCostCalculator;
2020
import org.opentripplanner.routing.algorithm.transferoptimization.model.costfilter.MinCostPathTailFilterFactory;
@@ -32,10 +32,10 @@ public class OptimizePathDomainServiceTest implements RaptorTestConstants {
3232
private static final int TRANSFER_COST_SEC = 20;
3333
private static final double WAIT_RELUCTANCE = 1.0;
3434

35-
private static final RaptorSlackProvider SLACK_PROVIDER = new DefaultSlackProvider(
36-
TRANSFER_SLACK,
35+
private static final RaptorSlackProvider SLACK_PROVIDER = new TestSlackProvider(
3736
BOARD_SLACK,
38-
ALIGHT_SLACK
37+
ALIGHT_SLACK,
38+
TRANSFER_SLACK
3939
);
4040

4141
public static final RaptorCostCalculator<TestTripSchedule> COST_CALCULATOR =
@@ -314,7 +314,7 @@ public void testSameStopTimesInPattern() {
314314

315315
static TestPathBuilder pathBuilder() {
316316
return new TestPathBuilder(
317-
new DefaultSlackProvider(TRANSFER_SLACK, BOARD_SLACK, ALIGHT_SLACK),
317+
new TestSlackProvider(BOARD_SLACK, ALIGHT_SLACK, TRANSFER_SLACK),
318318
COST_CALCULATOR
319319
);
320320
}

0 commit comments

Comments
 (0)