Extending GH is sometimes made hard through private methods, could we make them protected?

Hello,

I’m currently working on switching my application (trailrouter.com) to use mainline GraphHopper. My hope is to end up with a small shim layer over GH, with just my extra additional classes (mostly weightings and extra storages), and then include an unmodified GH 2.3 JAR file as a library.

This is working pretty well so far, but I’m finding that some classes in GH are quite hard to extend due methods being marked as private rather than protected.

Would a PR to change specific instances of private methods to protected (with examples for why it is useful) be welcome?

I’ll give you an example: I’d like to add a feature for avoiding repetition in running routes. So if you have multiple legs in your route, then we should try not to cover the same edges twice. GH already has a nice class called AvoidEdgesWeighting that is used in round trip mode. I’d like to use this in normal (“Via”) routing mode. To make this work, I’d need to add all edges from the previous leg to the AvoidEdgesWeighting before the path for the next leg is calculated. This seems possible in Router.routeVia, specifically by overriding createPathCalculator and returning a custom path calculator that also injects the edges into the AvoidEdgesWeighting inside algoOpts. However, createPathCalculator is private, so we end up overriding routeVia instead, which in turn requires a whole bunch of other things to be duplicated/overridden too. If createPathCalculator was marked as protected then I could trim hundreds of lines of duplicated code.

Thanks,

Sam

No, we probably won’t accept such a PR, mostly because we do not want to take the burden to support certain APIs in the future. However, you can very easily work around this: Just fork GH, then change whatever you like from private to public and then use this modified version instead of the original GH jar.
See also this discussion: https://github.com/graphhopper/graphhopper/pull/2106#issuecomment-688914649

Powered by Discourse