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?

Answer 1

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

Answer 2

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

Answer 3

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

    }

Answer 4

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

Answer 5

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