Inject Isochrone class in IsochroneResource?

I recently tried to extend the Isochrone class when using GraphHopper as a lib, Unfortunately, this is not easily possible, because you cannot change the used implementation of the Isochrone class.

How about we also inject the Isochrone class in IsocchroneResource?

Can we not extract most of the code from IsochroneResource and put it into GraphHopper instead, just like we do for RouteResource? The idea would be that GraphHopper can also be used standalone without the (web)resource stuff. In a next step we should then extract the isochrone (and the route) stuff from GraphHopper (but GraphHopper would still use this). For route() I already did this here (but need to do it again due to recent changes): https://github.com/graphhopper/graphhopper/pull/1901

And of course we can then make the Isochrone creation (or whatever you need to adjust configurable somehow in the code that does the actual isochrone calculation)…

Yes, and I think we should do this as well. There is too much business logic in the IsochroneResource IMHO.

Yes, that makes sense as well.

If we move the business logic from the IsochroneResource to GraphHopper, this should be solved automatically. The GraphHopper class is already injected.

Should I create a PR to move the business logic of the IsochroneResource into GraphHopper?

I would prefer to have this after PRs #1901 / #1902 as this is already not that easy to do.

Ok sure, that makes sense. In the meantime, should we at least Inject the Isochrone class so that the Isochrone logic can be customized?

Ok, sure.

1 Like
Powered by Discourse