Multi parameter delegation

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

Multi parameter delegation #47

Closed hzy-6 closed 7 months ago

hzy-6 commented 7 months ago

https://github.com/nreco/lambdaparser/issues/3#issuecomment-1912173889

sum(1,2,3,4 ... )

VitaliyMF commented 7 months ago

Sorry, I cannot accept this PR:

it doesn't add "params" support for delegate calls in fact
changes in LambdaParameterWrapper are contradictory. It is wrong assumption that if delegate has 1 argument with object type we can pass all arguments as an array - this is not support of "params" at all
support of params in LambdaParameterWrapper.InvokeDelegate should be implemented in the same way as in LambdaParameterWrapper.InvokeMethod: invocation is delegated to IInvokeMethod implementation, which checks for ParamArrayAttribute of the last delegate argument and handles this accordingly.

hzy-6 commented 7 months ago

Is this okay?

hzy-6 commented 7 months ago

It may be better to leave the InvokeDelegate function to developers for implementation

VitaliyMF commented 7 months ago

@hzy-6 an approach in overall is correct (first screenshot), however implementation in OptionsParamInvokeMethod is definitely wrong - take a look how correct handling of "params" is implemented in OptionsParamInvokeMethod.Invoke method; also, I guess some refactoring is needed to avoid copy paste between these 2 methods.

If you don't know how to implement this, you can use your own implementation which works for you for now (your forked version), and then switch to upstream when it will be implemented.