Refactor To Eliminate Repetition In Lambda Expression

Question

These two methods exhibit repetition:

public static Expression<Func<Foo, FooEditDto>> EditDtoSelector()
{
 return f => new FooEditDto
 {
 PropertyA = f.PropertyA,
 PropertyB = f.PropertyB,
 PropertyC = f.PropertyC,
 PropertyD = f.PropertyD,
 PropertyE = f.PropertyE
 };
}
public static Expression<Func<Foo, FooListDto>> ListDtoSelector()
{
 return f => new FooDto
 {
 PropertyA = f.PropertyA,
 PropertyB = f.PropertyB,
 PropertyC = f.PropertyC
 };
}

How can I refactor to eliminate this repetition?

UPDATE: Oops, I neglected to mention an important point. FooEditDto is a subclass of FooDto.

• Tim Scott: See the note I added. FooEditDto is a subclass of FooDto.

Answer 1

Well, I have a really horrible way you could do it.

You could write a method which used reflection (bear with me!) to work out all the properties for a particular type, and built a delegate (using Reflection.Emit) to copy properties from that type to another. Then use an anonymous type to make sure you only need to build the copying delegate once, so it's fast. Your method would then look like this:

public static Expression<Func<Foo, FooEditDto>> EditDtoSelector()
{
 return f => MagicCopier<FooEditDto>.Copy(new { 
 f.PropertyA, f.PropertyB, f.PropertyC, f.PropertyD, f.PropertyC
 });
}

The nuances here:

• MagicCopier is a generic type and Copy is a generic method so that you can explicitly specify the "target" type, but implicitly specify the "source" type.
• It's using a projection initializer to infer the names of the properties from the expressions used to initialize the anonymous type

I'm not sure whether it's really worth it, but it's quite a fun idea... I may have to implement it anyway :)

EDIT: With MemberInitExpression we could do it all with an expression tree, which makes it a lot easier than CodeDOM. Will give it a try tonight...

EDIT: Done, and it's actually pretty simple code. Here's the class:

/// <summary>
/// Generic class which copies to its target type from a source
/// type specified in the Copy method. The types are specified
/// separately to take advantage of type inference on generic
/// method arguments.
/// </summary>
public static class PropertyCopy<TTarget> where TTarget : class, new()
{
 /// <summary>
 /// Copies all readable properties from the source to a new instance
 /// of TTarget.
 /// </summary>
 public static TTarget CopyFrom<TSource>(TSource source) where TSource : class
 {
 return PropertyCopier<TSource>.Copy(source);
 }
 /// <summary>
 /// Static class to efficiently store the compiled delegate which can
 /// do the copying. We need a bit of work to ensure that exceptions are
 /// appropriately propagated, as the exception is generated at type initialization
 /// time, but we wish it to be thrown as an ArgumentException.
 /// </summary>
 private static class PropertyCopier<TSource> where TSource : class
 {
 private static readonly Func<TSource, TTarget> copier;
 private static readonly Exception initializationException;
 internal static TTarget Copy(TSource source)
 {
 if (initializationException != null)
 {
 throw initializationException;
 }
 if (source == null)
 {
 throw new ArgumentNullException("source");
 }
 return copier(source);
 }
 static PropertyCopier()
 {
 try
 {
 copier = BuildCopier();
 initializationException = null;
 }
 catch (Exception e)
 {
 copier = null;
 initializationException = e;
 }
 }
 private static Func<TSource, TTarget> BuildCopier()
 {
 ParameterExpression sourceParameter = Expression.Parameter(typeof(TSource), "source");
 var bindings = new List<MemberBinding>();
 foreach (PropertyInfo sourceProperty in typeof(TSource).GetProperties())
 {
 if (!sourceProperty.CanRead)
 {
 continue;
 }
 PropertyInfo targetProperty = typeof(TTarget).GetProperty(sourceProperty.Name);
 if (targetProperty == null)
 {
 throw new ArgumentException("Property " + sourceProperty.Name 
 + " is not present and accessible in " + typeof(TTarget).FullName);
 }
 if (!targetProperty.CanWrite)
 {
 throw new ArgumentException("Property " + sourceProperty.Name 
 + " is not writable in " + typeof(TTarget).FullName);
 }
 if (!targetProperty.PropertyType.IsAssignableFrom(sourceProperty.PropertyType))
 {
 throw new ArgumentException("Property " + sourceProperty.Name
 + " has an incompatible type in " + typeof(TTarget).FullName);
 }
 bindings.Add(Expression.Bind(targetProperty, Expression.Property(sourceParameter, sourceProperty)));
 }
 Expression initializer = Expression.MemberInit(Expression.New(typeof(TTarget)), bindings);
 return Expression.Lambda<Func<TSource,TTarget>>(initializer, sourceParameter).Compile();
 }
 }

And calling it:

TargetType target = PropertyCopy<TargetType>.CopyFrom(new { First="Foo", Second="Bar" });

• Mark Cidade: you can also just reflect over the types and copy the values by matching property names, using the getters/setters off of the PropertyInfos
• Jon Skeet: Yes, but that uses reflection every time, which is pretty painfully slow. Given that you can cache the compiled expression tree to copy the values over, that will be much more efficient. Given that the library code only needs to be written once, it might as well be done properly :)
• bytebender: Thanks for the post... I could see where I might use this at some point.
• bytebender: Would it be possible to return Expression<Func<TSource, TTarget>> instead?
• bytebender: I was wondering if you would be able to take a look at a similar question regarding your solution here: stackoverflow.com/questions/1093449/…

Answer 2

If FooEditDto is a sublass of FooDto and you don't need the MemberInitExpressions, use a constructor:

class FooDto
 { public FooDto(Bar a, Bar b, Bar c) 
 { PropertyA = a;
 PropertyB = b;
 PropertyC = c;
 }
 public Bar PropertyA {get;set;}
 public Bar PropertyB {get;set;}
 public Bar PropertyC {get;set;}
 }
class FooEditDto : FooDto
 { public FooEditDto(Bar a, Bar b, Bar c) : base(a,b,c)
 public Bar PropertyD {get;set;}
 public Bar PropertyE {get;set;}
 }
public static Expression<Func<Foo, FooEditDto>> EditDtoSelector()
{
 return f => new FooEditDto(f.PropertyA,f.PropertyB,f.PropertyC)
 {
 PropertyD = f.PropertyD,
 PropertyE = f.PropertyE
 };
}

Answer 3

The repetition is in the names, but C# has no idea that PropertyA in one class is connected with PropertyA in another. You have to make the connection explicitly. The way you did it works fine. If you had enough of these, you might consider using reflection to write one method that could do this for any pair of classes.

Do pay attention to the performance implications of whatever method you choose. Reflection by itself is slower. However, you could also use reflection to emit IL that would, once it's emitted, run just as fast as what you have written. You could also generate an expression tree and convert it to a compiled delegate. These techniques are somewhat complicated, so you have to weigh the trade-offs.

Answer 4

You can let the caller return their own object of an anonymous type with only the properties they need:

public static Expression<Func<Foo,T>> 
 GetSelector<T>(Expression<Func<Foo,T>> f)
 { return f;
 }
/* ... */
var expr = GetSelector(f => new{f.PropertyA,f.PropertyB,f.PropertyC});