How to simplify repeating if-then-assign construction?

Question

I have the following method:

protected override bool ModifyExistingEntity(Product entity, ProductModel item) {     bool isModified = false;      if (entity.Title != item.Title)     {         isModified = true;         entity.Title = item.Title;     }      if (entity.ServerId != item.Id)     {         isModified = true;         entity.ServerId = item.Id;     }      return isModified; } 

I wonder if you could suggest a better way to implement the method.

The problem is obvious: 5 lines of almost copy-pasted code per property is too much. May be there's a solution using Func-s / Expression-s out of my vision.

Answer 1

You have a case of temporal coupling there, i.e. you're mixing the check whether the entity has changed with the assignments. If you separate the two, your code becomes much cleaner:

protected override bool ModifyExistingEntity(Product entity, ProductModel item) {     bool isModified = this.IsEntityModified(entity, item);      if (isModified)     {         this.UpdateEntity(entity, item);     }      return isModified; }  private bool IsEntityModified(Product entity, ProductModel item) {     return entity.Title != item.Title || entity.ServerId != item.ServerId; }  private void UpdateEntity(Product entity, ProductModel item) {     entity.Title = item.Title;     entity.ServerId = item.Id; } 

Doing any smart and funky stuff (TM) with Func<> or anything like that, doesn't seem helpful in this case as it wouldn't transport your intention as clearly.

Answer 2

Something like this should work

protected bool ModifyExistingEntity(Person entity, ProductModel item) {     bool isModified = CompareAndModify(() => entity.Title = item.Title, () => entity.Title != item.Title);     isModified |= CompareAndModify(() => entity.ServerId = item.Id, () => entity.ServerId != item.Id);      return isModified; }  private bool CompareAndModify(Action setter, Func<bool> comparator) {     if (comparator())     {         setter();         return true;     }     return false; } 

Not sure if this is readable. It is subjective.