Accept pre-parsed expressions in functions that evaluate expressions #171
Conversation
|
This pull request has been linked to Shortcut Story #26590: Cache Trigger Expressions for faster evaluation. |
| def parse!(expression) when is_number(expression), do: to_string(expression) |> parse!() | ||
|
|
||
| def parse!(expression_ast) when is_list(expression_ast), do: expression_ast |
There was a problem hiding this comment.
Is there a better way to match Expressions ASTs?
There was a problem hiding this comment.
Perhaps you can write a defguard function to make sure the given payload contains an :expression attribute and the expression payload is valid.
There was a problem hiding this comment.
Otherwise, it can be possible to just give a random list such as ["hola", "mundo"] which is not valid.
There was a problem hiding this comment.
Good idea @santiagocardo! Will do tomorrow!
test/expression_test.exs
Outdated
| # If the expression is a already provided as a parsed AST then it is simply | ||
| # returned as is | ||
| assert [expression: [attribute: [atom: "contact", atom: "age"]], text: " + 2"] == | ||
| Expression.parse!( |
There was a problem hiding this comment.
supplying AST (parsed structure) to a function called parse! seems a bit odd to be honest, what problem are we solving here?
There was a problem hiding this comment.
This is just because this function is called from inside every evaluate_* function, so it was easier to add this check here than adding it to every evaluate_* function, but I could also do that. I agree that it feels odd.
As per the PR description the goal here is to allow the library user to cache/store AST representations of commonly evaluated expressions rather than forcing the library user to supply the expression as a string (and waste time parsing it again) at every evaluation.
|
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
|
Closing for now due to inactivity, sorry @fedme ! |
Accept pre-parsed expressions (Expressions ASTs instead of strings) in functions that evaluate expressions.
This allows the library user to cache pre-parsed expressions when evaluating them rather than have this library parse the expression AST from an expression string at every evaluation.