Apply suggestions from code review

Co-authored-by: Johan Torin <jtorin@users.noreply.github.com>

src/main/java/org/opentripplanner/framework/lang/IntBox.java

@@ -1,8 +1,8 @@
 package org.opentripplanner.framework.lang;
 
 /**
- * An int box, is a writable container for an int. The most common use-case for this class is to
- * be able to set an integer value inside a lambda callback where method variables is not
+ * An IntBox is a writable container for an int. The most common use-case for this class is to
+ * be able to set an integer value inside a lambda callback where local variables is not
  * accessible.
  */
 public final class IntBox {

src/main/java/org/opentripplanner/raptor/api/request/BitSetPassThroughPoints.java

@@ -15,7 +15,7 @@
  * <p>
  * We update the c2 value only if the current value is 1 less than the pass-through point
  * sequence number. This make sure pass-through points are visited in the right order.
- * The c2 value is equal to the last visited pass-through point sequence number. If 0(zero)
+ * The c2 value is equal to the last visited pass-through point sequence number. If zero
  * no pass-through point is visited yet.
  */
 public class BitSetPassThroughPoints implements PassThroughPoints {
@@ -46,15 +46,14 @@ public static PassThroughPoints create(final List<int[]> passThroughStops) {
 
   @Override
   public boolean isPassThroughPoint(int stop) {
-    // Make sure the c2 is NOT set if the stop is not a pass-through point
-    this.currentPassThroughPointSeqNo = RaptorConstants.NOT_SET;
-
     for (int i = 0; i < passThroughPoints.size(); ++i) {
       if (passThroughPoints.get(i).get(stop)) {
         currentPassThroughPointSeqNo = i + 1;
         return true;
       }
     }
+    // Make sure the c2 is NOT set if the stop is not a pass-through point
+    this.currentPassThroughPointSeqNo = RaptorConstants.NOT_SET;
     return false;
   }
 

src/main/java/org/opentripplanner/raptor/api/request/RaptorTransitPassThroughRequest.java

@@ -10,7 +10,6 @@ public RaptorTransitPassThroughRequest(final PassThroughPoints passThroughPoints
     this.passThroughPoints = passThroughPoints;
   }
 
-  // TODO: Should this method be part of the interface?
   public PassThroughPoints passThroughPoints() {
     return passThroughPoints;
   }

src/main/java/org/opentripplanner/raptor/rangeraptor/internalapi/RoutingStrategy.java

@@ -30,7 +30,7 @@ public interface RoutingStrategy<T extends RaptorTripSchedule> {
 
   /**
    * First the {@link #prepareForTransitWith(RaptorRoute)} is called, then this method is
-   * called before alight and board methods is called. This allows the strategy to  update
+   * called before alight and board methods are called. This allows the strategy to update
    * the state before alighting and boarding is processed. Especially if there is a change
    * in the state as a result of passing through a stop this method might come in handy.
    */

src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/McRangeRaptorWorkerState.java

@@ -1,7 +1,6 @@
 package org.opentripplanner.raptor.rangeraptor.multicriteria;
 
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import org.opentripplanner.raptor.api.model.RaptorAccessEgress;

src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/MultiCriteriaRoutingStrategy.java

@@ -72,7 +72,7 @@ public void prepareForNextStop(int stopIndex, int stopPos) {
       for (R ride : patternRides) {
         // Replace existing ride with same ride with the C2 value updated. This only happens if
         // the stop is a pass-through point and the path has visited the pass-through points in the
-        // right order so fare.
+        // correct order.
         //noinspection unchecked
         passThroughPoints.updateC2Value(
           ride.c2(),

src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/arrivals/ArrivalParetoSetComparatorFactory.java

@@ -63,16 +63,16 @@ > ArrivalParetoSetComparatorFactory<T> createFactoryC1AndC2(
       @Override
       public ParetoComparator<T> compareArrivalTimeRoundAndCost() {
         return (l, r) ->
-          c2DominanceFunction.leftDominateRight(l.c2(), r.c2()) || McStopArrival.compareBase(l, r);
+          McStopArrival.compareBase(l, r) || c2DominanceFunction.leftDominateRight(l.c2(), r.c2());
       }
 
       @Override
       public ParetoComparator<T> compareArrivalTimeRoundCostAndOnBoardArrival() {
         return (
           (l, r) ->
-            c2DominanceFunction.leftDominateRight(l.c2(), r.c2()) ||
             McStopArrival.compareBase(l, r) ||
-            McStopArrival.compareArrivedOnBoard(l, r)
+            McStopArrival.compareArrivedOnBoard(l, r) ||
+            c2DominanceFunction.leftDominateRight(l.c2(), r.c2())
         );
       }
     };

src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/ride/PatternRide.java

@@ -17,7 +17,7 @@
  * comparator used by the pareto-set of patternRides.
  * <p>
  * This implementation of the multi-criteria Range Raptor keeps all pareto-optimal _rides_ for each
- * pattern while possessing each stops down the line. This class keep the needed state for these
+ * pattern while possessing each stop down the line. This class keep the needed state for these
  * rides to avoid recalculating each value more than once and to be able to put them in a
  * {@link ParetoSet}.
  * <p>
@@ -34,7 +34,7 @@
  *    calculation.
  *  </li>
  *  <li>
- *    We do NOT allow a one trip to exclude the pattern-rides of another trip in the pareto-set.
+ *    We do NOT allow one trip to exclude the pattern-rides of another trip in the pareto-set.
  *    Two pattern-rides are both optimal, if they have boarded the same pattern, in the same round,
  *    but on different trips/vehicles. This have no measurable impact on performance, compared with
  *    allowing an earlier trip dominating a later one. But, it allows for a trip to be optimal at
@@ -48,7 +48,7 @@
  *    of pattern-rides.
  *  </li>
  * </ul>
- * This interface extends the {@link PatternRideView} with methods witch mutate the PatternRide
+ * This interface extends the {@link PatternRideView} with methods which mutate the PatternRide
  * (a new copy is created, since the rides are immutable). The interface is used by Raptor to
  * perform the routing, while the view is used by other parts; hence only need read access.
  * <p>
@@ -57,7 +57,7 @@
 public interface PatternRide<T extends RaptorTripSchedule>
   extends PatternRideView<T, McStopArrival<T>> {
   /**
-   * Change the ride by setting a new c2 value. Since, the ride is immutable the
+   * Change the ride by setting a new c2 value. Since the ride is immutable the
    * new ride is copied and returned.
    */
   PatternRide<T> updateC2(int newC2);

src/main/java/org/opentripplanner/raptor/rangeraptor/path/ForwardPathMapper.java

@@ -71,7 +71,6 @@ public RaptorPath<T> mapToPath(final DestinationArrival<T> destinationArrival) {
     }
 
     if (destinationArrival.supportsC2()) {
-      // TODO: 2023-05-19 via pass through: can we set c2 through builder like that?
       pathBuilder.c2(destinationArrival.c2());
     }
 

src/test/java/org/opentripplanner/raptor/moduletests/J01_PassThroughTest.java

@@ -71,7 +71,7 @@ private RaptorRequestBuilder<TestTripSchedule> prepareRequest() {
   }
 
   @Test
-  @DisplayName("Pass-through stop point as a first point in the journey.")
+  @DisplayName("Pass-through stop point as a last point in the journey.")
   public void passThroughPointOnEgress() {
     var data = new TestTransitData();
 
@@ -109,7 +109,7 @@ public void passThroughPointOnEgress() {
   }
 
   @Test
-  @DisplayName("Pass-through stop point as a last point in the journey.")
+  @DisplayName("Pass-through stop point as a first point in the journey.")
   public void passThroughPointOnAccess() {
     var data = new TestTransitData();
 
@@ -147,7 +147,7 @@ public void passThroughPointOnAccess() {
   }
 
   @Test
-  @DisplayName("Pass-through stop point as a intermediate point in the journey.")
+  @DisplayName("Pass-through stop point as an intermediate point in the journey.")
   public void passThroughPointInTheMiddle() {
     var data = new TestTransitData();
 

src/test/java/org/opentripplanner/raptor/moduletests/support/ModuleTestDebugLogging.java

@@ -25,13 +25,16 @@
  *  Accept  |  Access |   0 |     2 | 00:08:30 |    12 000 |      | Accepted element: Walk 30s ~ 2 (cost: 12000)
  * </pre>
  * <p>
- * You can enable debugging by setting variable DEBUG, an environment variable, or a system
- * property. If {@code debugRaptor} exist as either a system property or an environment variable
- * the module tests will log debug events.
+ * You can enable debug logging from the module tests by either:
+ * <ul>
+ *     <li> setting the variable 'DEBUG' below to true
+ *     <li> setting the environment variable 'debugRaptor' to an arbitrary value
+ *     <li> setting the system property 'debugRaptor' to an arbitrary value
+ * </ul>
  * <p>
  * Remember to revert to {@code DEBUG=false} before committing or use the other options.
  * <p>
- * Tip! Setting the system properties can be done on individual test in intellij, or in the JUnit
+ * Tip! Setting the system properties can be done on individual test in IntelliJ, or in the JUnit
  * template. Add {@code -DdebugRaptorOff} or {@code -DdebugRaptor} to the JUnit template. You can
  * easily change it when needed in the specific unit-test config, since the template is copied
  * before the first test run, and not before re-runs.

src/test/java/org/opentripplanner/raptor/rangeraptor/transit/ForwardRaptorTransitCalculatorTest.java

@@ -49,7 +49,7 @@ public void exceedsTimeLimit() {
 
   @Test
   public void acceptDestinationArrival() {
-    // TODO PT: Wrire a test that test that the exceedsTimeLimit and acceptC2 predicate is
+    // TODO PT: Write a test that test that the exceedsTimeLimit and acceptC2 predicate is
     //          done correct.
   }