I am migrating from 0.11 to 1.0. Generally nice progress, really appreciate your work, good idea with enums and encoded values.
I have one request - please, think up encapsulation policy.
For example, if someone wants to provide his own algorithm factory which uses landmarks approximation… it is impossible to do it clearly:
- you can make your class in package
LandmarkStorageis package private
- you can also make a workaround like use
LMRoutingAlgorithmFactory, then make dummy algorithm like
AstarBiand then extract resulting approximator from this algorithm.
I have noticed that in some places, where delegation was prefered, it dissapeared. For example in all places, except node based ch, weighting is wrapped in graph weighting. It is ok that it is wrapping, but… what is it actually doing? (also please notice https://github.com/graphhopper/graphhopper/blob/a5b48ceadc6b3973659075233687dd8991748d87/core/src/main/java/com/graphhopper/routing/ch/CHRoutingAlgorithmFactory.java#L58
getWeighting call and this bellow).
Not sure about
TurnCostWeighting inlining in
AbstractBidirCHAlgo but taking into account graph bellow and that node based routing cannot provide turn cost…https://github.com/graphhopper/graphhopper/blob/a5b48ceadc6b3973659075233687dd8991748d87/core/src/main/java/com/graphhopper/routing/AbstractBidirCHAlgo.java#L211
And the last - no SRP in fastest weighting, different approach in shortest. I will reference to my issue https://github.com/graphhopper/graphhopper/issues/1951.
You guys don’t have to take my words into account, but please consider that some users are not only using you service - also low level API, like me, which I am migrating for recent 2 weeks already, and long way ahead. Some changes are good, but others… would like to avoid.
Thanks for any response, maybe counterarguments or critism, and regards