ncalc / ncalc

Mathematical Expressions Evaluator for .NET
https://ncalc.github.io/ncalc/
MIT License
550 stars 79 forks source link

Remove legacy `EvaluateFunction` and `EvaluateParameter` #275

Open gumbarros opened 1 month ago

gumbarros commented 1 month ago

For the development of #174 we must get rid of the event handlers, because it is impossible to clean up the subscribers without storing the delegates somewhere (it would create some unnecessary overhead).

image

@Bykiev please let me know in this issue if your scenario that depends on event handlers is solved with ExpressionContext.Functions and ExpressionContext.DynamicParameters.

I'm also opening this issue to get feedback on more scenarios covered by event handlers and how we can resolve them.

Furthermore, the removal would not be from one version to another, I would add ObsoleteAttribute with instructions of the migration and wait a few months.

Bykiev commented 1 month ago

I need some more time to research if it can be replaced... As I can see we can support it only when event handlers are not used.

Upd. we can remove event subscription with reflection: https://stackoverflow.com/questions/36084469/how-to-remove-all-eventhandler

gumbarros commented 1 month ago

Upd. we can remove subscription with reflection: ht

I tried to support object pooling with a TryReset method and had no problems with a check to see if the Expression have events. But I find more problems like how to return an Expression to the pool without creating a dependency in Microsoft.Extensions.ObjectPool and decided to shelve the idea for now.

But, I think this issue should remain opened, because the codebase would also be cleaner with only one way to declare functions and params.

Bykiev commented 1 month ago

I did some research and finally it's not possible to implement our use case without event handlers. We have a formula created by the user and we don't know which exactly parameters will be used. We can call Expression.GetParameterNames to get all parameters of the current expression. But let's imagine that the values depends on function arguments, i.e. p1 + GetValue(p2; T+5) which means, that we need to get the value of p2 parameter at current time + 5 hours. Right now we're creating a new expression to get the p2 value at T+5 hour (precomputed)

gumbarros commented 1 month ago

` which means, that we need to get the value ofp2parameter atcurrent time + 5hours. Right now we're creating a new expression to get thep2value atT+5` hour (precomputed)

I did some research and finally it's not possible to implement our use case without event handlers. We have a formula created by the user and we don't know which exactly parameters will be used. We can call Expression.GetParameterNames to get all parameters of the current expression. But let's imagine that the values depends on function arguments, i.e. p1 + GetValue(p2; T+5) which means, that we need to get the value of p2 parameter at current time + 5 hours. Right now we're creating a new expression to get the p2 value at T+5 hour (precomputed)

Should be possible to recover other arguments using ExpressionContext?

Like

expression.Functions["Foo"] = (args)=>{
        args.Context.DynamicParameters // Bar is here 
    }
Bykiev commented 1 month ago

Should be possible to recover other arguments using ExpressionContext?

Like

expression.Functions["Foo"] = (args)=>{
        args.Context.DynamicParameters // Bar is here 
    }

It seems to be not working as expected or maybe my code is wrong:

var expr = new Expression("GetValue(p1) + p1");
expr.DynamicParameters["p1"] = _ =>
{
    return 10;
};

expr.Functions["GetValue"] = (args2) =>
{
    args2.Context.DynamicParameters["p1"] = _ =>
    {
        return 11;
    };

    return args2[0].Evaluate();
};

var res = expr.Evaluate(); // should be 21, but returns 22
gumbarros commented 1 month ago

Should be possible to recover other arguments using ExpressionContext? Like

expression.Functions["Foo"] = (args)=>{
        args.Context.DynamicParameters // Bar is here 
    }

It seems to be not working as expected or maybe my code is wrong:

var expr = new Expression("GetValue(p1) + p1");
expr.DynamicParameters["p1"] = _ =>
{
    return 10;
};

expr.Functions["GetValue"] = (args2) =>
{
    args2.Context.DynamicParameters["p1"] = _ =>
    {
        return 11;
    };

    return args2[0].Evaluate();
};

var res = expr.Evaluate(); // should be 21, but returns 22

Solved, it's because you are changing the context before evaluating.

using NCalc;

var expr = new Expression("GetValue(p1) + p1");
expr.DynamicParameters["p1"] = _ =>
{
    return 10;
};
expr.Functions["GetValue"] = (args2) =>
{
    var args2Result =  args2[0].Evaluate();
    args2.Context.DynamicParameters["p1"] = _ =>
    {
        return 11;
    };

    return args2Result;
};

var res = expr.Evaluate(); // Now should be 21
Console.Write(res);
Bykiev commented 1 month ago

This doesn't seems to be correct, because I need to get the same value with "p1 + GetValue(p1*2)" and "GetValue(p1*2) + p1"

gumbarros commented 1 month ago

This doesn't seems to be correct, because I need to get the same value with "p1 + GetValue(p1*2)" and "GetValue(p1*2) + p1"

Yeah, the order affects and I can't fix. Can you share the event handler version?

Bykiev commented 1 month ago

I think the global class variable should solve the issue:

private static DateTime Dt = DateTime.Now;

static void Main(string[] args)
{
    var expr = new Expression("p1 + GetValue(p1*2)");
    expr.DynamicParameters["p1"] = _ =>
    {
        if(Dt.Hour == 23)
            return 10;

        return 21;
    };

    expr.Functions["GetValue"] = (args2) =>
    {
        DateTime oldDt = Dt;
        Dt = Dt.AddHours(1);

        var args2Result = args2[0].Evaluate();

        Dt = oldDt;

        return args2Result;
    };

    var res = expr.Evaluate();
    Console.Write(res);
    Console.ReadLine();
}
Bykiev commented 1 month ago

But still this is not very user friendly, I've a common method to get the value of any dynamic parameter but without event handler I need to add all of them to the dictionary. Inside the handler I parsed the parameter name (it was like {pId, pName}) and based on it I assigned the value