Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Property vs method issue #33

Open
space-alien opened this issue Apr 12, 2022 · 4 comments
Open

Property vs method issue #33

space-alien opened this issue Apr 12, 2022 · 4 comments

Comments

@space-alien
Copy link

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?

@axelheer
Copy link
Owner

axelheer commented Apr 13, 2022

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
Copy link
Owner

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... (?)

@space-alien
Copy link
Author

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)])

@axelheer
Copy link
Owner

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. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants