Add cost of egress leg to the cost of the last transit leg if transit leg arrives at destination#4547
Conversation
|
Could you fix the formatting issue by running EDIT: Seems that the docs are outdated, you can use the internal prettier plugin to format the code nowdays |
… leg arrives att destination
MikaelJarfors
force-pushed
the
egress-cost-on-final-transit-leg
branch
from
28fd74f to
6fe4a36
Compare
|
@hannesj The coding style should be fixed now - was just some whitespace. Have you had a look at the idea behind this? |
|
We discussed this in the dev meeting last week. The general idea behind this is good but @MikaelJarfors will create a better encapsulation so it's clearer what is happening and why. |
|
@hannesj and @leonardehrenfried I have encapsulated the zero duration logic and added a test for this specific functionality! |
Codecov ReportBase: 59.98% // Head: 60.46% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #4547 +/- ##
=============================================
+ Coverage 59.98% 60.46% +0.48%
- Complexity 11990 12116 +126
=============================================
Files 1562 1574 +12
Lines 62358 63279 +921
Branches 6986 7004 +18
=============================================
+ Hits 37405 38263 +858
- Misses 22794 22837 +43
- Partials 2159 2179 +20
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| } | ||
|
|
||
| @Override | ||
| public boolean stopReachedOnBoard() { |
There was a problem hiding this comment.
This method has another meaning, which we shouldn't override. It is used to signal that an access or egress departs onboard a vehicle (currently only flex), and those generally have a duration greater than zero. See Javadoc
There was a problem hiding this comment.
Ok - that is fine - I will make a new method for this functionality.
There was a problem hiding this comment.
To be honest with you Hannes - I read the java docs and thought it suited this perfectly. I will revise the Javadoc to make it closer to its actual use.
There was a problem hiding this comment.
Yes, in the egress case it isn't described correctly. It should say that if it is an access, it is true if the access arrives onboard a vehicle to the stop, and in case of egress if the egress leaves the stop onboard a vehicle. In this case there is no egress at all, since the destination is the stop in question.
| // leg given that it is the last leg. | ||
| int lastLegCost = 0; | ||
| PathLeg<T> nextLeg = pathLeg.nextLeg(); | ||
| if (nextLeg.isEgressLeg() && nextLeg.asEgressLeg().egress().stopReachedOnBoard()) { |
There was a problem hiding this comment.
Rather than checking if the last leg was reached on board, would it not make sense to somehow check that the alight penalty has been shifted from the transit leg to the egress one?
There was a problem hiding this comment.
This check is needed in order to choose whether to shift the cost from one leg to the other - so it is difficult to do this the other way around.
Have I interpreted you correctly?
There was a problem hiding this comment.
My question is: do we only want to shift the cost back only if the egress leg is empty/zero duration?
Could there not be an egress leg with a short walking duration, say at a cost of 100, that still has a -3000 penalty, so we end up with a leg with a cost of -2900?
leonardehrenfried
changed the title
6 participants
Closes #4546