3

I'm trying to create a simple mapper using Expression by this code:

public static class MyUtility {

    public static Action<TSource, TTarget> BuildMapAction<TSource, TTarget>(IEnumerable<PropertyMap> properties) {

        var sourceInstance = Expression.Parameter(typeof(TSource), "source");
        var targetInstance = Expression.Parameter(typeof(TTarget), "target");

        var statements = BuildPropertyGettersSetters(sourceInstance, targetInstance, properties);

        Expression blockExp = Expression.Block(new[] { sourceInstance, targetInstance }, statements);

        if (blockExp.CanReduce)
            blockExp = blockExp.ReduceAndCheck();
        blockExp = blockExp.ReduceExtensions();

        var lambda = Expression.Lambda<Action<TSource, TTarget>>(blockExp, sourceInstance, targetInstance);

        return lambda.Compile();
    }

    private static IEnumerable<Expression> BuildPropertyGettersSetters(
        ParameterExpression sourceInstance,
        ParameterExpression targetInstance,
        IEnumerable<PropertyMap> properties) {

        var statements = new List<Expression>();

        foreach (var property in properties) {

            // value-getter
            var sourceGetterCall = Expression.Call(sourceInstance, property.SourceProperty.GetGetMethod());
            var sourcePropExp = Expression.TypeAs(sourceGetterCall, typeof(object));

            // value-setter
            var targetSetterCall =
                    Expression.Call(
                        targetInstance,
                        property.TargetProperty.GetSetMethod(),
                        Expression.Convert(sourceGetterCall, property.TargetProperty.PropertyType)
                        );
            var refNotNullExp = Expression.ReferenceNotEqual(sourceInstance, Expression.Constant(null));
            var propNotNullExp = Expression.ReferenceNotEqual(sourcePropExp, Expression.Constant(null));
            var notNullExp = Expression.And(refNotNullExp, propNotNullExp);
            var ifExp = Expression.IfThen(notNullExp, targetSetterCall);

            statements.Add(ifExp);
        }

        return statements;
    }

}

Everything seems OK to me, but when I'm trying to test it, I just get a null-reference exception. The test objects and method:

public class UserEntity {

    public string Name { get; set; }
    public string Family { get; set; }
    public int Age { get; set; }
    public string Nickname { get; set; }

}

public class UserModel {

    public string FirstName { get; set; }
    public string LastName { get; set; }
    public int Age { get; set; }
    public string Nickname { get; set; }

}

public static class CallTest {

    public static void Call() {
        var entity = new UserEntity {
            Name="Javad",
            Family="Amiry",
            Age = 25,
            Nickname = "my nickname is here",
        };
        var model = new UserModel();

        var map1 = new PropertyMap {
            SourceProperty = entity.GetType().GetProperty("Age"),
            TargetProperty = model.GetType().GetProperty("Age"),
        };
        var map2 = new PropertyMap {
            SourceProperty = entity.GetType().GetProperty("Nickname"),
            TargetProperty = model.GetType().GetProperty("Nickname"),
        };

        var action = MyUtility.BuildMapAction<UserEntity, UserModel>(new[] {map1, map2});
        action(entity, model); // here I get the error System.NullReferenceException: 'Object reference not set to an instance of an object.'
    }

}

Do you have any idea what's going on there? What I missed?


NOTE: I cannot use third-party mappers (like AutoMapper)

amiry jd
  • 27,021
  • 30
  • 116
  • 215

1 Answers1

6

The issue is caused by this line:

Expression blockExp = Expression.Block(new[] { sourceInstance, targetInstance }, statements);

The first argument of the used Expression.Block overload represents the local variables of the block. By passing the lambda parameters there you just define 2 local unassigned variables, hence the NRE at execution time. You can see that by examining the lambda expression DebugView in VS locals/watch window, which in your sample call looks something like this:

.Lambda #Lambda1<System.Action`2[ConsoleApp3.UserEntity,ConsoleApp3.UserModel]>(
    ConsoleApp3.UserEntity $source,
    ConsoleApp3.UserModel $target) {
    .Block(
        ConsoleApp3.UserEntity $source,
        ConsoleApp3.UserModel $target) {
        .If (
            $source != null & .Call $source.get_Age() .As System.Object != null
        ) {
            .Call $target.set_Age((System.Int32).Call $source.get_Age())
        } .Else {
            .Default(System.Void)
        };
        .If (
            $source != null & .Call $source.get_Nickname() .As System.Object != null
        ) {
            .Call $target.set_Nickname((System.String).Call $source.get_Nickname())
        } .Else {
            .Default(System.Void)
        }
    }
}

