CODEWITHSUNDEEP
c#.netexpressionexpression-trees
Gerwald
Gerwald

Reputation: 1589

Iterating a expression tree to check for null

I am really struggling to write a generic class, that needs to check if all members of a passed expression are not null, by invoking the actual object.

I am calling the method like:

new TestClass<Class1, Class2>.IsAnyMemberNull(x => x.Property1.List1, object1);

and the Method looks like:

public bool IsAnyMemberNull(Expression<Func<T, IList<TM>>> listExpression, T entityDb)
{
    var expressionsToCheck = new List<MemberExpression>();
    var expression = listExpression.Body as MemberExpression;
    while (expression != null)
    {
        expressionsToCheck.Add(expression);
        expression = expression.Expression as MemberExpression;
    }

    for (var i = expressionsToCheck.Count - 1; i >= 0; i--)
    {
        var objectMember = Expression.Convert(expressionsToCheck[i], typeof (object));
        var lambda = Expression.Lambda<Func<T, object>>(objectMember);
        var value = lambda.Compile().Invoke(entityDb);
        if (value == null)
            return true;
    }

    return false;
}

When executing, I get the exception:

incorrect number of parameters supplied for lambda declaration

Any ideas, what I have done wrong?

Upvotes: 0

Views: 430

Answers (2)

Alice
Alice

Reputation: 625

While it is possible to get your code working, so that correct lambdas are built and compiled, using repeatedly compiled lambdas to achieve null-checking is a costly overkill.

Generally, using lambdas so casually (compiling a lambda for each property in chain and a single object) comes with a notable performance hit. I've run some tests, and on my computer executing this method 1000 times for a given object yielded ~300-700 ms time (depending on the number of properties in the chain). Dunno how many entities you deal with, but it's not a good sign, and better replacements are available. Please read further...

The question is, what are you using this for? That method of yours reminds me of null-conditional operators quite a lot. Generally, if you:

  • just want to check if any property in the property chain is a null
  • use C# 6
  • have all parameter chain lambdas (like x => x.Property1.List1) known at runtime

Then you might scrap all that IsAnyMemberNull method altogether in favour of something like:

object1.Property1.List1 == null

Much more concise and no additional methods are required. I ran it 1 million times and it was still within ~23ms time. It means it's dozens thousands faster than creating all those lambdas.

If you can't use null-coalescing operators for whatever reason (especially when the expression is built dynamically), you might instead decide to use Field/Property reflection.

I took the liberty of removing all that generic class wrapping in favour of a generic method. From your usage example, it seemed the only purpose of the generic class was to access a specific method with class' generic type parameters. It means one new class would have to be made and stored for each variation of the method, for no apparent reason, if I'm not mistaken, for the rest of application's lifetime. Generic methods in specific classes are generally preferred to specific methods in generic classes, in cases like these.

Also, I removed IList, because I see no reason how requiring the last parameter to be of IList type serves the purpose of the function; it only limits its applicability with no apparent gain.

Overall, the result is the following:

public bool IsAnyMemberNull<TEntity, TMember>(Expression<Func<TEntity, TMember>> paramChain, TEntity entityDb)
{
    var expressionsToCheck = new List<MemberExpression>();
    var expression = paramChain.Body as MemberExpression;
    while (expression != null)
    {
        expressionsToCheck.Add(expression);
        expression = expression.Expression as MemberExpression;
    }

    object value = entityDb;
    for (var i = expressionsToCheck.Count - 1; i >= 0; i--)
    {
        var member = expressionsToCheck[i].Member;
        if (member is PropertyInfo) value = (member as PropertyInfo).GetValue(value);
        else if (member is FieldInfo) value = (member as FieldInfo).GetValue(value);
        else throw new Exception(); // member generally should be a property or field, shouldn't it?

        if (value == null)
            return true;
    }

    return false;
}

After running this ~1000 times, it took about 4-6ms; 50-100 times better than lambdas, though null-propagation still reigns supreme.

Called as following (assuming it still resides in TestClass, which it doesn't need to):

new TestClass().IsAnyMemberNull<Class1,Class2>(x => x.Property1.List1, object1);

(Class1 and Class2 might not be necessary, thanks to type inference)

Hope this helps. It's not exactly what you asked for, but I'm afraid with all this lambda-spawning you could run into serious performance issues; especially if this code was to be used many times per request.

Upvotes: 1

Aleksey L.
Aleksey L.

Reputation: 37948

You have a problem in lambda expression creation - it is simpler than you thought. You should build lambda for each expressionToCheck with original expression parameter:

for (var i = expressionsToCheck.Count - 1; i >= 0; i--)
{     
    var lambda = Expression.Lambda<Func<T, object>>(expressionsToCheck[i], listExpression.Parameters);
    var value = lambda.Compile().Invoke(entityDb);
    if (value == null)
        return true;
}

Upvotes: 0

Related Questions