How would you improve this shallow copying class?

Question

I've written a class with a single static method that copies property values from one object to another. It doesn't care what type each object is, only that they have identical properties. It does what I need, so I'm not engineering it further, but what improvements would you make?

Here's the code:

public class ShallowCopy
{
    public static void Copy<From, To>(From from, To to)
        where To : class
        where From : class
    {
        Type toType = to.GetType();
        foreach (var propertyInfo in from.GetType().GetProperties(BindingFlags.GetProperty | BindingFlags.Public | BindingFlags.Instance))
        {
            toType.GetProperty(propertyInfo.Name).SetValue(to, propertyInfo.GetValue(from, null), null);
        }
    }
}

I'm using it as follows:

EmployeeDTO dto = GetEmployeeDTO();
Employee employee = new Employee();
ShallowCopy.Copy(dto, employee);

Answer 1

A new method that created a new instance of To and called the Copy() method before returning might be useful.

Like this:

public static To Create<From, To>(From from)
    where To : class, new()
    where From : class
{
    var to = new To();
    Copy(from, to);
    return to;
}

Answer 2

Are your DTOs serializable? I would expect so, in which case:

MemberInfo[] sm = FormatterServices.GetSerializableMembers(typeof(From));
object[] data = FormatterServices.GetObjectData(from, sm);
FormatterServices.PopulateObjectMembers(to, sm, data);

But note that I don't really agree with this general approach. I would prefer a strong contract for copying on your DTOs that each DTO implements.

HTH, Kent

Answer 3

Decide what you want to do if passed objects of types that share some properties but not all. Check for the existence of the property in the From object in the To object before trying to set it's value. Do the "right thing" when you come to a property that doesn't exist. If all of the public properties need to be identical, then you will need to check if you've set all of them on the To object and handle the case where you haven't appropriately.

I'd also suggest that you may want to use attributes to decorate the properties that need to be copied and ignore others. This would allow you to go back and forth between the two different objects more easily and continue to maintain some public properties that are derived rather than stored on your business object.

Answer 4

• Change your type parameter names to comply with naming conventions, e.g. TFrom and TTo, or TSource and TDest (or TDestination).

• Do most of your work in a generic type instead of in just a generic method. That allows you to cache the properties, as well as allowing type inference. Type inference is important on the "TFrom" parameter, as it will allow anonymous types to be used.

• You could potentially make it blindingly fast by dynamically generating code to do the property copying and keeping it in a delegate which is valid for the "from" type. Or potentially generate it for every from/to pair, which would mean the actual copying wouldn't need to use reflection at all! (Preparing the code would be a one-time hit per pair of types, but hopefully you wouldn't have too many pairs.)