From 3b5a64c989a17c00f90862a0f46dd5868857b644 Mon Sep 17 00:00:00 2001 From: oblonski Date: Thu, 30 Nov 2017 09:49:00 +0100 Subject: [PATCH] refine error messages --- .../core/problem/VehicleRoutingProblem.java | 23 +++++++++------ .../jsprit/core/problem/job/Job.java | 1 + .../jsprit/core/problem/job/Service.java | 16 +++++------ .../jsprit/core/problem/job/Shipment.java | 28 +++++++++++-------- .../core/problem/vehicle/VehicleTypeImpl.java | 14 ++++++---- 5 files changed, 48 insertions(+), 34 deletions(-) diff --git a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/VehicleRoutingProblem.java b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/VehicleRoutingProblem.java index ce3f9cad..ea5101dd 100644 --- a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/VehicleRoutingProblem.java +++ b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/VehicleRoutingProblem.java @@ -226,9 +226,15 @@ public class VehicleRoutingProblem { */ public Builder addJob(AbstractJob job) { if (tentativeJobs.containsKey(job.getId())) - throw new IllegalArgumentException("vehicle routing problem already contains a service or shipment with id " + job.getId() + ". make sure you use unique ids for all services and shipments"); + throw new IllegalArgumentException("The vehicle routing problem already contains a service or shipment with id " + job.getId() + ". Please make sure you use unique ids for all services and shipments."); if (!(job instanceof Service || job instanceof Shipment)) +<<<<<<< Upstream, based on branch 'master' of https://github.com/michalmac/jsprit.git throw new IllegalArgumentException("job must be either a service or a shipment"); +======= + throw new IllegalArgumentException("Job must be either a service or a shipment."); + job.setIndex(jobIndexCounter); + incJobIndexCounter(); +>>>>>>> b610626 refine error messages tentativeJobs.put(job.getId(), job); addLocationToTentativeLocations(job); return this; @@ -277,10 +283,11 @@ public class VehicleRoutingProblem { for (Vehicle v : uniqueVehicles) { if (v.getBreak() != null) { if (!uniqueBreakIds.add(v.getBreak().getId())) - throw new IllegalArgumentException("problem already contains a vehicle break with id " + v.getBreak().getId() + ". choose unique ids for each vehicle break."); + throw new IllegalArgumentException("The vehicle routing roblem already contains a vehicle break with id " + v.getBreak().getId() + ". Please choose unique ids for each vehicle break."); hasBreaks = true; List breakActivities = jobActivityFactory.createActivities(v.getBreak()); - if(breakActivities.isEmpty()) throw new IllegalArgumentException("at least one activity for break needs to be created by activityFactory"); + if (breakActivities.isEmpty()) + throw new IllegalArgumentException("At least one activity for break needs to be created by activityFactory."); for(AbstractActivity act : breakActivities){ act.setIndex(activityIndexCounter); incActivityIndexCounter(); @@ -343,7 +350,7 @@ public class VehicleRoutingProblem { private void addShipment(Shipment job) { if (jobs.containsKey(job.getId())) { - logger.warn("job " + job + " already in job list. overrides existing job."); + logger.warn("The job " + job + " has already been added to the job list. This overrides the existing job."); } addLocationToTentativeLocations(job); // tentative_coordinates.put(job.getPickupLocation().getId(), job.getPickupLocation().getCoordinate()); @@ -359,7 +366,7 @@ public class VehicleRoutingProblem { * */ public Builder addVehicle(Vehicle vehicle) { if (!(vehicle instanceof AbstractVehicle)) - throw new IllegalArgumentException("vehicle must be an AbstractVehicle"); + throw new IllegalArgumentException("A vehicle must be an AbstractVehicle."); return addVehicle((AbstractVehicle) vehicle); } @@ -371,7 +378,7 @@ public class VehicleRoutingProblem { */ public Builder addVehicle(AbstractVehicle vehicle) { if(addedVehicleIds.contains(vehicle.getId())){ - throw new IllegalArgumentException("problem already contains a vehicle with id " + vehicle.getId() + ". choose unique ids for each vehicle."); + throw new IllegalArgumentException("The vehicle routing problem already contains a vehicle with id " + vehicle.getId() + ". Please choose unique ids for each vehicle."); } else addedVehicleIds.add(vehicle.getId()); if (!uniqueVehicles.contains(vehicle)) { @@ -444,7 +451,7 @@ public class VehicleRoutingProblem { boolean hasBreaks = addBreaksToActivityMap(); if (hasBreaks && fleetSize.equals(FleetSize.INFINITE)) - throw new UnsupportedOperationException("breaks are not yet supported when dealing with infinite fleet. either set it to finite or omit breaks."); + throw new UnsupportedOperationException("Breaks are not yet supported when dealing with infinite fleet. Either set it to finite or omit breaks."); return new VehicleRoutingProblem(this); } @@ -511,7 +518,7 @@ public class VehicleRoutingProblem { // tentative_coordinates.put(service.getLocation().getId(), service.getLocation().getCoordinate()); addLocationToTentativeLocations(service); if (jobs.containsKey(service.getId())) { - logger.warn("service " + service + " already in job list. overrides existing job."); + logger.warn("The service " + service + " has already been added to job list. This overrides existing job."); } jobs.put(service.getId(), service); return this; diff --git a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/job/Job.java b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/job/Job.java index 55728235..76b65084 100644 --- a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/job/Job.java +++ b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/job/Job.java @@ -30,6 +30,7 @@ import com.graphhopper.jsprit.core.problem.Skills; */ public interface Job extends HasId, HasIndex { + /** * Returns the unique identifier (id) of a job. * diff --git a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/job/Service.java b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/job/Service.java index af4e6b85..1f4f436e 100644 --- a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/job/Service.java +++ b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/job/Service.java @@ -92,7 +92,7 @@ public class Service extends AbstractJob { protected Object userData; protected double maxTimeInVehicle = Double.MAX_VALUE; - + Builder(String id){ this.id = id; timeWindows = new TimeWindowsImpl(); @@ -135,7 +135,7 @@ public class Service extends AbstractJob { */ public Builder setServiceTime(double serviceTime) { if (serviceTime < 0) - throw new IllegalArgumentException("serviceTime must be greater than or equal to zero"); + throw new IllegalArgumentException("The service time of a service must be greater than or equal to zero."); this.serviceTime = serviceTime; return this; } @@ -167,20 +167,20 @@ public class Service extends AbstractJob { * @throws IllegalArgumentException if dimensionValue < 0 */ public Builder addSizeDimension(int dimensionIndex, int dimensionValue) { - if (dimensionValue < 0) throw new IllegalArgumentException("capacity value cannot be negative"); + if (dimensionValue < 0) throw new IllegalArgumentException("The capacity value must not be negative."); capacityBuilder.addDimension(dimensionIndex, dimensionValue); return this; } public Builder setTimeWindow(TimeWindow tw){ - if(tw == null) throw new IllegalArgumentException("time-window arg must not be null"); + if (tw == null) throw new IllegalArgumentException("The time window must not be null."); this.timeWindows = new TimeWindowsImpl(); timeWindows.add(tw); return this; } public Builder addTimeWindow(TimeWindow timeWindow) { - if(timeWindow == null) throw new IllegalArgumentException("time-window arg must not be null"); + if (timeWindow == null) throw new IllegalArgumentException("The time window must not be null."); if(!twAdded){ timeWindows = new TimeWindowsImpl(); twAdded = true; @@ -205,7 +205,7 @@ public class Service extends AbstractJob { * @throws IllegalArgumentException if neither locationId nor coordinate is set. */ public T build() { - if (location == null) throw new IllegalArgumentException("location is missing"); + if (location == null) throw new IllegalArgumentException("The location of service " + id + " is missing."); this.setType("service"); capacity = capacityBuilder.build(); skills = skillBuilder.build(); @@ -246,13 +246,13 @@ public class Service extends AbstractJob { */ public Builder setPriority(int priority) { if (priority < 1 || priority > 10) - throw new IllegalArgumentException("incorrect priority. only priority values from 1 to 10 are allowed where 1 = high and 10 is low"); + throw new IllegalArgumentException("The priority value is not valid. Only 1 (very high) to 10 (very low) are allowed."); this.priority = priority; return this; } public Builder setMaxTimeInVehicle(double maxTimeInVehicle){ - throw new UnsupportedOperationException("maxTimeInVehicle is not yet supported for Pickups and Services (only for Deliveries and Shipments)"); + throw new UnsupportedOperationException("The maximum time in vehicle is not yet supported for Pickups and Services (only for Deliveries and Shipments)."); // if(maxTimeInVehicle < 0) throw new IllegalArgumentException("maxTimeInVehicle should be positive"); // this.maxTimeInVehicle = maxTimeInVehicle; // return this; diff --git a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/job/Shipment.java b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/job/Shipment.java index a3eeaaf6..60085b3a 100644 --- a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/job/Shipment.java +++ b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/job/Shipment.java @@ -148,7 +148,8 @@ public class Shipment extends AbstractJob { * @throws IllegalArgumentException if servicTime < 0.0 */ public Builder setPickupServiceTime(double serviceTime) { - if (serviceTime < 0.0) throw new IllegalArgumentException("serviceTime must not be < 0.0"); + if (serviceTime < 0.0) + throw new IllegalArgumentException("The service time of a shipment must not be < 0.0."); this.pickupServiceTime = serviceTime; return this; } @@ -164,7 +165,7 @@ public class Shipment extends AbstractJob { * @throws IllegalArgumentException if timeWindow is null */ public Builder setPickupTimeWindow(TimeWindow timeWindow) { - if (timeWindow == null) throw new IllegalArgumentException("delivery time-window must not be null"); + if (timeWindow == null) throw new IllegalArgumentException("The delivery time window must not be null."); this.pickupTimeWindows = new TimeWindowsImpl(); this.pickupTimeWindows.add(timeWindow); return this; @@ -193,7 +194,8 @@ public class Shipment extends AbstractJob { * @throws IllegalArgumentException if serviceTime < 0.0 */ public Builder setDeliveryServiceTime(double deliveryServiceTime) { - if (deliveryServiceTime < 0.0) throw new IllegalArgumentException("deliveryServiceTime must not be < 0.0"); + if (deliveryServiceTime < 0.0) + throw new IllegalArgumentException("The service time of a delivery must not be < 0.0."); this.deliveryServiceTime = deliveryServiceTime; return this; } @@ -209,7 +211,7 @@ public class Shipment extends AbstractJob { * @throws IllegalArgumentException if timeWindow is null */ public Builder setDeliveryTimeWindow(TimeWindow timeWindow) { - if (timeWindow == null) throw new IllegalArgumentException("delivery time-window must not be null"); + if (timeWindow == null) throw new IllegalArgumentException("The delivery time window must not be null."); this.deliveryTimeWindows = new TimeWindowsImpl(); this.deliveryTimeWindows.add(timeWindow); return this; @@ -224,7 +226,8 @@ public class Shipment extends AbstractJob { * @throws IllegalArgumentException if dimVal < 0 */ public Builder addSizeDimension(int dimensionIndex, int dimensionValue) { - if (dimensionValue < 0) throw new IllegalArgumentException("capacity value cannot be negative"); + if (dimensionValue < 0) + throw new IllegalArgumentException("The capacity value must not be negative, but is " + dimensionValue + "."); capacityBuilder.addDimension(dimensionIndex, dimensionValue); return this; } @@ -245,8 +248,8 @@ public class Shipment extends AbstractJob { * is set */ public Shipment build() { - if (pickupLocation_ == null) throw new IllegalArgumentException("pickup location is missing"); - if (deliveryLocation_ == null) throw new IllegalArgumentException("delivery location is missing"); + if (pickupLocation_ == null) throw new IllegalArgumentException("The pickup location is missing."); + if (deliveryLocation_ == null) throw new IllegalArgumentException("The delivery location is missing."); capacity = capacityBuilder.build(); skills = skillBuilder.build(); return new Shipment(this); @@ -271,7 +274,7 @@ public class Shipment extends AbstractJob { } public Builder addDeliveryTimeWindow(TimeWindow timeWindow) { - if(timeWindow == null) throw new IllegalArgumentException("time-window arg must not be null"); + if (timeWindow == null) throw new IllegalArgumentException("The time window must not be null."); if(!deliveryTimeWindowAdded){ deliveryTimeWindows = new TimeWindowsImpl(); deliveryTimeWindowAdded = true; @@ -291,7 +294,7 @@ public class Shipment extends AbstractJob { } public Builder addPickupTimeWindow(TimeWindow timeWindow) { - if(timeWindow == null) throw new IllegalArgumentException("time-window arg must not be null"); + if (timeWindow == null) throw new IllegalArgumentException("The time window must not be null."); if(!pickupTimeWindowAdded){ pickupTimeWindows = new TimeWindowsImpl(); pickupTimeWindowAdded = true; @@ -319,7 +322,7 @@ public class Shipment extends AbstractJob { */ public Builder setPriority(int priority) { if (priority < 1 || priority > 10) - throw new IllegalArgumentException("incorrect priority. only 1 (very high) to 10 (very low) are allowed"); + throw new IllegalArgumentException("The priority value is not valid. Only 1 (very high) to 10 (very low) are allowed."); this.priority = priority; return this; } @@ -331,7 +334,8 @@ public class Shipment extends AbstractJob { * @return */ public Builder setMaxTimeInVehicle(double maxTimeInVehicle){ - if(maxTimeInVehicle < 0) throw new IllegalArgumentException("maxTimeInVehicle should be positive"); + if (maxTimeInVehicle < 0) + throw new IllegalArgumentException("The maximum time in vehicle must be positive."); this.maxTimeInVehicle = maxTimeInVehicle; return this; } @@ -436,7 +440,7 @@ public class Shipment extends AbstractJob { return pickupTimeWindows.getTimeWindows(); } - + /** * Returns a string with the shipment's attributes. *

diff --git a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/vehicle/VehicleTypeImpl.java b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/vehicle/VehicleTypeImpl.java index 3e8da61d..112c137c 100644 --- a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/vehicle/VehicleTypeImpl.java +++ b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/vehicle/VehicleTypeImpl.java @@ -151,7 +151,8 @@ public class VehicleTypeImpl implements VehicleType { * if velocity is smaller than zero */ public VehicleTypeImpl.Builder setMaxVelocity(double inMeterPerSeconds) { - if (inMeterPerSeconds < 0.0) throw new IllegalArgumentException("velocity cannot be smaller than zero"); + if (inMeterPerSeconds < 0.0) + throw new IllegalArgumentException("The velocity of a vehicle (type) cannot be smaller than zero."); this.maxVelo = inMeterPerSeconds; return this; } @@ -166,7 +167,7 @@ public class VehicleTypeImpl implements VehicleType { * @throws IllegalArgumentException if fixedCost is smaller than zero */ public VehicleTypeImpl.Builder setFixedCost(double fixedCost) { - if (fixedCost < 0.0) throw new IllegalArgumentException("fixed costs cannot be smaller than zero"); + if (fixedCost < 0.0) throw new IllegalArgumentException("Fixed costs must not be smaller than zero."); this.fixedCost = fixedCost; return this; } @@ -181,7 +182,8 @@ public class VehicleTypeImpl implements VehicleType { * @throws IllegalArgumentException if perDistance is smaller than zero */ public VehicleTypeImpl.Builder setCostPerDistance(double perDistance) { - if (perDistance < 0.0) throw new IllegalArgumentException("cost per distance must not be smaller than zero"); + if (perDistance < 0.0) + throw new IllegalArgumentException("Cost per distance must not be smaller than zero."); this.perDistance = perDistance; return this; } @@ -260,9 +262,9 @@ public class VehicleTypeImpl implements VehicleType { * @throws IllegalArgumentException if capacity dimension is already set */ public Builder addCapacityDimension(int dimIndex, int dimVal) { - if (dimVal < 0) throw new IllegalArgumentException("capacity value cannot be negative"); + if (dimVal < 0) throw new IllegalArgumentException("The capacity value must not be negative."); if (capacityDimensions != null) - throw new IllegalArgumentException("either build your dimension with build your dimensions with " + + throw new IllegalArgumentException("Either build your dimension with build your dimensions with " + "addCapacityDimension(int dimIndex, int dimVal) or set the already built dimensions with .setCapacityDimensions(Capacity capacity)." + "You used both methods."); dimensionAdded = true; @@ -283,7 +285,7 @@ public class VehicleTypeImpl implements VehicleType { */ public Builder setCapacityDimensions(Capacity capacity) { if (dimensionAdded) - throw new IllegalArgumentException("either build your dimension with build your dimensions with " + + throw new IllegalArgumentException("Either build your dimension with build your dimensions with " + "addCapacityDimension(int dimIndex, int dimVal) or set the already built dimensions with .setCapacityDimensions(Capacity capacity)." + "You used both methods."); this.capacityDimensions = capacity;