Freight contrib: Improvements based on code inspection

contribs/freight/src/main/java/org/matsim/freight/carriers/CarrierJob.java

@@ -10,7 +10,7 @@
  * In the next steps it will be extended, as follows
  * 1) existing common methods of {@link CarrierShipment} and {@link
  * CarrierService} where moved up here
- * 2) some similiar, but differently named methods of {@link
+ * 2) some similar, but differently named methods of {@link
  * CarrierShipment} and {@link CarrierService} were renamed to the same name and moved up here
  * ...
  * future) It maybe gets generalized in way, that we only have one job definition with 1 or 2

contribs/freight/src/main/java/org/matsim/freight/carriers/CarrierPlan.java

@@ -69,7 +69,6 @@ public Double getScore() {
 	/**
 	 * In future (starting in May'23) this is the score from the MATSim scoring function.
 	 * The jsprit score is saved in attribute. Please use the method setJspritScore() to store it.
-	 * @return the (MATSim) score
 	 */
 	@Override
 	public void setScore(Double score) {

contribs/freight/src/main/java/org/matsim/freight/carriers/CarrierShipment.java

@@ -47,7 +47,7 @@ public final class CarrierShipment implements CarrierJob {
 	public static class Builder {
 
 		/**
-		 * @Deprecated Please use Builder newInstance(Id<CarrierShipment> id, Id<Link> from, Id<Link> to, int size) instead.
+		 * @deprecated Please use Builder newInstance(Id<CarrierShipment> id, Id<Link> from, Id<Link> to, int size) instead.
 		 * <p>
 		 * Returns a new shipment builder.
 		 *
@@ -90,7 +90,7 @@ public static Builder newInstance(Id<CarrierShipment> id, Id<Link> from, Id<Link
 		double delServiceTime = 0.0;
 
 		/**
-		 * @Deprecated Please use Builder (Id<CarrierShipment> id, Id<Link> from, Id<Link> to, int size) instead.
+		 * @deprecated Please use Builder (Id<CarrierShipment> id, Id<Link> from, Id<Link> to, int size) instead.
 		 */
 		@Deprecated
 		public Builder(Id<Link> from, Id<Link> to, int size) {

contribs/freight/src/main/java/org/matsim/freight/carriers/CarrierVehicleType.java

@@ -28,7 +28,7 @@
 /**
  * The carrier vehicle type.
  * <p>
- * I decided to put vehicle cost information into the type (which is indeed not a physical attribute of the type). Thus physical and
+ * I decided to put vehicle cost information into the type (which is indeed not a physical attribute of the type). Thus, physical and
  * non-physical attributes are used. This is likely to be changed in the future.
  *
  * @author sschroeder
@@ -71,7 +71,7 @@ private Builder(Id<VehicleType> typeId){
 		/**
 		 * Sets fixed costs of vehicle.
 		 *
-		 * <p>By default it is 0.
+		 * <p>By default, it is 0.
 		 * @param fix 	fixed costs
 		 * @return 		this builder
 		 */
@@ -83,7 +83,7 @@ public Builder setFixCost(double fix){
 		/**
 		 * Sets costs per distance-unit.
 		 *
-		 * <p>By default it is 1.
+		 * <p>By default, it is 1.
 		 *
 		 * @param perDistanceUnit 	costs per distance-unit
 		 * @return 					this builder
@@ -96,7 +96,7 @@ public Builder setCostPerDistanceUnit(double perDistanceUnit){
 		/**
 		 * Sets costs per time-unit.
 		 *
-		 * <p>By default it is 0.
+		 * <p>By default, it is 0.
 		 *
 		 * @param perTimeUnit 	costs per time-unit
 		 * @return 				this builder

contribs/freight/src/main/java/org/matsim/freight/carriers/CarrierVehicleTypeReader.java

@@ -78,7 +78,7 @@ public void readURL( URL url ){
 			reader.readURL(url);
 		}  catch (Exception e) {
 			log.warn("### Exception: Message={}", e.getMessage());
-			log.warn("### Exception: Cause={}", e.getCause());
+			log.warn("### Exception: Cause={}", e.getCause().toString());
 			log.warn("### Exception: Class={}", e.getClass());
 			if (e.getCause().getMessage().contains("cvc-elt.1.a")) { // "Cannot find the declaration of element" -> exception comes most probably because no validation information was found
 				log.warn("read with validation = true failed. Try it again without validation... url: {}", url.toString());

contribs/freight/src/main/java/org/matsim/freight/carriers/CarriersUtils.java

@@ -234,7 +234,7 @@ public static void runJsprit(Scenario scenario, CarrierSelectionForSolution carr
 						continue;
 					}
 				}
-				case solveForAllCarriersAndAddPLans -> {carrier.setSelectedPlan(null);} // Keep existing plan(s), but make them not selected.
+				case solveForAllCarriersAndAddPLans -> carrier.setSelectedPlan(null); // Keep existing plan(s), but make them not selected.
 				default -> throw new IllegalStateException("Unexpected value: " + carriersSolutionType);
 			}
 			carrierActivityCounterMap.put(carrier.getId(), carrierActivityCounterMap.getOrDefault(carrier.getId(), 0) + carrier.getServices().size());

contribs/freight/src/main/java/org/matsim/freight/carriers/analysis/CarrierLoadAnalysis.java

@@ -51,7 +51,7 @@ public class CarrierLoadAnalysis implements CarrierShipmentPickupStartEventHandl
 
 	private static final Logger log = LogManager.getLogger(CarrierLoadAnalysis.class);
 	private final String delimiter;
-	Carriers carriers;
+	final Carriers carriers;
 
 	private final Map<Id<Vehicle>, LinkedList<Integer>> vehicle2Load = new LinkedHashMap<>();
 
@@ -104,7 +104,7 @@ void writeLoadPerVehicle(String analysisOutputDirectory, Scenario scenario) thro
 		for (Id<Vehicle> vehicleId : vehicle2Load.keySet()) {
 
 			final LinkedList<Integer> load = vehicle2Load.get(vehicleId);
-			final Integer maxLoad = load.stream().max(Comparator.naturalOrder()).get();
+			final Integer maxLoad = load.stream().max(Comparator.naturalOrder()).orElseThrow();
 
 			final VehicleType vehicleType = VehicleUtils.findVehicle(vehicleId, scenario).getType();
 			final Double capacity = vehicleType.getCapacity().getOther();

contribs/freight/src/main/java/org/matsim/freight/carriers/analysis/CarrierPlanAnalysis.java

@@ -54,7 +54,7 @@ public class CarrierPlanAnalysis {
 	private static final Logger log = LogManager.getLogger(CarrierPlanAnalysis.class);
 	public final String delimiter;
 
-	Carriers carriers;
+	final Carriers carriers;
 
 	public CarrierPlanAnalysis(String delimiter, Carriers carriers) {
 		this.delimiter = delimiter;

contribs/freight/src/main/java/org/matsim/freight/carriers/analysis/FreightTimeAndDistanceAnalysisEventsHandler.java

@@ -226,7 +226,7 @@ void writeTravelTimeAndDistancePerVehicle(String analysisOutputDirectory, Scenar
 	void writeTravelTimeAndDistancePerVehicleType(String analysisOutputDirectory, Scenario scenario) throws IOException {
 		log.info("Writing out Time & Distance & Costs ... perVehicleType");
 
-		//----- All VehicleTypes in CarriervehicleTypes container. Used so that even unused vehTypes appear in the output
+		//----- All VehicleTypes in CarrierVehicleTypes container. Used so that even unused vehTypes appear in the output
 		TreeMap<Id<VehicleType>, VehicleType> vehicleTypesMap = new TreeMap<>(CarriersUtils.getCarrierVehicleTypes(scenario).getVehicleTypes());
 		//For the case that there are additional vehicle types found in the events.
 		for (VehicleType vehicleType : vehicleId2VehicleType.values()) {

contribs/freight/src/main/java/org/matsim/freight/carriers/analysis/RunFreightAnalysisEventBased.java

@@ -82,11 +82,12 @@ public RunFreightAnalysisEventBased(String simOutputPath, String analysisOutputP
 
 		// the better version with the globFile method is not available since there is a circular dependency between the modules application and freight
 
-		this.EVENTS_PATH = Path.of(simOutputPath).resolve("output_events.xml.gz").toString();
-		String vehiclesPath = Path.of(simOutputPath).resolve("output_allVehicles.xml.gz").toString();
-		String networkPath = Path.of(simOutputPath).resolve("output_network.xml.gz").toString();
-		String carriersPath = Path.of(simOutputPath).resolve("output_carriers.xml.gz").toString();
-		String carriersVehicleTypesPath = Path.of(simOutputPath).resolve("output_carriersVehicleTypes.xml.gz").toString();
+		final Path path = Path.of(simOutputPath);
+		this.EVENTS_PATH = path.resolve("output_events.xml.gz").toString();
+		String vehiclesPath = path.resolve("output_allVehicles.xml.gz").toString();
+		String networkPath = path.resolve("output_network.xml.gz").toString();
+		String carriersPath = path.resolve("output_carriers.xml.gz").toString();
+		String carriersVehicleTypesPath = path.resolve("output_carriersVehicleTypes.xml.gz").toString();
 
 		createScenarioForFreightAnalysis(vehiclesPath, networkPath, carriersPath, carriersVehicleTypesPath, globalCrs);
 	}
@@ -135,7 +136,7 @@ private void createScenarioForFreightAnalysis(String vehiclesPath, String networ
 		//freight settings
 		FreightCarriersConfigGroup freightCarriersConfigGroup = ConfigUtils.addOrGetModule(config, FreightCarriersConfigGroup.class);
 		freightCarriersConfigGroup.setCarriersFile(carriersPath);
-		freightCarriersConfigGroup.setCarriersVehicleTypesFile(carriersVehicleTypesPath.toString());
+		freightCarriersConfigGroup.setCarriersVehicleTypesFile(carriersVehicleTypesPath);
 
 		scenario = ScenarioUtils.loadScenario(config);
 

contribs/freight/src/main/java/org/matsim/freight/carriers/controller/CarrierDriverAgent.java

@@ -264,7 +264,7 @@ else if( index < elementsSize ){
 	 * Basic event handler that collects the relation between vehicles and drivers.
 	 * Necessary since link enter and leave events do not contain the driver anymore.
 	 * <p>
-	 * This is the vice-versa implementation of {@link org.matsim.core.events.algorithms.Vehicle2DriverEventHandler}.
+	 * This is the vice versa implementation of {@link org.matsim.core.events.algorithms.Vehicle2DriverEventHandler}.
 	 * <p>
 	 * In a first step only used internally. When needed more often, I have nothing against putting it more central.
 	 *

contribs/freight/src/main/java/org/matsim/freight/carriers/events/CarrierTourStartEventCreator.java

@@ -68,9 +68,9 @@ public Event createEvent(Event event, Carrier carrier, Activity activity, Schedu
 	/**
 	 * Creating the FreightTourStartsEvent
 	 *
-	 * @param personId
-	 * @param carrier
-	 * @param scheduledTour
+	 * @param personId id of the driver (person)
+	 * @param carrier the carrier
+	 * @param scheduledTour the scheduledTour
 	 * @return CarrierTourStartEvent
 	 */
 	private CarrierTourStartEvent createFreightTourStartsEvent(Id<Person> personId, Carrier carrier, ScheduledTour scheduledTour) {

contribs/freight/src/main/java/org/matsim/freight/carriers/jsprit/NetworkBasedTransportCosts.java

@@ -56,7 +56,7 @@
  *
  * <p>
  * It can be used with multiple threads. Note that each thread gets its own
- * leastCostPathCalculator. It is created only once and cached afterwards. Thus,
+ * leastCostPathCalculator. It is created only once and cached afterward. Thus,
  * it requires a threadSafe leastCostPathCalculatorFactory (the calculator
  * itself does not need to be thread-safe).
  *
@@ -84,8 +84,6 @@
  */
 public class NetworkBasedTransportCosts implements VRPTransportCosts {
 
-	private final RoadPricingScheme roadPricingScheme;
-
 	public interface InternalLeastCostPathCalculatorListener {
 
 		void startCalculation(long routerId);
@@ -496,7 +494,7 @@ public Builder setRoadPricingScheme( RoadPricingScheme roadPricingScheme) {
 		 * </p>
 		 * Comments:
 		 * <ul>
-		 * <li>By default this will take free speed travel times.
+		 * <li>By default, this will take free speed travel times.
 		 * <li>yyyy These free speed travel times do <i>not</i> take the time-dependent
 		 * network into account. kai, jan'14
 		 * <li>Either can be changed with builder.setTravelTime(...) or with
@@ -577,7 +575,6 @@ private NetworkBasedTransportCosts(Builder builder) {
 		this.travelTime = builder.travelTime;
 		this.network = builder.network;
 		this.leastCostPathCalculatorFactory = builder.leastCostPathCalculatorFactory;
-		this.roadPricingScheme = builder.roadPricingScheme;
 		this.timeSliceWidth = builder.timeSliceWidth;
 		this.defaultTypeId = builder.defaultTypeId;
 		this.ttMemorizedCounter = new Counter("#TransportCostValues cached ");

contribs/freight/src/main/java/org/matsim/freight/carriers/jsprit/VRPTransportCosts.java

@@ -27,7 +27,7 @@
 import org.matsim.core.router.util.TravelTime;
 
 /**
- * @author: Steffen Axer
+ * @author Steffen Axer
  */
 public interface VRPTransportCosts extends VehicleRoutingTransportCosts {
     LeastCostPathCalculator getRouter();

contribs/freight/src/main/java/org/matsim/freight/carriers/usecases/analysis/CarrierScoreStats.java

@@ -80,7 +80,6 @@ public class CarrierScoreStats implements StartupListener, IterationEndsListener
 	 *
 	 * @param filename including the path, excluding the file type extension
 	 * @param createPNG true if in every iteration, the scorestats should be visualized in a graph and written to disk.
-	 * @throws UncheckedIOException
 	 */
 	public CarrierScoreStats(Carriers carriers, final String filename, final boolean createPNG) throws UncheckedIOException {
 		this.carriers = carriers;

contribs/freight/src/main/java/org/matsim/freight/carriers/usecases/analysis/LegHistogram.java

@@ -391,8 +391,7 @@ private int getBinIndex(final double time) {
 
 	private ModeData getDataForMode(final String legMode) {
 		// +1 for all times out of our range
-		ModeData modeData = this.data.computeIfAbsent(legMode, k -> new ModeData(this.nofBins + 1));
-		return modeData;
+		return this.data.computeIfAbsent(legMode, k -> new ModeData(this.nofBins + 1));
 	}
 
 	private static class ModeData {

contribs/freight/src/main/java/org/matsim/freight/carriers/usecases/chessboard/CarrierScoringFunctionFactoryImpl.java

@@ -186,9 +186,7 @@ public void handleLeg(Leg leg) {
 				Id<Vehicle> vehicleId = nRoute.getVehicleId();
 				CarrierVehicle vehicle = CarriersUtils.getCarrierVehicle(carrier, vehicleId);
 				Gbl.assertNotNull(vehicle);
-				if(!employedVehicles.contains(vehicle)){
-					employedVehicles.add(vehicle);
-				}
+				employedVehicles.add(vehicle);
 				double distance = 0.0;
 				if(leg.getRoute() instanceof NetworkRoute){
 					Link startLink = network.getLinks().get(leg.getRoute().getStartLinkId());

contribs/freight/src/main/java/org/matsim/freight/logistics/FreightLogisticsConfigGroup.java

@@ -62,7 +62,7 @@ public void setLspsFile(String lspsFile) {
 
 
     //---
-    // Commenting this out, because in a frist step I think it is better/ more streight forward to have the VRP logic in the carriers as an attribute.
+    // Commenting this out, because in a frist step I think it is better/ more straight forward to have the VRP logic in the carriers as an attribute.
     // please see {@link CarrierSchedulerUtils#setVrpLogic(carrier, VRPLogic)} and {@link CarrierSchedulerUtils#getVrpLogic(carrier)}
     //---
 

contribs/freight/src/main/java/org/matsim/freight/logistics/InitialShipmentAssigner.java

@@ -25,12 +25,12 @@
 /**
  * Takes an {@link LspShipment} and normally assigns it to something that belongs to an {@link LSP}.
  * <br>
- * After changes in fall 2023 (see master thesis of nrichter), the assingment is
+ * After changes in fall 2023 (see master thesis of nrichter), the assignment is
  * there to be done one time initially.
  * <br>
  * If there are several {@link LogisticChain}s in a {@link LSPPlan}, the {@link LSP} has to assign each {@link
  * LspShipment} to the suitable {@link LogisticChain}. For this purpose, each {@link LSPPlan}
- * (or only the LSP? - kmt'jan'24), contains a pluggable strategy
+ * (or only the LSP? - kmt jan'24), contains a pluggable strategy
  * that is contained in classes implementing the interface {@link InitialShipmentAssigner}. <br>
  * <br>
  * During iterations, it can happen that the {@link LspShipment} should be moved to another

contribs/freight/src/main/java/org/matsim/freight/logistics/LSPPlan.java

@@ -51,7 +51,7 @@ public interface LSPPlan extends BasicPlan, KnowsLSP {
 
   Collection<LspShipmentPlan> getShipmentPlans();
 
-  LSPPlan addShipmentPlan(LspShipmentPlan lspShipmentPlan);
+  void addShipmentPlan(LspShipmentPlan lspShipmentPlan);
 
   String getType();
 

contribs/freight/src/main/java/org/matsim/freight/logistics/LSPPlanImpl.java

@@ -67,9 +67,8 @@ public Collection<LspShipmentPlan> getShipmentPlans() {
   }
 
   @Override
-  public LSPPlan addShipmentPlan(LspShipmentPlan lspShipmentPlan) {
+  public void addShipmentPlan(LspShipmentPlan lspShipmentPlan) {
     this.lspShipmentPlans.add(lspShipmentPlan);
-    return null;
   }
 
   @Override

contribs/freight/src/main/java/org/matsim/freight/logistics/LSPUtils.java

@@ -50,7 +50,7 @@ public static boolean isPlanTheSelectedPlan(LSPPlan lspPlan) {
     return lspPlan.getLSP().getSelectedPlan() == lspPlan;
   }
 
-  public static LogisticChainScheduler createForwardLogisiticChainScheduler() {
+  public static LogisticChainScheduler createForwardLogisticChainScheduler() {
     return new ForwardLogisticChainSchedulerImpl();
   }
 

contribs/freight/src/main/java/org/matsim/freight/logistics/analysis/Driver2VehicleEventHandler.java

@@ -35,7 +35,7 @@
  * Basic event handler that collects the relation between vehicles and drivers.
  * Necessary since link enter and leave events do not contain the driver anymore.
  * <p>
- * This is the vice-versa implementation of {@link org.matsim.core.events.algorithms.Vehicle2DriverEventHandler}.
+ * This is the vice versa implementation of {@link org.matsim.core.events.algorithms.Vehicle2DriverEventHandler}.
  * <p>
  * In a first step only used internally. When needed more often, I have nothing against putting it more central. -> matsim-libs
  *

contribs/freight/src/main/java/org/matsim/freight/logistics/analysis/LspScoreStatsControlerListener.java

@@ -154,10 +154,10 @@ private void collectScoreInfo(final IterationEndsEvent event) {
 		}
 
 
-		log.info("-- avg. score of the executed plan of each agent: " + (info.sumExecutedScores / info.nofExecutedScores));
-		log.info("-- avg. score of the worst plan of each agent: " + (info.sumScoreWorst / info.nofScoreWorst));
-		log.info("-- avg. of the avg. plan score per agent: " + (info.sumAvgScores / info.nofAvgScores));
-		log.info("-- avg. score of the best plan of each agent: " + (info.sumScoreBest / info.nofScoreBest));
+		log.info("-- avg. score of the executed plan of each agent: {}", info.sumExecutedScores / info.nofExecutedScores);
+		log.info("-- avg. score of the worst plan of each agent: {}", info.sumScoreWorst / info.nofScoreWorst);
+		log.info("-- avg. of the avg. plan score per agent: {}", info.sumAvgScores / info.nofAvgScores);
+		log.info("-- avg. score of the best plan of each agent: {}", info.sumScoreBest / info.nofScoreBest);
 
 		try {
 			info.write(event.getIteration(), this.out, this.delimiter);
@@ -188,7 +188,7 @@ private void writePng() {
 	}
 
 	@Override
-	public void notifyShutdown(final ShutdownEvent controlerShudownEvent) {
+	public void notifyShutdown(final ShutdownEvent controlerShutdownEvent) {
 		try {
 			this.out.close();
 			for (ScoreHist data : this.perLsp.values()) {

contribs/freight/src/main/java/org/matsim/freight/logistics/events/AbstractLogisticEvent.java

@@ -41,7 +41,7 @@ public final Id<Link> getLinkId() {
   }
 
   /**
-   * Adds the {@link Id< LspShipment >} to the list of attributes. {@link Id<Link>} is handled by
+   * Adds the {@link Id<LspShipment>} to the list of attributes. {@link Id<Link>} is handled by
    * superclass {@link Event}
    *
    * @return The map of attributes

contribs/freight/src/main/java/org/matsim/freight/logistics/examples/initialPlans/ExampleSchedulingOfTransportChainHubsVsDirect.java

@@ -61,7 +61,7 @@
  * Part is removed, Chain is now starting at the CollectionHub
  *
  * <p>Scheduler = Macht die Pläne für die Fahrzeuge für die nächste MATSim-Iteration. Er plant es
- * für jede Ressource. --> jede Ressource hat einen eigenen Scheduler: 1.) Simple: Nimm die
+ * für jede Ressource. → jede Ressource hat einen eigenen Scheduler: 1.) Simple: Nimm die
  * mitgegebene Reihenfolge. 2.)
  */
 /*package-private*/ final class ExampleSchedulingOfTransportChainHubsVsDirect {
@@ -428,7 +428,7 @@ private static LSP createInitialLSP(Scenario scenario, SolutionType solutionType
         log.error(
             "This is totally untested. I can neither say if it will work nor if it will do anything useful - kmt feb22");
 
-        // TODO: Habe das vorziehen vor das switch statement rückgängig gemacht, weil es sideeffekte
+        // TODO: Habe das vorziehen vor das switch statement rückgängig gemacht, weil es sideeffects
         // hatte -> Die dürften hier auch sein!!!! (KMT may22)
         // Die createLSPPlan_reloading(..) Methoden sind nicht unabhängig voneinander.
         // Das liegt wohl am statischen und das dann dort wieder Verknüpfungen gesetzt werden -->

contribs/freight/src/main/java/org/matsim/freight/logistics/examples/initialPlans/MyEventBasedCarrierScorer.java

@@ -171,7 +171,7 @@ private void handleEvent(LinkEnterEvent event) {
    */
   class LinkBasedTollScoring implements SumScoringFunction.ArbitraryEventScoring {
 
-    final Logger log = LogManager.getLogger(EventBasedScoring.class);
+    final Logger log = LogManager.getLogger(LinkBasedTollScoring.class);
 
     private final double toll;
     private final List<String> vehicleTypesToBeTolled;

contribs/freight/src/main/java/org/matsim/freight/logistics/examples/lspReplanning/package-info.java

@@ -27,7 +27,7 @@
  *  This class here is in contrast used as a Replanning strategy. This behavior is not wanted anymore.
  *  <p></p>
  *  Please use the new approach as shown in
- *  {@link org.matsim.freight.logistics.example.lsp.multipleChains} ExampleMultipleOneEchelonChainsReplanning instead.
+ *  {@link org.matsim.freight.logistics.examples.multipleChains} ExampleMultipleOneEchelonChainsReplanning instead.
  *  ((The old approach is still available in CollectionLSPReplanningTest but will get removed earlier or later)).
  *  <p></p>
  *  KMT, Jul'24

contribs/freight/src/main/java/org/matsim/freight/logistics/examples/multipleChains/EventBasedCarrierScorer4MultipleChains.java

@@ -148,12 +148,12 @@ private void handleEvent(LinkEnterEvent event) {
    */
   class LinkBasedTollScoring implements SumScoringFunction.ArbitraryEventScoring {
 
-    final Logger log = LogManager.getLogger(EventBasedScoring.class);
+    final Logger log = LogManager.getLogger(LinkBasedTollScoring.class);
 
     private final double toll;
     private final List<String> vehicleTypesToBeTolled;
       private double score;
-    private List<String> tolledLinkList;
+    private final List<String> tolledLinkList;
     private final Vehicle2CarrierEventHandler v2c = new Vehicle2CarrierEventHandler();
 
     public LinkBasedTollScoring(double toll, List<String> vehicleTypeToBeTolled, List<String> tolledLinkListBerlin) {