Conversation
benedikt-brandtner-bikemap
left a comment
benedikt-brandtner-bikemap
left a comment
There was a problem hiding this comment.
The parser looks very nice maybe we can even use this on android as well 👍
Just one comment as i don't get why it was added
| return true; | ||
| } | ||
| } | ||
| } else if (n.identifiers.length == 1 && mi.arguments.length == 0 && variableValidator.isValid(n.identifiers[0])) { |
There was a problem hiding this comment.
Why was this added it seems like this tries to support a method invocation pattern?
There was a problem hiding this comment.
Codex:
I added that branch in core/src/main/java/com/graphhopper/routing/weighting/custom/ConditionalExpressionVisitor.java:108 because, while testing the refactored custom-model caching, Janino sometimes reports the target of an ordinal() call as a single identifier. For expressions like track_type.ordinal() in the built-in models (e.g. motorcycle.json), mi.target.toRvalue() now yields an AmbiguousName with length 1, so our earlier logic never recorded track_type as a used variable and the parser rejected the condition (“… not available”). The new else if just mirrors the existing road_class.ordinal() case for that AST shape so we still accept the expression and we add the variable to guessedVariables, ensuring the enum accessor is created when the model is re-used from the cache.
There was a problem hiding this comment.
For me it's a quick fix for .ordinal() operation. I'm not sure if we use it. But might be useful in future:
change is not specific to track_type; it kicks in whenever Janino gives us a zero-argument method call whose target reduces to a single identifier that the validator accepts. So any bike-oriented custom model that ever does something like bike_network.ordinal() (or a similar accessor on a bike-specific encoded value) now stays valid and keeps the variable in guessedVariables, ensuring the lookup/accessor is built correctly. Even though the shipped bike models don’t currently rely on .ordinal(), they’re covered automatically if we add such expressions later.
There was a problem hiding this comment.
The fact that core tests are passing without this addition kind of confirms the statement.
There was a problem hiding this comment.
Hmm alright sounds good to me
2 participants
AI (codex) refactored
static CustomWeighting.Parameters createWeightingParametersto avoid instantiation of customModel Java class in runtime. Instead class is parsed and parameters are built based on restored logic.