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.

Was it helpful?

Solution

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

OTHER TIPS

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

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.

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});
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top