10
\$\begingroup\$

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;
    }
    

    }

\$\endgroup\$
3
  • 3
    \$\begingroup\$ I would vastly prefer the variable name 'item' in your foreach loop than 'thing' \$\endgroup\$
    – Nick Udell
    Nov 10, 2014 at 9:11
  • \$\begingroup\$ ah yep will change - had it on my mind at the time but normally wouldnt use it \$\endgroup\$ Nov 10, 2014 at 13:04
  • \$\begingroup\$ Found fluent assertions which seems to have a solution for the problem - github.com/dennisdoomen/fluentassertions/… \$\endgroup\$ Nov 17, 2014 at 6:58

1 Answer 1

6
+25
\$\begingroup\$

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);
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.