What is the best way to apply these changes to an object without duplicating code?

Question

I am trying update a number of properties of one object from another and I wind up repeating this same code over and over again (i am showing an example with Name and LastName but i have 15 other properties with similar code).

But its important to Note that its NOT all properties so i can't blindly just copy everything.

 public class Person
 {

     public bool UpdateFrom(Person otherPerson)
     {

        if (!String.IsNullOrEmpty(otherPerson.Name))
        {
            if (Name!= otherPerson.Name)
            {
                change = true;
                Name = otherPerson.Name;
            }
        }

       if (!String.IsNullOrEmpty(otherPerson.LastName))
        {
            if (LastName!= otherPerson.LastName)
            {
                change = true;
                LastName = otherPerson.LastName;
            }
        }

        return change;
     }
  }

is there a more elegant way to writing this code?

Check out https://github.com/AutoMapper/AutoMapper - it might be useful for this. Also see http://stackoverflow.com/questions/5713556/copy-object-to-object-with-automapper — Matthew Watson, Apr 06 '13 at 17:38
This post may help: http://stackoverflow.com/questions/129389/how-do-you-do-a-deep-copy-an-object-in-net-c-specifically — chue x, Apr 06 '13 at 17:40
@chuex: He isn't making a copy (which creates a new instance), he's overwriting an *existing* instance. But Copy/Clone semantics may in fact be a better approach, so worth looking at. — Matthew Watson, Apr 06 '13 at 17:41
@MatthewWatson - understood. I thought there was something worth looking at there. — chue x, Apr 06 '13 at 17:44
You could also retrieve all the `PropertyInfo`s that you *do* want to check like this, and add them into a list, using `Type.GetProperty`. Then `foreach` over that list. — dialer, Apr 06 '13 at 17:45
@dialer - I don't want to do EVERY property (as per the updated question) — leora, Apr 06 '13 at 17:45
@leora You can retrieve properties one by one; see my other comment. Unfortunately, there is no refactoring-safe way to do that though. // Actually you could introduce an attribute and apply it to only the properties you want to clone. Then, you can use `Type.GetProperties()` and check for the attribute. This would be refactroing-safe. — dialer, Apr 06 '13 at 17:49
You should be able to reduce each block down to `UpdateStringFrom(this, otherPerson, x => x.LastName);` and a helper function. — Ben Voigt, Apr 06 '13 at 17:54
Also see here for a property-copying implementation that you could modify with the idea from @dialer about decorating with a special attribute the properties that you want copied: http://stackoverflow.com/questions/2624823/copy-values-from-one-object-to-another — Matthew Watson, Apr 06 '13 at 17:56
Another option would be to use code generation, such a [T4 Text Template (.tt)](http://msdn.microsoft.com/en-us/library/vstudio/dd820620%28v=vs.100%29.aspx). It's not more elegant but it is less tedious. — Tom Blodget, Apr 06 '13 at 18:05

Ian Mercer · Answer 1 · 2013-04-06T18:15:00.667

You could use an Expression to define which field you want to access, the code to handle the updates would look like this:-

   Person one = new Person {FirstName = "First", LastName = ""};
   Person two = new Person {FirstName = "", LastName = "Last"};
   Person three = new Person ();

   bool changed = false;
   changed = SetIfNotNull(three, one, p => p.FirstName) || changed;
   changed = SetIfNotNull(three, one, p => p.LastName) || changed;
   changed = SetIfNotNull(three, two, p => p.FirstName) || changed;
   changed = SetIfNotNull(three, two, p => p.LastName) || changed;

Note that the order in the || expression matters since .NET will short-circuit the evaluation if it can. Or as Ben suggests in the comments below, use changed |= ... as a simpler alternative.

The SetIfNotNull method relies on this other method that does a bit of Expression magic to convert a getter ino a setter.

    /// <summary>
    /// Convert a lambda expression for a getter into a setter
    /// </summary>
    public static Action<T, U> GetSetter<T, U>(Expression<Func<T, U>> expression)
    {
        var memberExpression = (MemberExpression)expression.Body;
        var property = (PropertyInfo)memberExpression.Member;
        var setMethod = property.GetSetMethod();

        var parameterT = Expression.Parameter(typeof(T), "x");
        var parameterU = Expression.Parameter(typeof(U), "y");

        var newExpression =
            Expression.Lambda<Action<T, U>>(
                Expression.Call(parameterT, setMethod, parameterU),
                parameterT,
                parameterU
            );

        return newExpression.Compile();
    }


    public static bool SetIfNotNull<T> (T destination, T source, 
                            Expression<Func<T, string>> getter)
    {
        string value = getter.Compile()(source);
        if (!string.IsNullOrEmpty(value))
        {
            GetSetter(getter)(destination, value);
            return true;
        }
        else
        {
            return false;
        }
    }

Yep, that's what my comment was suggesting. Note that you can also do `changed |= ...` without short-circuiting. — Ben Voigt, Apr 06 '13 at 18:10

score 0 · Answer 2 · answered Apr 06 '13 at 18:05

Using Func and Action delegates you can do it like this:

public class Person
{
    public string Name { get; set; }

    public string LastName { get; set; }

    public bool UpdateFrom(Person otherPerson)
    {
        bool change = false;

        change = Check(otherPerson.Name, p => p.Name, (p, val) => p.Name = val);

        change = change ||
                 Check(otherPerson.LastName, p => p.LastName, (p, val) => p.LastName = val);

        return change;
    }

    public bool Check(string value, Func<Person, string> getMember, Action<Person, string> action)
    {
        bool result = false;

        if (!string.IsNullOrEmpty(value))
        {
            if (getMember(this) != value)
            {
                result = true;
                action(this, value);
            }
        }

        return result;
    }
}

Your change expression will 'short circuit', should put `|| change` at the end. — Ian Mercer, Apr 06 '13 at 18:10

score 0 · Answer 3 · answered Apr 06 '13 at 18:07

You can use reflecton to do it.. here's an example implementation (need to add extra code to handle arrays etc.)

    public class Person
    {
        public bool UpdateFromOther(Person otherPerson)
        {
            var properties =
                this.GetType()
                    .GetProperties(
                        BindingFlags.Instance | BindingFlags.Public | BindingFlags.SetProperty
                        | BindingFlags.GetProperty);


            var changed = properties.Any(prop =>
            {
                var my = prop.GetValue(this);
                var theirs = prop.GetValue(otherPerson);
                return my != null ? !my.Equals(theirs) : theirs != null;
            });

            foreach (var propertyInfo in properties)
            {
                propertyInfo.SetValue(this, propertyInfo.GetValue(otherPerson));
            }

            return changed;
        }

        public string Name { get; set; }
    }

    [Test]
    public void Test()
    {
        var instance1 = new Person() { Name = "Monkey" };
        var instance2 = new Person() { Name = "Magic" };
        var instance3 = new Person() { Name = null};

        Assert.IsFalse(instance1.UpdateFromOther(instance1), "No changes should be detected");
        Assert.IsTrue(instance2.UpdateFromOther(instance1), "Change is detected");
        Assert.AreEqual("Monkey",instance2.Name, "Property updated");
        Assert.IsTrue(instance3.UpdateFromOther(instance1), "Change is detected");
        Assert.AreEqual("Monkey", instance3.Name, "Property updated");

    }

incidentally.. there are other more efficient ways of doing it (look up compiled expression trees and assembly.emit) — Mo the Conqueror, Apr 06 '13 at 18:09
forgot to read the "Blindly Copy" bit.. but AOP is your friend here.. just a new attribute to the properties that you want.. then add a Where predicate (Property.GetAttribute != null) — Mo the Conqueror, Apr 06 '13 at 18:15

score 0 · Answer 4 · answered Apr 06 '13 at 18:12

This is just my comment typed out, you can refer to the comments to your question about further details about this technique.

Define this class:

[AttributeUsage(AttributeTargets.Property)]
public sealed class CloningAttribute : Attribute
{
}

In your Person class:

[Cloning] // <-- applying the attribute only to desired properties
public int Test { get; set; }

public bool Clone(Person other)
{
    bool changed = false;
    var properties = typeof(Person).GetProperties();
    foreach (var prop in properties.Where(x => x.GetCustomAttributes(typeof(CloningAttribute), true).Length != 0))
    {
        // get current values
        var myValue = prop.GetValue(this, null);
        var otherValue = prop.GetValue(other, null);
        if (prop.PropertyType == typeof(string))
        {
            // special treatment for string:
            // ignore if null !!or empty!!
            if (String.IsNullOrEmpty((string)otherValue))
            {
                continue;
            }
        }
        else
        {
            // do you want to copy if the other value is null?
            if (otherValue == null)
            {
                continue;
            }
        }

        // compare and only check 'changed' if they are different
        if (!myValue.Equals(otherValue))
        {
            changed = true;
            prop.SetValue(this, otherValue, null);
        }
    }
    return changed;
}

score 0 · Answer 5 · answered Apr 06 '13 at 18:34

You can create generic rewriting tool with will look on properties with particular attribute:

 public class Updater
    {
        public static bool Update(object thisObj, object otherObj)
        {
            IEnumerable<PropertyInfo> props = thisObj.GetType().GetProperties().Where(
                prop => Attribute.IsDefined(prop, typeof(UpdateElementAttribute)));

            bool change = false;
            foreach (var prop in props)
            {
                object value = prop.GetValue(otherObj);

                if (value != null && (value is string || string.IsNullOrWhiteSpace((string)value)))
                {
                    if (!prop.GetValue(thisObj).Equals(value))
                    {
                        change = true;
                        prop.SetValue(thisObj, value);
                    }
                }
            }
            return change;
        }
    }

And then just use it:

public class Person
    {
        public bool UpdateFrom(Person otherPerson)
        {
            return Updater.Update(this, otherPerson);
        }
        [UpdateElement]
        public string Name { get; set; }
        [UpdateElement]
        public string LastName { get; set; }
    }

What is the best way to apply these changes to an object without duplicating code?

5 Answers5