Remove walk leg in a stay seated transfer

[optionsome]

Summary

Removes walk between two trips that have a stay seated transfer. OTP1 did not have a walk leg in an interline transfer.

Issue

closes #5086

Unit tests

TODO

Documentation

Not needed

Changelog

From title

[optionsome]

There is an issue that the walk leg has a cost that affects the total cost of the itinerary but if we remove the walk leg, the cost is no longer attached to any leg in the end results and the cost of the walk leg might affect results. Also, I'm not sure if a transfer is blocked because the user wouldn't be able to walk in time between two stops (even though that is not really needed).

[codecov]

Codecov Report

Patch coverage: 86.76% and project coverage change: +0.55 🎉

Comparison is base (b157ce2) 64.78% compared to head (6d1a12d) 65.34%.

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #5135      +/-   ##
=============================================
+ Coverage      64.78%   65.34%   +0.55%     
- Complexity     14164    14421     +257     
=============================================
  Files           1733     1751      +18     
  Lines          67502    67862     +360     
  Branches        7223     7221       -2     
=============================================
+ Hits           43730    44343     +613     
+ Misses         21344    21057     -287     
- Partials        2428     2462      +34     

Impacted Files	Coverage Δ
...algorithm/mapping/RaptorPathToItineraryMapper.java	72.34% <50.00%> (-1.39%)	⬇️
...graphqlapi/datafetchers/LegacyGraphQLStopImpl.java	17.07% <66.66%> (+4.04%)	⬆️
.../org/opentripplanner/ext/geocoder/LuceneIndex.java	68.57% <87.50%> (+1.14%)	⬆️
.../org/opentripplanner/ext/geocoder/StopCluster.java	100.00% <100.00%> (ø)
...pentripplanner/ext/geocoder/StopClusterMapper.java	96.77% <100.00%> (ø)
...planner/transit/service/DefaultTransitService.java	47.61% <100.00%> (+5.08%)	⬆️

... and 168 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[optionsome]

We discussed this pr today in the dev meeting and whether tests should be written for it in the scope of this pr. @t2gran thought it would make sense to do some refactoring so we can write better tests outside of this pr.

[leonardehrenfried]

Please document why this is necessary and perhaps link to the relevant tickets.

[leonardehrenfried]

Please document.

[t2gran]

Sorry, but @leonardehrenfried comment triggered my review. This is a bit detailed - maybe too much, but to be clean I would do it like the suggested code.

If you use // to document a business feature, it is probably better to extract a method with a name witch hints to what you try to achieve, and write JavaDoc for details. This does not interrupt the reader of the main code, and let hen to go and read the detail if hen want.

[t2gran]

I would put the doc on the method as JavaDoc and name the method according to its intention. Also, I would code the if like this keeping the same abstraction level in each outer block:

if (pathLeg.isTransitLeg()) {
  ...
}
else if (pathLeg.isTransferLeg() {
  if(includeTransferInItinerary(transitLeg)) {
    ...
  }
}

The following else is implicit, but not included, because it is considered noice and unlikely to happen, if
we expected more leg types to be added later it would make sense to add it.

else {
  throw IllegalStateException("Unexpected leg type: " + pathLeg);
}

[optionsome]

Good points, I have updated the code.