Note the redefinition of source and target inside the block.

After using the correct overload:

Expression blockExp = Expression.Block(statements);

the view is now like this:

.Lambda #Lambda1<System.Action`2[ConsoleApp3.UserEntity,ConsoleApp3.UserModel]>(
    ConsoleApp3.UserEntity $source,
    ConsoleApp3.UserModel $target) {
    .Block() {
        .If (
            $source != null & .Call $source.get_Age() .As System.Object != null
        ) {
            .Call $target.set_Age((System.Int32).Call $source.get_Age())
        } .Else {
            .Default(System.Void)
        };
        .If (
            $source != null & .Call $source.get_Nickname() .As System.Object != null
        ) {
            .Call $target.set_Nickname((System.String).Call $source.get_Nickname())
        } .Else {
            .Default(System.Void)
        }
    }
}

and the NRE is gone.

That was regarding the original issue. But the generated code looks ugly and suboptimal. The source object null check can be surrounding the whole block and the type conversion and value null checks can be performed only when needed. As a bonus, here is how I would write it:

public static Action<TSource, TTarget> BuildMapAction<TSource, TTarget>(IEnumerable<PropertyMap> properties)
{
    var source = Expression.Parameter(typeof(TSource), "source");
    var target = Expression.Parameter(typeof(TTarget), "target");

    var statements = new List<Expression>();
    foreach (var propertyInfo in properties)
    {
        var sourceProperty = Expression.Property(source, propertyInfo.SourceProperty);
        var targetProperty = Expression.Property(target, propertyInfo.TargetProperty);
        Expression value = sourceProperty;
        if (value.Type != targetProperty.Type)
            value = Expression.Convert(value, targetProperty.Type);
        Expression statement = Expression.Assign(targetProperty, value);
        // for class/interface or nullable type
        if (!sourceProperty.Type.IsValueType || Nullable.GetUnderlyingType(sourceProperty.Type) != null)
        {
            var valueNotNull = Expression.NotEqual(sourceProperty, Expression.Constant(null, sourceProperty.Type));
            statement = Expression.IfThen(valueNotNull, statement);
        }
        statements.Add(statement);
    }

    var body = statements.Count == 1 ? statements[0] : Expression.Block(statements);
    // for class.interface type
    if (!source.Type.IsValueType)
    {
        var sourceNotNull = Expression.NotEqual(source, Expression.Constant(null, source.Type));
        body = Expression.IfThen(sourceNotNull, body);
    }

    // not sure about the need of this
    if (body.CanReduce)
        body = body.ReduceAndCheck();
    body = body.ReduceExtensions();

    var lambda = Expression.Lambda<Action<TSource, TTarget>>(body, source, target);

    return lambda.Compile();
}

which generates a more C# looking code:

.Lambda #Lambda1<System.Action`2[ConsoleApp3.UserEntity,ConsoleApp3.UserModel]>(
    ConsoleApp3.UserEntity $source,
    ConsoleApp3.UserModel $target) {
    .If ($source != null) {
        .Block() {
            $target.Age = $source.Age;
            .If ($source.Nickname != null) {
                $target.Nickname = $source.Nickname
            } .Else {
                .Default(System.Void)
            }
        }
    } .Else {
        .Default(System.Void)
    }
}
Ivan Stoev
  • 195,425
  • 15
  • 312
  • 343
  • 2
    RE your note in the comments: The CanReduce/ReduceAndCheck/ReduceExtensions will do nothing with the built in expression types. If you are subclassing Expression with your own custom Expression types, then they may be potentially useful (of course, depending on your implementation) – pinkfloydx33 Oct 22 '17 at 14:04
  • @pinkfloydx33 Thanks for the clarification. – Ivan Stoev Oct 22 '17 at 14:11
  • Can you give an example where you would use such a subclassed expression type in the area of your example (where you are clearly doing c# code generation in expression trees)? Btw do you have a dynamically changing model? If not, then isnt generated code easier to understand and debug (and faster at startup)? – MBoros Oct 23 '17 at 04:56