New caching and deferred evaluation strategy

Question

This is a simple wrapper class for a lambda expression, that stores one as Expression and hands it back out either as Expression, or compiled as Func. This is useful because while Linq-to-Objects requires a Func, other technologies such as Entity Framework require an Expression (so that the expression tree can be projected into SQL, which isn't feasible with the compiled Func). However, it would be nice if the compilation overhead of the Func were not taken until use, since it might never be used. (There doesn't appear to be a way to get true compile-time coercion to both types as the compiler can only make one inference about which form you want, expression or func.)

Here's an example of how it's used.

Given an object:

public partial class MyObject {
    public int Value { get; set; }
}

That is represented in the database like so:

CREATE TABLE dbo.MyObjects (
    Value int NOT NULL CONSTRAINT PK_MyObjects PRIMARY KEY CLUSTERED
);

Then it works like this:

var greaterThan5 = new WhereConstraint<MyObject>(o => o.Value > 5);

// Linq to Objects
List<MyObject> list = GetObjectsList();
var filteredList = list.Where(greaterThan5).ToList(); // no special handling

// Linq to Entities
IQueryable<MyObject> myObjects = new MyObjectsContext().MyObjects;
var filteredList2 = myObjects.Where(greaterThan5).ToList(); // no special handling

If implicit conversion isn't suitable, you can cast explicitly to the target type, or more clearly, use one of the AsExpression() or AsFunc() methods.

var expression = (Expression<Func<MyObject, bool>>) greaterThan5;
var func = greaterThan5.AsFunc();

Some additional thought on this has yielded a couple more points I think are worth adding:

• I do think that locking is required because I don't know enough about how variable assignment works, and so to be conservative I have to allow the possibility that without the lock, the _funcGetter = line could result in a state where a multithreaded application could attempt to execute it while it's still being assigned and there is an interim neither-fully-assigned-nor-unassigned state.
• If two threads did perform the _funcGetter function at a very close time, it's possible that the expression could be compiled twice (the lambda containing lock having started executing in both, but had not locking in both), but this shouldn't have deleterious side effects since everything would still work correctly.

Instead of checking whether a local variable is null, then locking when it is, then computing the value, and then on every call still checking if the variable is null and returning the cached value, what do you think of the following pattern (where the value is stored instead as a delegate that rewrites "itself")?

public class VersatileLambda<T> where T : class {
    private readonly Expression<T> _expression;
    private Func<T> _funcGetter;

    public VersatileLambda(Expression<T> expression) {
        if (expression == null) {
            throw new ArgumentNullException(nameof(expression));
        }
        _expression = expression;
        _funcGetter = () => {
            lock (this) {
                var func = _expression.Compile();
                _funcGetter = () => func;
                return func;
            }
        };
    }

    public static implicit operator Expression<T>(VersatileLambda<T> lambda) {
        return lambda?._expression;
    }

    public static implicit operator T(VersatileLambda<T> lambda) {
        return lambda?._funcGetter();
    }

    public Expression<T> AsExpression() { return this; }
    public T AsLambda() { return this; }
}

public class WhereConstraint<[DelegateConstraint] T> : VersatileLambda<Func<T, bool>> {
    public WhereConstraint(Expression<Func<T, bool>> lambda)
        : base(lambda) { }
}

Are there holes in this? Is the lock even necessary? (I suspect that it is but couldn't make a solid objective case why.) Is this a good strategy to use that other programmers will understand? Is avoiding a null check even worth this perhaps greater level of obfuscation?

Finally, I did check that the this reference points to the VersatileLambda class. But is there a way to get the this of a delegate from within it?

Note: use of Fody.ExtraConstraints and [DelegateConstraint] can get the same effect as (not yet available in C#) where T : Delegate.

Answer 1

I'm not sure I fully understand the point of the code so I might be missing the mark but...

Your _funcGetter would make far more sense as a Lazy<T>

private readonly Expression<T> _expression;
private Lazy<T> _func;

public VersatileLambda(Expression<T> expression) {
    if (expression == null) {
        throw new ArgumentNullException(nameof(expression));
    }
    _expression = expression;
    _func = new Lazy<T>(() => expression.Compile());
}

public static implicit operator T(VersatileLambda<T> lambda) {
    return lambda?._func.Value;
}

Lazy will handle all the thread safety issues for you. You don't need to worry about locking at all - the default will ensure only one is created (i.e. compile only called once).

As an aside - you should never lock (this) when this is public instance... you have no idea whether or not someone else will try to lock the instance and cause a deadlock. Always use a private object to synchronise access.

(code from memory directly into browser so might not compile but should be substantially correct)

*updated to use Lazy<T> as per comments.