ConditionalAnd with variables

[HelgeL]

First, I want to say it's a fascinating and high-quality module, congratulations and thank you for putting it up here!

I have a problem evaluating expressions with variables when using ConditionalAnd and null values:

Dictionary<string, object> dictValues = new Dictionary<string, object>();
string nullString = null;
dictValues[ "TestNullString" ] = nullString;

//pass dictValues as expEval.Variables into expression evaluation instance and then...
string expression = "!string.IsNullOrEmpty(TestNullString) && TestNullString.StartsWith(\"ABC\")";
var result = expEval.Evaluate(expression);

This throws a NullReferenceException with the second half, where "plain C#" would skip evaluating this part. The issue seems related to some that were discussed here recently, especially #51 and #53, but the fixes don't address it. The exception details are:

 at ExpressionEvaluator.DetermineInstanceOrStatic(Type& objType, Object& obj, ValueTypeNestingTrace& valueTypeNestingTrace) in EvaluatorTest\ExpressionEvaluator.cs

[codingseb]

Hello @HelgeL
Thanks to opening this issue.
I will look about this bug as soon as possible.

[codingseb]

😅 This one is tricky.
Because of the way ExpressionEvaluator evaluate operators. It evaluate first all elements between operators and then evaluate operators in the result stack with simple values. For a majority of operators it works well.

But in the case of ConditionalAnd and ConditionalOr I need to find a way to delay the evaluation of what is to the right. And decide later if it need to be evaluate depending on the result of what is to the left.

Maybe with temporary encapsulate the right part in a SubExpression it will works. I will make some tests and publish a new version ASAP.

Not sure if it will not break the precedence rules.

[HelgeL]

Thanks @codingseb for looking into the issue. Unfortunately I cannot be much of help for fixing it (I wish I was...). But what you write makes total sense, it seems a way to eliminate the test what's on the right is needed in case left is null, given the operator is one of the "Conditional" ones.

[codingseb]

I am sorry but I did not find a way to do this easily.
The current way ExpressionEvaluator evaluate things avoid to delay or cancel evaluation of the right part of operators without breaking precedence rules (priorities of execution between operators)

For a while I thing to refactor more deeply ExpressionEvaluator to use a syntaxic tree in place of just evaluating "on the fly".
If I succeed one day to do this transition. It should allow a set of interesting new functionalities and to correct this bug. But it will probably take longer than I thought.

I opened a specific issue to follow the evolution of implementing this transition to a 2.0 ExpressionEvaluator (#58)

For now I can only suggest that you find another way to write the expressions affected by this bug.

[Karbot]

Hello @codingseb,
I am a coworker of @HelgeL and we had another look at the problem.
We figured out a way to realize left associativity and put together a solution that's working for our cases and passes all of your test cases as well.
I will add a few comments to it and than suggest a pull request for our fork.
Maybe it will help you or others.

[codingseb]

Hello @Karbot.

Ho that's very nice. Yes I will gladly look to your pull request if you suggest one.

For the operators precedence bugs I added some tests in the branch V2_TrySyntaxicTreeImplementation that are not in master if you want to test it on your solution. Otherwise I will test it myself on your pull request.

Thanks for your contribution for you and @HelgeL. It is very appreciated.

[Karbot]

I took your new defined tests from V2_TrySyntaxicTreeImplementation) into the pull request. They were passed as well.

[codingseb]

OK Thanks again guys. I think your solution is very good.
I just published version 1.4.12.0 that contains your work and correct this bug.

I close this issue.
I will keep the v2.0 issue open and maybe one day this will be manage by syntaxic tree.
But it's now less important to do it fast.

[codingseb]

I Reopen this issue because the current solution do not manage all possibles cases and create a new bug with exception in scripts.
So I am currently working on adapting and extending the current found solution with more tests to manage theses additional linked bugs

[codingseb]

Just published version 1.4.13.0 with additionals corrections for this. And some Refactoring and simplification.