Draft: DRT consolidate prebookings and group bookings #2991
Conversation
…BookingConsolidation
|
This PR aims to consolidate prebookings and group bookings and removes the additional group passenger engine by merging the functionality in the default passenger engine. The recently added sharing metrics now also work with the same idea of group bookings. Please let me know what you think. |
|
Very nice! I just had a very brief look, will check more closely later on. One first comment: would it maybe make sense to have something like a |
| List<Id<Person>> personIds = new ArrayList<>(); | ||
| for (String person : personIdsAttribute) { | ||
| personIds.add(Id.create(person, Person.class)); | ||
| } |
There was a problem hiding this comment.
Why not to use stream as you did in PrebookingManager L163?
| import com.google.inject.Key; | ||
| import com.google.inject.Singleton; | ||
| import com.google.inject.name.Names; |
There was a problem hiding this comment.
General note (not limited to this PR): This PR contains lots of line changes due to different formatting standards, especially imports reordering/wildcarding. There are, for instance, no other changes in this file.
I hope this will get reduced after we switch to a formatting standard.
There was a problem hiding this comment.
Yes, I see. Guess it's from my auto-formatting in IntelliJ...
Replaced the tuples with "PersonLeg" records. Booking doesn't really fit here I guess as it's really just combinations of agents and a specific leg that are not yet grouped together as a booking |
contribs/drt/src/main/java/org/matsim/contrib/drt/prebooking/PrebookingManager.java
Show resolved
Hide resolved
|
HI @nkuehnel, just added one comment, otherwise this looks nice! Thanks a lot! |
…BookingConsolidation # Conflicts: # contribs/drt/src/test/java/org/matsim/contrib/drt/sharingmetrics/SharingFactorTest.java # contribs/dvrp/src/test/java/org/matsim/contrib/dvrp/passenger/PassengerGroupTest.java
|
@sebhoerl done, thanks for your input |
No description provided.