Introduce generalizedCostPlusPenalty to make cost comparsion fairer#5483
Conversation
leonardehrenfried
added
the
!Improvement
leonardehrenfried
force-pushed
the
subtract-penalty
branch
from
6cd0d6e to
441af5d
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #5483 +/- ##
==========================================
Coverage 67.53% 67.53%
- Complexity 16282 16285 +3
==========================================
Files 1887 1886 -1
Lines 71583 71588 +5
Branches 7408 7408
==========================================
+ Hits 48341 48350 +9
+ Misses 20740 20736 -4
Partials 2502 2502 ☔ View full report in Codecov by Sentry. |
|
@t2gran this is now ready for review again. I've tested the P+R case but we agreed a while ago that we will go through it together to check which filters need to use the version with and without penalty. Let me know when you're free. |
leonardehrenfried
changed the title
generalizedCostWithPenalty to make cost comparsion fairergeneralizedCostPlusPenalty to make cost comparsion fairer
leonardehrenfried
force-pushed
the
subtract-penalty
branch
from
6d7a478 to
b766ff9
Compare
|
@t2gran went through this today and identified the cases in the filter chain where we want to compare with and without penalty. This is now ready for review again. |
| if (penalty == null) { | ||
| return 0; | ||
| } else { | ||
| return penalty.cost().toSeconds(); | ||
| } |
There was a problem hiding this comment.
| if (penalty == null) { | |
| return 0; | |
| } else { | |
| return penalty.cost().toSeconds(); | |
| } | |
| return (penalty == null ? Cost.ZERO : penalty.cost()).toSeconds(); | |
| } |
Avoid magic numbers: 0
There was a problem hiding this comment.
A better way to do this is to make the penalty none null, and initialize it to TimeAndCost.ZERO. I do not think we need to differensiate between ZERO and NOT_SET.
There was a problem hiding this comment.
I replaced this earlier using the SortOrderComparator#generalizedCostComparator() - forgot to delete the class, so good to se this go :-)
…n/filters/transit/TransitGeneralizedCostFilter.java Co-authored-by: Thomas Gran <t2gran@gmail.com>
3 participants
Summary
This introduces a new method to
Itinerarywhich allows you to get the cost but also subtract the access/egress penalty.This means that street-only (direct) results don't have an unfair advantage over those that combine access/egress with transit.
Issue
Relates to #5381 #5337
Unit tests
Added.
Documentation
Javadoc added.