introduces under_score notation for all properties, fixes #682

[karussell]

This change fixes #682 and requires a re-import as not only the storable properties are changed but also the EncodingManager properties like turnCosts -> turn_costs

Done:

• under score parameters are now default for get and put in PMap
• keep 'routing.' prefix for defaults specified in config.properties, on query the 'routing.' is not allowed (no fallback yet)

Some backward breaking changes (no fallback for now):

• The biggest change is that we do no longer handle upper case as before e.g. stuff like this will fail now assertEquals("valueA", subject.get("FOO", ""));. The reason is that camelCase still works and is automatically converted to under_score
• osmreader.preferred-language -> osmreader.preferred_language
• prepare.contracted-nodes -> prepare.contracted_nodes
• prepare.logmessages -> prepare.log_messages
• osmreader.way_point_max_distance -> routing.way_point_max_distance

TODOs:

• pass correct properties to measurement -> graphhopper.sh script (measurement.git_info, graph.import_time, graph.size_in_MB, location_index, etc) and config-example.properties
• update measurement JS
• update I18N keys and access to those key in the JS UI
• change constants versions
• routing.roundTrip.maxRetries
• documentation e.g. about routing etc
• add fallback for graph.elevation.*
  • graph.elevation.calcmean <- graph.elevation.calc_mean
  • graph.elevation.cachedir <- graph.elevation.cache_dir
  • graph.elevation.baseurl <- graph.elevation.base_url

[devemux86]

It appears we cannot avoid the backwards compatibility break (some will complain), but it's for better library organization.

[devemux86]

My suggestion is to group all these fixed strings into a central public class e.g. GHConstants, instead of being hard coded all over.

• That way is less prone to errors, as they'll be maintained only in one place
• And those using them from outside can access them safely, without worrying for typos or changes

[karussell]

Not sure about this constant class as this tightly coupling is only possible if used from a java client (also probably the name should be (GH)Parameters as we already have Constants).

Renaming of those keys should not happen often as this breaks usage from 'outside'. Or would a more context specific solution like currently done very seldom for e.g. CHAlgoFactoryDecorator.DISABLE or AlgorithmOptions.DIJKSTRA make more sense?

If not context sensitive the name would be ugly long like Parameters.ALGORITHM_DIJKSTRA etc

[devemux86]

Or would a more context specific solution like currently done very seldom for e.g. CHAlgoFactoryDecorator.DISABLE or AlgorithmOptions.DIJKSTRA make more sense?

Yes that could work too.
Still I'm talking about all fixed properties / parameters to be grouped in such a way. It'll be very helpful inside GH and from outside.

[karussell]

What came to my mind is to use embedded classes:

public class Parameters {
   public static class Algorithms {
       public static String DIJKSTRA="dijkstra";
   }
}

Then one could import e.g. Parameters.Algorithms and just use DIJKSTRA

[devemux86]

I'd vote for that, good organization.

[karussell]

The advantage of having all in one place is also a good overview / documentation of features.

[karussell]

Please see this new class 9fa4441#diff-c6031a04cf6bb054003314f01e9a4239R24

Maybe we should improve the handling for "on query parameters" vs. "init parameters". E.g. max_visited_nodes vs. init.max_visited_nodes or ch.disable vs. init.ch.disabling_allowed.

To help migration we could throw an exception (can be disabled) if parameters are passed to GH on init and are not listed in the Parameters class.

[devemux86]

Nice, I like the constants synthesis in the class too.
There are still strings to be added right?

[karussell]

There are still strings to be added right?

Yes, although these should be only "init parameters" (should we move them there too?) or 'private' parameters (I would keep them private as there are e.g. implementation specific)

[devemux86]

Inner classes can work, unless we have at the end a big amount of strings to handle.

'private' parameters (I would keep them private)

e.g. how you put the graph properties?

[karussell]

Inner classes can work, unless we have at the end a big amount of strings to handle.

Yeah, I'm not sure if some of the properties should be moved e.g. into the algorithms.

'private' parameters (I would keep them private)

I more meant params like alternative_route.min_plateau_factor which could be changed for other alternative route implementations. The graph.* and prepare.* params are what I mean with 'init parameters' and could be moved into the class as well - maybe not that important for now (?) and would let it for another issue (?)

[devemux86]

Probably a separate issue about hosting all fixed strings - regardless category - in such class(es) would be best and keep this for the notation?

[karussell]

This is merged now.

[devemux86]

Another thought, do you think the strings should be sorted in the class for better reviewing?

[karussell]

Do you mean the Parameters class and what do you mean with better reviewing?

[devemux86]

Yes in that. Like you mentioned above it's a good overview / documentation of features. So I'm wondering if the various properties should be better sorted alphabetically in each sub-class, e.g. here.

[karussell]

Ah, yes. Makes sense for the future, also for the static classes. For now the setting number is small enough also hopefully we can create enough static classes to keep the number small.