Object to object mapping verification - is this extension method useful?

Question

In our .NET tests we use NSubstitute & ExpectedObjects.

Testing object expectations involves hand crafting large anonymous objects and when new properties are added we need to go back to these anonymous objects & update them.

Attempting below to get a fluent Object to DTO builder - which fails when a property is missed.

Here is the implementation:

 //CustomerCreatedEvent has all below properties 
 var exp1 = @event.ToDto<CustomerCreatedEvent,CustomerDetail>(
 x=> x.AggregateId.As("CustomerId"), 
 x => x.Email, // comment this out and test FAILS -> CustomerDetail.Email required
 x => x.FirstName, 
 x => x.Surname);
 var exp2 = new {
 CustomerId= @event.AggregateId,
 @event.Email, // comment this out and test still passes
 @event.FirstName,
 @event.Surname};
 exp1.ToExpectedObject().ShouldMatch(actual);
 exp2.ToExpectedObject().ShouldMatch(actual);

I have 2 questions:

1. Is my code just adding 'noise'?

2. Is the implementation code below sound?

   public static TResult ToDto(this TSource obj, params Expression>[] items) where TSource : class { var eo = new ExpandoObject(); var props = eo as IDictionary;

    foreach (var item in items)
    {
    var member = item.Body as MemberExpression;
    var unary = item.Body as UnaryExpression;
    var body = member ?? (unary != null ? unary.Operand as MemberExpression : null);
    if (member != null && body.Member is PropertyInfo)
    {
    var property = body.Member as PropertyInfo;
    props[property.Name] = obj.GetType()
    .GetProperty(property.Name)
    .GetValue(obj, null);
    }
    else
    {
    var property = unary.Operand as MemberExpression;
    if (property != null)
    {
    props[property.Member.Name] = obj.GetType()
    .GetProperty(property.Member.Name)
    .GetValue(obj, null);
    }
    else
    {
    var compiled = item.Compile();
    var output = (KeyValuePair<string, object>)compiled.Invoke(obj);
    props[output.Key] = obj.GetType()
    .GetProperty(output.Value.ToString())
    .GetValue(obj, null);
    }
    }
    }
    TResult result = Activator.CreateInstance<TResult>();
    foreach (var item in props)
    {
    result.GetType().GetProperty(item.Key).SetValue(result, item.Value, null);
    }
    return result;
   }
   

   }

• Nick Udell: I would vastly prefer the variable name 'item' in your foreach loop than 'thing'
• Chris McKelt: ah yep will change - had it on my mind at the time but normally wouldnt use it
• Chris McKelt: Found fluent assertions which seems to have a solution for the problem - github.com/dennisdoomen/fluentassertions/…

Answer 1

Minor nitpick first:

public static T2 ToDto<T1,T2>(this T1 obj, params Expression<Func<T1, dynamic>>[] items) where T1 : class
{

I think putting generic type constraints on a new line makes them easier to spot and adds to readability (especially with long signatures) - consider:

public static T2 ToDto<T1,T2>(this T1 obj, params Expression<Func<T1, dynamic>>[] items)
 where T1 : class
{

I really don't like T1 and T2 for generic type parameter names. T is ok when it's the only parameter, but I'd much, much rather like to see TSource and TResult - it makes it much clearer which one serves which purpose, at a glance.

This also instantly makes it easy to find source a much more meaningful name than obj. I'm all for readable identifiers, so I would like to see props renamed to properties, and eo to expandoObject, or perhaps just value - I will be using these identifiers for the remainder of this post.

I like that you're using var - I don't understand why you're not using it here, since JsonConvert.SerializeObject returns a String:

string json = JsonConvert.SerializeObject(value); // need json.net

That said, the comment isn't necessary. If you referenced Json.net from a NuGet package, the IDE will tell you when/if a package is missing. If you referenced Json.net from an assembly, the project will fail to build and under "references", Newtonsoft.Json will appear problematic - the comment really doesn't help with anything, it's just noise.

---

In this block:

if (member != null && body.Member is PropertyInfo)
{
 var property = body.Member as PropertyInfo;
 if (property != null)
 properties[property.Name] = source.GetType().GetProperty(property.Name).GetValue(source, null);
}

The condition if (property != null) will always be true, because body.Member is PropertyInfo is necessarily true in that branch. Hence it can be simplified to this:

if (member != null && body.Member is PropertyInfo)
{
 var property = body.Member as PropertyInfo;
 properties[property.Name] = source.GetType().GetProperty(property.Name).GetValue(source, null);
}

The else branch has a redundant cast - you defined unary as item.Body as UnaryExpression; hence, unary already has the reference you're getting in this superfluous declaration:

var ubody = (UnaryExpression)item.Body;

Just use unary, you don't need ubody at all.

I haven't tested it, but the type parameter in DeserializeAnonymousType<object> seems superfluous as well - Activator.CreateInstance<TResult>() will create a TResult object.. that you can readily return - so instead of this:

var json = JsonConvert.SerializeObject(value);
var anon = JsonConvert.DeserializeAnonymousType<object>(json, Activator.CreateInstance<TResult>());
return ((JObject)anon).ToObject<TResult>();

You'd have that:

var json = JsonConvert.SerializeObject(value);
return JsonConvert.DeserializeAnonymousType(json, Activator.CreateInstance<TResult>());

Notice I'm letting the compiler infer the generic type parameter here.

---

Lastly, the way chained reflection methods are laid out isn't consistent between the if and the else branch - one has it like this:

properties[property.Name] = source.GetType().GetProperty(property.Name).GetValue(source, null);

The other has it like that:

properties[property.Member.Name] = source.GetType()
 .GetProperty(property.Member.Name)
 .GetValue(source, null);

I like how putting each instruction on a new line makes it easier to see what's going on. Some may argue that it's even easier when the dots line up:

properties[property.Member.Name] = source.GetType()
 .GetProperty(property.Member.Name)
 .GetValue(source, null);