patterncsharpMinor
Object to object mapping verification - is this extension method useful?
Viewed 0 times
thismethodextensionusefulmappingobjectverification
Problem
In our .NET tests we use
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:
I have 2 questions:
-
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)
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(
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:
- Is my code just adding 'noise'?
-
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)
Solution
Minor nitpick first:
I think putting generic type constraints on a new line makes them easier to spot and adds to readability (especially with long signatures) - consider:
I really don't like
This also instantly makes it easy to find
I like that you're using
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",
In this block:
The condition
The
Just use
I haven't tested it, but the type parameter in
You'd have that:
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
The other has it like that:
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:
public static T2 ToDto(this T1 obj, params Expression>[] 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(this T1 obj, params Expression>[] 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.netThat 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 seems superfluous as well - Activator.CreateInstance() will create a TResult object.. that you can readily return - so instead of this:var json = JsonConvert.SerializeObject(value);
var anon = JsonConvert.DeserializeAnonymousType(json, Activator.CreateInstance());
return ((JObject)anon).ToObject();You'd have that:
var json = JsonConvert.SerializeObject(value);
return JsonConvert.DeserializeAnonymousType(json, Activator.CreateInstance());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);Code Snippets
public static T2 ToDto<T1,T2>(this T1 obj, params Expression<Func<T1, dynamic>>[] items) where T1 : class
{public static T2 ToDto<T1,T2>(this T1 obj, params Expression<Func<T1, dynamic>>[] items)
where T1 : class
{string json = JsonConvert.SerializeObject(value); // need json.netif (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);
}if (member != null && body.Member is PropertyInfo)
{
var property = body.Member as PropertyInfo;
properties[property.Name] = source.GetType().GetProperty(property.Name).GetValue(source, null);
}Context
StackExchange Code Review Q#69359, answer score: 6
Revisions (0)
No revisions yet.