diff --git a/.travis.yml b/.travis.yml index 373829e3..3d221ae1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,7 @@ matrix: fast_finish: true include: - jdk: oraclejdk8 + dist: trusty # Java 9 needs to be manually installed/upgraded # see: https://github.com/travis-ci/travis-ci/issues/2968#issuecomment-149164058 - jdk: oraclejdk9 diff --git a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/algorithm/recreate/BreakScheduling.java b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/algorithm/recreate/BreakScheduling.java index 66fbb182..a64b8154 100644 --- a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/algorithm/recreate/BreakScheduling.java +++ b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/algorithm/recreate/BreakScheduling.java @@ -45,8 +45,6 @@ public class BreakScheduling implements InsertionStartsListener,JobInsertedListe private final EventListeners eventListeners; - private Set modifiedRoutes = new HashSet(); - public BreakScheduling(VehicleRoutingProblem vrp, StateManager stateManager, ConstraintManager constraintManager) { this.stateManager = stateManager; this.breakInsertionCalculator = new BreakInsertionCalculator(vrp.getTransportCosts(), vrp.getActivityCosts(), new LocalActivityInsertionCostsCalculator(vrp.getTransportCosts(), vrp.getActivityCosts(), stateManager), constraintManager, vrp.getJobActivityFactory()); @@ -78,7 +76,6 @@ public class BreakScheduling implements InsertionStartsListener,JobInsertedListe @Override public void ruinStarts(Collection routes) { - } @Override @@ -88,7 +85,7 @@ public class BreakScheduling implements InsertionStartsListener,JobInsertedListe boolean removed = route.getTourActivities().removeJob(aBreak); if(removed) logger.trace("ruin: {}", aBreak.getId()); } - List breaks = new ArrayList(); + List breaks = new ArrayList<>(); for (Job j : unassignedJobs) { if (j instanceof Break) { breaks.add((Break) j); @@ -99,7 +96,6 @@ public class BreakScheduling implements InsertionStartsListener,JobInsertedListe @Override public void removed(Job job, VehicleRoute fromRoute) { - if(fromRoute.getVehicle().getBreak() != null) modifiedRoutes.add(fromRoute); } @Override @@ -119,6 +115,5 @@ public class BreakScheduling implements InsertionStartsListener,JobInsertedListe } } } - } } diff --git a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java index 9730de76..22b9db33 100644 --- a/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java +++ b/jsprit-core/src/main/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TourActivities.java @@ -81,7 +81,6 @@ public class TourActivities { } public TourActivities() { - } public List getActivities() { @@ -89,7 +88,30 @@ public class TourActivities { } public Iterator iterator() { - return tourActivities.iterator(); + final Iterator iterator = tourActivities.iterator(); + return new Iterator() { + private TourActivity lastReturned = null; + + @Override + public boolean hasNext() { + return iterator.hasNext(); + } + + @Override + public TourActivity next() { + return lastReturned = iterator.next(); + } + + @Override + public void remove() { + if (lastReturned instanceof JobActivity) { + throw new IllegalStateException("Cannot remove JobActivities via iterator. " + + "Use TourActivities.removeActivity(), or alternatively, consider TourActivities.removeJob()"); + } else { + iterator.remove(); + } + } + }; } public boolean isEmpty() { @@ -154,26 +176,35 @@ public class TourActivities { * @return true if activity has been removed, false otherwise */ public boolean removeActivity(TourActivity activity) { - Job job = null; - if (activity instanceof JobActivity) { - job = ((JobActivity) activity).getJob(); + if (!(activity instanceof JobActivity)) { + //assumes that an activity can be added only once to tourActivities + return tourActivities.remove(activity); } + + Job job = ((JobActivity) activity).getJob(); boolean jobIsAlsoAssociateToOtherActs = false; boolean actRemoved = false; - List acts = new ArrayList<>(tourActivities); - for (TourActivity act : acts) { + for (TourActivity act : new ArrayList<>(tourActivities)) { if (act == activity) { tourActivities.remove(act); + if (jobIsAlsoAssociateToOtherActs) { + // other activities also refer to job --> do not remove job + // thus no need to iterate any further + return true; + } actRemoved = true; } else { - if (act instanceof JobActivity && job != null) { - if (((JobActivity) act).getJob().equals(job)) { - jobIsAlsoAssociateToOtherActs = true; + if (act instanceof JobActivity && ((JobActivity) act).getJob().equals(job)) { + if (actRemoved) { + // other activities also refer to job --> do not remove job + // thus no need to iterate any further + return true; } + jobIsAlsoAssociateToOtherActs = true; } } } - if (!jobIsAlsoAssociateToOtherActs && actRemoved) { + if (actRemoved) { jobs.remove(job); } return actRemoved; diff --git a/jsprit-core/src/test/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TestTourActivities.java b/jsprit-core/src/test/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TestTourActivities.java index 51fac31b..6604bf2e 100644 --- a/jsprit-core/src/test/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TestTourActivities.java +++ b/jsprit-core/src/test/java/com/graphhopper/jsprit/core/problem/solution/route/activity/TestTourActivities.java @@ -22,6 +22,7 @@ import com.graphhopper.jsprit.core.problem.job.Service; import com.graphhopper.jsprit.core.problem.job.Shipment; import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; import static org.junit.Assert.*; @@ -118,6 +119,19 @@ public class TestTourActivities { assertEquals(0, tour.getActivities().size()); } + @Test + public void removingNonJobActivityShouldWork() { + TourActivity nonJobAct = Mockito.mock(TourActivity.class); + + tour.addActivity(nonJobAct); + assertTrue(tour.getActivities().contains(nonJobAct)); + + tour.removeActivity(nonJobAct); + + assertTrue(tour.isEmpty()); + assertFalse(tour.getActivities().contains(nonJobAct)); + } + @Test public void removingActivityShouldWork() { tour.addActivity(act);