Arian Motamedi
Arian Motamedi

Reputation: 7423

Caching Compiled Expression Delegates

Backstory: Throughout our project we constantly find ourselves using the System.ComponentModel.DataAnnotations.Validator object as well as attributes to validate properties passed in to our API. The Validator object requires passing in the property name as well as the value, which is fair, but we are all a little grossed out by the [constantly-increasing] number of magic strings being passed around for these property names. Not only this entails the risk of mistyping the property name, it also reduces maintainability in the case that one of those properties get renamed (again, there are way too many of them).

To automatically resolve the member name, I created this helper method (for the sake of simplicity, let's assume I'm only dealing with reference-type properties, hence the P: class constraint):

public static P ValidateProperty<T, P>(T obj, Expression<Func<T, P>> member, List<ValidationResult> results, Func<P, P> onValidCallback = null)
    where T : class
    where P : class
{
    var expr = member.Compile();
    var memberName = ((MemberExpression)member.Body).Member.Name;
    var value = expr(obj);

    bool isValid = Validator.TryValidateProperty(value, new ValidationContext(obj) { MemberName = memberName }, results);
    return isValid ? (onValidCallback != null ? onValidCallback(value) : value) : null;
}

And I simply call it like this:

var value = ValidateProperty(myObject, x => x.PropertyFoo, errors, result => result.Trim());

Which works like a charm and doesn't involve any magic strings being passed around.

A sample request would look like this:

public class Request
{
    public class C1
    {
        Property1;
        Property2;
        ...
    }
    public class C2
    {
        Property1;
        Property2;
        ...
    }
    public class C3
    {
        Property1;
        Property2;
        ...
    }
    ...
}

My concern here however, is on performance implications for member.Compile(). Since there are many different possible permutations of T, P (C1, Property1, C1, Property2, C2, Property1 etc.) I won't be able to cache the compiled expression and execute it on the next call, unless both T and P are of the same type, which happens rarely.

I could optimize this by changing Expression<Func<T, P>> member to Expression<Func<T, object>> member so that now I would have to only cache the expression once per type of T (i.e. C1, C2, etc.)

I was wondering if anyone has any input on a better way to do it, or am I trying to "over engineer" the problem? Is there a common pattern for situations that involve passing around magic strings over and over again?

Upvotes: 2

Views: 1324

Answers (3)

Ivan Stoev
Ivan Stoev

Reputation: 205629

Well, you are correct, Expression.Compile really has a significant performance overhead. If the performance is really a concern, and the reflection approach mentioned in another answer is still concerning you, here is how you can implement compiled delegate caching:

public static class ValidateUtils
{
    static readonly Dictionary<Type, Dictionary<string, Delegate>> typeMemberFuncCache = new Dictionary<Type, Dictionary<string, Delegate>>();

    public static P ValidateProperty<T, P>(this T obj, Expression<Func<T, P>> member, List<ValidationResult> results, Func<P, P> onValidCallback = null)
        where T : class
        where P : class
    {
        var memberInfo = ((MemberExpression)member.Body).Member;
        var memberName = memberInfo.Name;
        Func<T, P> memberFunc;
        lock (typeMemberFuncCache)
        {
            var type = typeof(T);
            Dictionary<string, Delegate> memberFuncCache;
            if (!typeMemberFuncCache.TryGetValue(type, out memberFuncCache))
                typeMemberFuncCache.Add(type, memberFuncCache = new Dictionary<string, Delegate>());
            Delegate entry;
            if (memberFuncCache.TryGetValue(memberName, out entry))
                memberFunc = (Func<T, P>)entry;
            else
                memberFuncCache.Add(memberName, memberFunc = member.Compile());
        }
        var value = memberFunc(obj);
        bool isValid = Validator.TryValidateProperty(value, new ValidationContext(obj) { MemberName = memberName }, results);
        return isValid ? (onValidCallback != null ? onValidCallback(value) : value) : null;
    }
}

Btw, I would remove P : class constraint, thus allowing validating also numeric, date etc. values, but anyway.

Upvotes: 2

Michael Gunter
Michael Gunter

Reputation: 12811

Another option: Rather than using C# 6 features or compiling, just use reflection. Now you're trading compile performance degradation for reflection performance degradation (which is probably less).

...
var property = (PropertyInfo) ((MemberExpression)member.Body).Member;
var propertyName = property.Name;
var value =  property.GetValue(obj, new object[0]);
...

Upvotes: 2

Michael Gunter
Michael Gunter

Reputation: 12811

In C# 6 (VS2015), you can use nameof. This call to your ValidateProperty method involves an extra parameter (name of property vs. value of property) which introduces a bit of redundancy but resolves your potential performance issue of compiling a bunch of stuff dynamically.

var value = ValidateProperty(myObject, nameof(PropertyFoo), PropertyFoo, errors, result => result.Trim());

Upvotes: 1

Related Questions