nreco / lambdaparser

Runtime parser for string expressions (formulas, method calls). Builds dynamic LINQ expression tree and compiles it to lambda delegate.
http://www.nrecosite.com/
MIT License
307 stars 55 forks source link

Support for dynamic types like expando object #36 #37

Open PeterHagen opened 3 years ago

PeterHagen commented 3 years ago

Added validation if object inherits (or is) an IDictionary<string, object>, and return the value if the key exists

VitaliyMF commented 3 years ago

I cannot accept this PR for these reasons:

As I understand only change that is actually needed is

            // An epandoObject has a IDictionary underneath
            object objectValue;
            if (obj is IDictionary<string, object> dictionary && dictionary.TryGetValue(propertyName, out objectValue))
            {
                return new LambdaParameterWrapper(objectValue, Cmp);
            }

To avoid possible misuse of other IDictionary<string, object> objects I think it is better to include a test for System.Dynamic.ExpandoObject type (obj is ExpandoObject).

You may change your PR to include only this change (+ add a unit test for it) or I may add support for expando object by myself.

PeterHagen commented 3 years ago

Ah, sorry, I thought the pull request would only contain the first commit 09ffd3b, not the others. So yes, the change you quoted is the only change I suggest. I had to add the other changes to get it to run correctly for my situation, but just temporary.

To avoid possible misuse of other IDictionary<string, object> objects I think it is better to include a test for System.Dynamic.ExpandoObject type (obj is ExpandoObject).

I'm not sure what would happen with a dynamic object, and if it that would work with IDictionary too. So as ExpandoObject would be better.

After using this change I actually ran into another (logical) problem. Expando properties can have "-" symbols in their name. This won't work in LambdaParser, as it will (and I'm guessing) resolve in minus, instead of a string value. For example, I added a property "ODO-BMI-T-MC", and an expression like "diploma.ODO-BMI-T-MC == true" (where diploma is the expand object).

VitaliyMF commented 3 years ago

After using this change I actually ran into another (logical) problem. Expando properties can have "-" symbols in their name. This won't work in LambdaParser, as it will (and I'm guessing) resolve in minus, instead of a string value. For example, I added a property "ODO-BMI-T-MC", and an expression like "diploma.ODO-BMI-T-MC == true" (where diploma is the expand object).

Since ExpandoObject implements IDictionary<string,object> you can access its properties simply like any other (real) dictionary? Like that: diploma["ODO-BMI-T-MC"] == true

If this works it seems no need to add special support for ExpandoObject at all.

PeterHagen commented 3 years ago

True, therefor I added a "safe" dictionary, which won't give exceptions on missing keys. So indeed, expandoObject won't be necessary at this moment, although I prefer expressions without [ ], to keep it clean for the users.

I'll remove my fork and please drop the request.