nein-linq icon indicating copy to clipboard operation
nein-linq copied to clipboard

Property vs method issue

Open space-alien opened this issue 2 years ago • 4 comments

This works:

[InjectLambda]
public decimal Total => TotalExpr.Compiled(this); // **Property**

private static CachedExpression<Func<Order, decimal>> TotalExpr { get; }
= new(order => order.Rows.Sum(row => row.Total));

But changing the property to a method throws Unable to retrieve lambda expression from FooBarWhatever: returns no lambda expression:

[InjectLambda]
public decimal Total() => TotalExpr.Compiled(this); // **Method**

private static CachedExpression<Func<Order, decimal>> TotalExpr { get; }
= new(order => order.Rows.Sum(row => row.Total));

In my case, the usage is nested in a basic projection. Giving Nein-Linq some clues in the form of: [InjectLambda(nameof(TotalExpr))] gives us the following: Unable to retrieve lambda expression from FooBarWhatever.TotalExpr: non-static implementation expected.

Any thoughts?

space-alien avatar Apr 12 '22 13:04 space-alien

Mixing properties and static methods has some nasty side effects, if I remember correctly... (thus, the error message "non-static implementation expected").

Try a static extension method Total(this Whatever ...) and lambda injection.

But I'll have a look, if I can improve this concrete use case anyway.

axelheer avatar Apr 13 '22 11:04 axelheer

Okay, after further investigation: we have some extra handling for properties on "model" objects, which handles them like static extension methods having one and only one parameter - the parameter representing the "model" object.

Which is fine:

class Order
{
    [InjectLambda]
    public int Total { get; }
}

var query =
    from o in orders
    select new
    {
        o.Total
    }

Gets a "treatment" like:

static class OrderExtensions
{
    [InjectLambda]
    public int Total(this Order order);
}

var query =
    from o in orders
    select new
    {
        o.Total()
    }

But, extending this to:

class Order
{
    [InjectLambda]
    public int Total(Foo foo, Bar bar);
}

var query =
    from o in orders
    select new
    {
        o.Total(foo, bar)
    }

Which would basically the same like:

static class OrderExtensions
{
    [InjectLambda]
    public int Total(this Order order, Foo foo, Bar bar);
}

var query =
    from o in orders
    select new
    {
        o.Total(foo, bar)
    }

...can get quite complicated.

I'm sure it's possible and there's a "clean" solution, but I don't see the benefit for the effort here... (?)

axelheer avatar Apr 19 '22 15:04 axelheer

I see what you mean.

I haven't dug around in the inner workings of NeinLinq, so forgive any lack of understanding or vagueness here, but I wonder if the approach could be "inverted" to get the best of both worlds:

So, where you have a special treatment for property getters, instead, treat property getters as instance methods, and generalise the approach so that all instance methods can also potentially benefit from the static treatment you described above (i.e., when a static expression signature matches the instance method's signature plus the implicit "this" parameter).

In fact...... now that I think about it some more, I remember (from the docs) that this should already work:

public class Functions : IFunctions
{
    [InjectLambda]
    public string Foo(Entity value)  // instance method!
    {
        ...
    }

    public Expression<Func<Entity, string>> Foo()  // matching sig
    {
        ...
    }
}

So, I guess instance method signature matching is already in place? If so, perhaps it could be extended to also look for a suitable matching static expression with the extra "this" parameter?

And then, perhaps you would have all the necessary plumbing to take over from the current special approach applied to property getters?

However... if that is all completely infeasible/nonsense, then perhaps the error messages given by NeinLinq in the two scenarios I mentioned up top could give hints as to the correct approach. (I.e. [InjectLambda] for an instance method with a suitably-named static expression, and [InjectLambda(explicitStaticExpressionName)])

space-alien avatar Apr 19 '22 16:04 space-alien

Yeah, this makes basically sense, but it's somehow different too.

class Order
{
    [InjectLambda]
    public int Total { get; } // (2)

    [InjectLambda]
    public int Total(Foo foo, Bar bar); // (4)
}

static class OrderExtensions
{
    [InjectLambda]
    public int Total(this Order order, Foo foo, Bar bar); // (1)
}

class OrderFunctions
{
    [InjectLambda]
    public int Total(Order order, Foo foo, Bar bar); // (3)
}

var utility = new OrderFunctions()

var query =
    from o in orders
    select new
    {
        o.Total(foo, bar), // (1)
        o.Total, // (2)
        utility.Total(o, foo, bar), // (3)
        o.Total(foo, bar) // (4)
    }

Thus, we'd get the best of three or four worlds:

  1. Injection for extension methods for expressions reduceable to parameters (o, foo, bar)
  2. Injection for properties of expressions reduceable to parameters treated as extension methods above (just o)
  3. Injection for functions of expressions reducable to constant values (utility) for expressions reduceable to parameters (o, foo, bar)
  4. (not possible yet) Injection for functions of expressions reduceable to parameters treated as functions above (o, foo, bar)

So, we need to analyze some expressions a bit further or maybe re-work the entire thing. Sounds like fun. 😅

axelheer avatar Apr 19 '22 16:04 axelheer