Version 2 migration continues

[balage1551]

I created this pull request to inform you about my progress. You may merge or may wait.

What has been acomplished:

• BreakActivity kept its single time window function, but with the name getBreakTimeWindow()
• All other getSingleTimeWindow() reference and the function itselt removed. Note, that not all single time window references are removed, In tests, where they sole function was to get a time window to assert, I kept it by replacing with the getTimeWindows().iterator().next() expression.
• The impl_setIndex() methods now protected by friendly handshake pattern. They are still public, but the user can't call it.
• In the VehicleRoute.Builder, I created new activity add functions fitting better to the new job architectures. The old ones (addService, addPickup, etc.) are kept (used by the unit tests intensly), but marked deprecated.
• In the VehicleRoute.Builder, the half-inserted shipments were kept into account, but no validation were made for the more general, multi-activity CustomJobs. It is replaced with new validation which detects if all activities of all added jobs are really inserted and if any activities are inserted more than once.

What is next:

• Add Javadoc for the new acticity adding functions of the VehicleRoute.Builder.
• Get rid of the JobActivityFactory
• Debug, why the modular objective function returns wrong cost sometimes.

[balage1551]

I've run into a problem, which we have to solve, but not easy.
I've tried to remove JobActivityFactory implementations and references, because now Activities are created and owned by the job. There is two main implemenation of this interface: a simple and a copy one. The simple could be mapped to the activity.getActivityList().getAll() function call and its ok. The problem is with the other one: the copier version. It could be mapped to the activity.getActivityList().getAllDuplicated() function, too.

First it looks ok, but it is wrong! It hold a problem, which could cause problems hard to detect.

Because the activity is now belongs to the job, there is a stronger bond between the job and its activities. Even more importantly, this bond is two way: the job knows its activities, and the activity knows its job. Now imagine, that you copied the activities with the `getAllDuplicated()' function. You have a new set of objects and they point to the job. When you access the job from the activity somewhere in your code, and then from the job you query the other activities, they would point to an other set of activities: to the original ones.

What can be done?

This is really tricky. First I have to fully understand all the reasons, why we need copies of the activities. One of the problem is, that the activity -- in contrast with most of the other configuration classes -- is not immutable. It holds both configuration AND runtime (optimization) information.

One of the possible solutions to use state pattern: separate the runtime and configuration data. The job would only contain the configuration data and the runtime data would refer to the cofig it belongs to. The copy would only copy the runtime part. The route would hold the runtime data instead of holding the config activity directly. I checked, that these runtime values are set only by a few classes, so the modification may be less hard. Although, this would not completely solve the activity->job->activity reference problem, but one copy would not reach the runtime data of an other copy.

The other solution is to restore the old way: the job is lightly connected to its activities and the factories a revived, but I'm not sure we can simply do this, because now only the job knows which are its activities.

This is a problem, we should find solution for!

[oblonski]

I d really love to make activities immutable. The reason they are not is that I also wanted the most important states easily accessible. However, one can differentiate between a JobActivity which should be immutable and a TourActivity and a TourActivity is nothing else than a JobActivity assigned to a tour/vehicle-route (here naming is also not 100% intuitive). Thus, we might design it such that a TourActivity is a wrapper of JobActivity then we only need to copy TourActivities if we create new tours, but do not need to copy its immutable JobActivity. What do you think?

[balage1551]

I think the same. It would be advisable to make the change. There are two ways to do wrapper: delegation or composition. Each has its benefits and drawbacks.

With wrapper pattern, we could make the extra layer transparent, but have to delegate all the methods of the wrapped class which works only if all the wrappable classes has the same API (same set of public functions). This would be painful generally, but in the case of Activity, it is simpler because the descendants of the AbstractActivity is a limited set and not meant to be extended, and the APIs of these activities are very similar. But even with this similarity, we will have to make some delegate functions with UnsupportedOperation exception.

With composition, to access the immutable part (the original Activity) would need one more call in the chain (for example, activity.getTimeWindows() would become a routeActivity.getActivity().getTimeWindows()), but would offer much flexibility on different APIs and doesn't prolong an invalid call to run-time exception.

There is a third option (and I propose this): the mix of the above two. Let it be composition and delegate only the common methods. To access the most important methods would remain transparent, but would allow to access the activity type specific methods with the long form.

The TourActivity is on the top of the hierarchy, so it is not good for either version. I won't touch that (except by removing mutable part) to keep backward compatibility, and I would introduce a new class: RouteActivity or ActivityState. This would follow the mixed pattern described above and the VehicleRoure would store the list of this.

Also, I would add a search method for VehicleRoute to find the wrapper on the route of the imutable part. This would allow to access the runtime info of an other activity on the same route (the act->job->act problem).

I would gladly do this refactor, its something interesting and challenging. The question is: how urgent is this? I am on holiday till 21th, but I have all I need to do it with me, so I could find time to make it (when the children are sleeping, for example).

[oblonski]

@balage1551 Thanks for your suggestion. Sounds very good :). I d be happy if you could do it. It is not that urgent. Happy holiday.