Как упростить повторение конструкции if-then-assign?
У меня есть следующий метод:
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;
}
Интересно, можете ли вы предложить лучший способ реализовать метод.
Проблема очевидна: слишком много строк из почти скопированного кода на одно свойство. Может быть, есть решение, использующее Func -s/Expression -s из моего зрения.
Ответы
Ответ 1
У вас есть временная связь, т.е. вы смешиваете проверку, изменилась ли сущность с назначением. Если вы разделите два, ваш код станет намного чище:
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;
}
Выполнение каких-либо умных и забавных вещей (TM) с помощью Func<> или что-то в этом роде не кажется полезным в этом случае, поскольку оно не будет передавать ваше намерение как можно яснее.
Ответ 2
Что-то вроде этого должно работать
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;
}
Не уверен, что это доступно для чтения. Это субъективно.
Ответ 3
Я думаю, что расширение этого ответа может сработать для вас:
public static bool SetIfModified<CLeft, T>(Expression<Func<CLeft, T>> exprLeft, CLeft leftType, T rightValue)
{
var getterLeft = exprLeft.Compile();
if (EqualityComparer<T>.Default.Equals(getterLeft(leftType), rightValue))
{
var newValueLeft = Expression.Parameter(exprLeft.Body.Type);
var assignLeft = Expression.Lambda<Action<CLeft, T>>(Expression.Assign(exprLeft.Body, newValueLeft), exprLeft.Parameters[0], newValueLeft);
var setterLeft = assignLeft.Compile();
setterLeft(leftType, rightValue);
return true;
}
else
{
return false;
}
}
Требуется выражение для проверки значения. Он компилирует и выполняет его динамически.
Используйте его следующим образом:
public class Product { public string Title { get; set; } }
public class ProductModel { public string Title { get; set; } }
static void Main(string[] args)
{
Product lc = new Product();
ProductModel rc = new ProductModel();
rc.Title = "abc";
bool modified = SetIfModified(l => l.Title, lc, r.Title);
// modified is true
// lc.Title is "abc"
}
Ответ 4
Используйте T4 для метапрограммирования
Другой подход - очень часто, когда мы дублируем код, который на самом деле прост и, вероятно, очень быстрый. В этом случае каждый дублированный блок if не является тем же самым - он содержит небольшое знание - сопоставление от одного свойства к другому.
Досадно писать и поддерживать дублированные блоки.
Один из способов избежать написания полезного повторяющегося кода - это автоматически сгенерировать его.
С моим решением отображение является простым:
var mappings = new []{
new Mapper("ProductModel", "Product")
{
"Title", // ProductModel.Title goes to Product.Title
{"Id", "ServiceId"}, // ProductModel.Id goes to Product.ServiceId
},
};
Вот t4 Текстовый шаблон (встроенная функция для Visual Studio):
<#@ template debug="false" hostspecific="false" language="C#" #>
<#@ assembly name="System.Core" #>
<#@ import namespace="System.Collections.Generic" #>
<#@ output extension=".cs" #>
<#
// Consider including the namespace in the class names.
// You only need to change the mappings.
var product = new Mapper("Product", "ProductEntity") { "Name", {"Id", "ServiceId"} };
var person = new Mapper("Person", "DbPerson") { "Employee", {"Name", "FullName"}, {"Addredd", "HomeAddress"} };
var mappings = new [] {product, person};
#>
// !!!
// !!! Do not modify this file, it is automatically generated. Change the .tt file instead. !!!
// !!!
namespace Your.Mapper
{
partial class Mapper
{
<# foreach(var mapping in mappings) {
#>/// <summary>
/// Set <paramref name="target"/> properties by copying them from <paramref name="source"/>.
/// </summary>
/// <remarks>Mapping:<br/>
<#foreach(var property in mapping){
#>/// <see cref="<#=mapping.SourceType#>.<#=property.SourceProperty#>"/> → <see cref="<#=mapping.TargetType#>.<#=property.TargetProperty#>"/> <br/>
<#}
#>/// </remarks>
/// <returns><c>true</c> if any property was changed, <c>false</c> if all properties were the same.</returns>
public bool ModifyExistingEntity(<#=mapping.SourceType#> source, <#=mapping.TargetType#> target)
{
bool dirty = false;
<# foreach(var property in mapping) {
#>if (target.<#=property.TargetProperty#> != source.<#=property.SourceProperty#>)
{
dirty = true;
target.<#=property.TargetProperty#> = source.<#=property.SourceProperty#>;
}
<#}
#>return dirty;
}
<#
}
#>
}
}
<#+
class Mapper : IEnumerable<PropertyMapper>
{
private readonly List<PropertyMapper> _properties;
public Mapper(string sourceType, string targetType)
{
SourceType = sourceType;
TargetType = targetType;
_properties = new List<PropertyMapper>();
}
public string SourceType { get; set; }
public string TargetType { get; set; }
public void Add(string fieldName)
{
_properties.Add(new PropertyMapper {SourceProperty = fieldName, TargetProperty = fieldName});
}
public void Add(string sourceProperty, string targetProperty)
{
_properties.Add(new PropertyMapper { SourceProperty = sourceProperty, TargetProperty = targetProperty });
}
IEnumerator<PropertyMapper> IEnumerable<PropertyMapper>.GetEnumerator() { return _properties.GetEnumerator(); }
System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { return _properties.GetEnumerator(); }
}
class PropertyMapper
{
public string SourceProperty { get; set; }
public string TargetProperty { get; set; }
}
#>
Этот шаблон генерирует следующий код: https://gist.github.com/kobi/d52dd1ff27541acaae10
Преимущества:
- Тяжелый подъем выполняется во время компиляции (фактически один раз перед компиляцией) - сгенерированный код выполняется быстро.
- Сгенерированный код документирован.
- Простота обслуживания - вы можете изменить все картографы в одной точке.
- Сгенерированные методы документируются.
- Нет ошибки копирования-вставки.
- Это довольно забавно.
Недостатки:
- Использование строк для получения имен свойств. Имейте в виду - это не производственный код, он просто используется для генерации кода. Можно использовать реальные типы и деревья выражений (пример ниже).
- Статический анализ, вероятно, пропустит использование в шаблоне (даже если мы используем выражения, а не все инструменты в tt файлах).
- Многие люди не знают, что происходит.
- Если вы используете выражения, сложно ссылаться на ваши типы.
Примечания:
- Я назвал аргументы
source и target и изменил их порядок, поэтому source всегда первый.
Было некоторое беспокойство, что я использую строки вместо реальных свойств. Хотя это небольшая проблема в этом случае (результат компилируется), вот дополнение, которое работает с вашими реальными объектами.
В верхней части добавить это (третий должен быть вашим пространством имен):
<#@ assembly name="$(TargetPath)" #>
<#@ import namespace="System.Linq.Expressions" #>
<#@ import namespace="ConsoleApplicationT4So29913514" #>
Внизу добавьте:
class Mapper<TSource, TTarget> : Mapper
{
public Mapper()
: base(typeof(TSource).FullName, typeof(TTarget).FullName)
{
}
private static string GetExpressionMemberAccess(LambdaExpression getProperty)
{
var member = (MemberExpression)getProperty.Body;
//var lambdaParameterName = (ParameterExpression)member.Expression;
var lambdaParameterName = getProperty.Parameters[0]; // `x` in `x => x.PropertyName`
var labmdaBody = member.ToString();
//will not work with indexer.
return labmdaBody.Substring(lambdaParameterName.Name.Length + 1); //+1 to remove the `.`, get "PropertyName"
}
public void Add<TProperty>(Expression<Func<TSource, TProperty>> getSourceProperty, Expression<Func<TTarget, TProperty>> getTargetProperty)
{
Add(GetExpressionMemberAccess(getSourceProperty), GetExpressionMemberAccess(getTargetProperty));
}
/// <summary>
/// The doesn't really make sense, but we assume we have <c>source=>source.Property</c>, <c>target=>target.Property</c>
/// </summary>
public void Add<TProperty>(Expression<Func<TSource, TProperty>> getProperty)
{
Add(GetExpressionMemberAccess(getProperty));
}
}
Использование:
var mappings = new Mapper[] {
new Mapper<Student,StudentRecord>
{
{s=>s.Title, t=>t.EntityTitle},
{s=>s.StudentId, t=>t.Id},
s=>s.Name,
s=>s.LuckyNumber,
},
new Mapper<Car,RaceCar>
{
c=>c.Color,
c=>c.Driver,
{c=>c.Driver.Length, r=>r.DriverNameDisplayWidth},
},
};
Весь файл должен выглядеть следующим образом: https://gist.github.com/kobi/6423eaa13cca238447a8
Результат по-прежнему выглядит одинаково: https://gist.github.com/kobi/3508e9f5522a13e1b66b
Примечания:
- Выражения используются только для получения имени свойства как строки, мы не компилируем их и не запускаем.
- В С# 6 мы будем иметь оператор
nameof(), который является хорошим компромиссом между выражениями и цепочками без магии.
Ответ 5
Для упрощения этого нет волшебной палочки.
У вас может быть сама сущность, предоставляющая свойство IsModified, которое затем устанавливается установщиками свойств, например:
public string Title {
get { return _title; }
set {
if (value != _title)
{
_title = value;
IsModified = true;
}
}
}
Если это слишком много, ваше решение в порядке.
Ответ 6
Если вы хотите сделать его доступным для чтения, вы можете создать класс для этой цели с помощью действительно простого использования, избегая повторяющегося кода:
protected override bool ModifyExistingEntity(Product entity, ProductModel item)
{
return new Modifier<Product>(entity)
.SetIfNeeded(e => e.Title, item.Title);
.SetIfNeeded(e => e.ServerId, item.Id);
.EntityWasModified;
}
Реализация:
Я взял код от Патрика Хофмана, чтобы создать сеттер из выражения getter.
public class Modifier<TEntity>
{
public Modifier(TEntity entity)
{
Entity = entity;
}
public TEntity Entity { get; private set; }
public bool EntityWasModified { get; private set; }
public Modifier<TEntity> SetIfNeeded<TProperty>(Expression<Func<TEntity, TProperty>> entityPropertyGetter, TProperty modelValue)
{
var getter = entityPropertyGetter.Compile();
var setter = GetSetterExpression(entityPropertyGetter).Compile();
if (!object.Equals(getter(Entity), modelValue))
{
setter(Entity, modelValue);
EntityWasModified = true;
}
return this;
}
private static Expression<Action<TEntity, TProperty>> GetSetterExpression(Expression<Func<TEntity, TProperty>> getterExpression)
{
var newValue = Expression.Parameter(getterExpression.Body.Type);
return Expression.Lambda<Action<TEntity, TProperty>>(
Expression.Assign(getterExpression.Body, newValue),
getterExpression.Parameters[0], newValue);
}
}
Для повышения производительности вы можете кэшировать результат .Compile.
Ответ 7
Проверьте мое собственное (почти столь же странное, как и остальные) решение
[TestMethod]
public void DifferentTitleAndId_ExpectModified()
{
var entity = new Product
{
Id = 0,
ServerId = 0,
Title = "entity title"
};
var model = new ProductModel
{
Id = 1,
Title = "model title"
};
bool isModified = ModifyExistingEntity(entity, model);
Assert.IsTrue(isModified);
}
protected bool ModifyExistingEntity(Product entity, ProductModel model)
{
return
IsModified(entity.Title, model.Title, x => entity.Title = x) |
IsModified(entity.ServerId, model.Id, x => entity.ServerId = x);
}
protected bool IsModified<T>(T value1, T value2, Action<T> setter)
{
return IsModified(() => value1, () => value2, () => setter(value2));
}
protected bool IsModified<T>(Func<T> valueGetter1, Func<T> valueGetter2, Action setter)
{
if (!Equals(valueGetter1(), valueGetter2()))
{
setter();
return true;
}
return false;
}
Ответ 8
Я видел самые сложные ответы на этот вопрос, но я думаю, что вам лучше всего подходит довольно простое, без излишнего простого решения.
Я бы предположил, что вы используете какой-то шаблон Data Mapper в своей кодовой базе, а Product - ваш объект DAL/Domain, а ProductModel - ваш объект уровня приложения. В этом случае у меня будет просто метод, который сравнивает два (которые впоследствии могут быть перенесены на отдельный слой), а если они не равны, сопоставьте.
Но возникает вопрос: почему вас беспокоит только обновление, если оно изменилось? Вероятно, приемлемо просто обновлять каждый раз.
Кроме того, вы, вероятно, не должны передавать объект в метод с ожиданием его обновления.
Я бы изменил логику следующим образом:
protected bool UpdateIfChanged(Product entity, ProductModel item)
{
var areEqual = CompareProductAndProductModel(entity, item);
if(!areEqual)
UpdateProduct(MapProductModelToProduct(item));
return !areEqual;
}
internal bool CompareProductAndProductModel(Product product, ProductModel productModel)
{
return product.Title == productModel.Title && product.ServerId == productModel.Id; //could be abstracted to an equality comparer if you were inclined
}
Самый большой отход, который этот ответ дает из других ответов, заключается в том, что он не изменяет объект Product. Вместо этого он сравнивает Product и ProductModel, но если изменения обнаружены, он затем использует ProductModel для создания нового Product, который затем передается другому методу DAL, который фактически выполняет обновление. Я считаю, что это, пожалуй, самый удобный подход, так как вам не нужно иметь дело с методами, которые изменяют состояние передаваемых им объектов (неявное соединение, даже если метод и вызывающий существуют в разных местах), что означает, что вам не нужно умственно отслеживать изменения состояния для объекта при выполнении кода во время отладки.
Ответ 9
"Лучше" субъективно в этом контексте. Поскольку вы жаловались на количество строк, у меня есть более сжатый способ:
protected override bool ModifyExistingEntity(Product entity, ProductModel item)
{
bool isModified = false;
isModified |= (entity.Title!= item.Title) ? (entity.Title = item.Title) == item.Title : false;
isModified |= (entity.ServerId != item.Id) ? (entity.ServerId = item.Id) == item.Id : false;
return isModified;
}
Ответ 10
Продолжая отвечать на @bstenzel, не следует ли это делать трюк?
protected override bool ModifyExistingEntity(Product entity, ProductModel item)
{
bool isEntityModified = entity.Title != item.Title || entity.ServerId != item.ServerId;
entity.Title = item.Title;
entity.ServerId = item.Id;
return isEntityModified;
}
Чистый и простой.
Ответ 11
Я думаю, вы ищете способ сравнить содержимое свойств двух объектов. Ваш образец содержит два свойства, но я ожидаю, что ваш реальный код содержит намного больше (поскольку объект Product, вероятно, имеет много свойств).
Вы должны начать с написания метода, который сравнивает ваши два объекта. Для ваших рекомендаций здесь приведены некоторые ВОПРОСЫ, касающиеся вопроса:
Ваш метод будет выглядеть так:
public static bool IsEqualTo<TSource>(this TSource sourceObj, TDestination destinationObj)
where TSource : class
where TDestination : class
{
// Your comparison code goes here
}
Затем вам придется написать второй метод для копирования данных между вашими объектами. Эти вопросы могут помочь вам (проверьте ответ Marc):
- Как глубоко копировать объекты разных типов в С#.NET
Ваш метод будет выглядеть так:
public static bool CopyDataTo<TSource>(this TSource sourceObj, TDestination destinationObj)
where TSource : class
where TDestination : class
{
// Your data copy code goes here
}
Окончательный код будет выглядеть так же просто, как
protected override bool ModifyExistingEntity(Product entity, ProductModel item)
{
if (!entity.IsEqualTo(item))
{
item.CopyDataTo(entity);
return true;
}
return false;
}
Ответ 12
Я не уверен, что класс Product является расширяемым для вас, и я не уверен, что вы ищете "причудливый" ответ или просто более простой... но если это так, вы можете все вокруг бит и поместить логику в класс Product; imho вы получите довольно читаемый метод:
protected override bool ModifyExistingEntity(Product entity, ProductModel item)
{
entity.SetTitle(item.Title);
entity.SetServerId(item.Id);
return entity.WasModified();
}
Добавленный бонус заключается в том, что вы аккуратно инкапсулируете поведение в Product (а также проверку и т.д.).
public partial class Product
{
public void SetTitle(string title)
{
if(this.Title!=title) //and other validation, etc
{
this.Title = title;
Modified();
}
}
public void SetServerId(int serverId)
{
if(this.ServerId!=serverId)
{
this.ServerId=serverID;
Modified();
}
}
private bool _wasModified;
private void Modified()
{
//Or implement INotifyPropertyChanged if you like
_wasModified=true;
}
public bool WasModified()
{
return _wasModified;
}
}
Конечно, если вам не нужна какая-либо "бизнес-логика", и это действительно просто непроверенная операция сопоставления, любой из очень умных ответов здесь будет делать:)
Ответ 13
Поскольку вы, скорее всего, не хотите писать код для каждой пары сущностей и моделей, вы должны полагаться на отражение, чтобы сопоставить свойства, которые являются одинаковыми между сущностью и моделью, а затем просто сравнить эти значения с свойством Infos.
Изменить: добавлено копирование измененного значения и блокировка для нескольких потоков.
class Program
{
static void Main(string[] args)
{
if (ModifyExistingEntity(new Product { Name = "bar" }, new ProductModel { Name = "test" }))
Console.WriteLine("Product modified");
if (ModifyExistingEntity(new Customer { Number = 1001 }, new CustomerModel { Number = 1002 }))
Console.WriteLine("Customer was modified");
if (!ModifyExistingEntity(new Customer { Number = 1001 }, new CustomerModel { Number = 1001 }))
Console.WriteLine("Customer was not modified");
Console.ReadKey();
}
protected static bool ModifyExistingEntity<TEntity, TModel>(TEntity entity, TModel model)
{
var isModified = false;
GetProperties(entity, model).ForEach(
propertyInfo =>
{
var item2Value = propertyInfo.Item2.GetValue(model, null);
if (Equals(propertyInfo.Item1.GetValue(entity, null), item2Value))
return;
propertyInfo.Item1.SetValue(entity, item2Value);
isModified = true;
});
return isModified;
}
private static readonly object MappingLock = new object();
private static readonly Dictionary<Tuple<Type, Type>, List<Tuple<PropertyInfo, PropertyInfo>>> Mapping =
new Dictionary<Tuple<Type, Type>, List<Tuple<PropertyInfo, PropertyInfo>>>();
protected static List<Tuple<PropertyInfo, PropertyInfo>> GetProperties<TEntity, TModel>(TEntity entity, TModel model)
{
lock (MappingLock)
{
var key = new Tuple<Type, Type>(typeof (TEntity), typeof (TModel));
if (Mapping.ContainsKey(key))
{
return Mapping[key];
}
var modelProperties = typeof (TModel).GetProperties();
var newMapping = (from propertyInfo in typeof (TEntity).GetProperties()
let modelPropertyInfo = modelProperties.SingleOrDefault(mp => mp.Name == propertyInfo.Name)
select new Tuple<PropertyInfo, PropertyInfo>(propertyInfo, modelPropertyInfo))
.ToList();
Mapping.Add(key, newMapping);
return newMapping;
}
}
}
Ответ 14
Если вы просто хотите более короткие строки кода, вы можете скомбинировать его с этими назначениями C-Style:
ModifyExistingEntity( Product entity, ProductModel item )
{
bool isModified = false;
isModified |= ( entity.Title != item.Title )
&& retTrue( entity.Title = item.Title );
isModified |= ( entity.ServerId != item.Id )
&& retTrue( entity.ServerId = item.Id );
return isModified;
}
static bool RetTrue<T>(T dummy) { return true; } // Helper method.
Ответ 15
Это пример использования макросов:
#define CheckAndAssign(dst,src) (dst != src && (dst = src, true))
return ( CheckAndAssign (entity.Title, item.Title)
| CheckAndAssign (entity.ServerId, item.Id));
#undef CheckAndAssign
Хорошо, если язык C, С++, Objective-C или что-то еще с макросами. Надеюсь, что на С++ вы можете каким-то образом превратить его в шаблон.
