Ответ довольно длинный, поэтому будьте терпеливы, хотя каждый шаг рефакторинга со мной.
Вариант А определенно лучше, потому что вы не дублируете поведение в производных классах, однако мне интересно, какова функция Update(),Точно такое же поведение во время выполнения может быть получено с помощью:
abstract class WorkItem {
public int X {
get => _x;
set {
Validate(value);
_x = value;
}
}
private int _x;
}
Для чего вам нужен отдельный метод?Вымышленные имена затрудняют рассуждение о вашей логике, но это может иметь смысл, если Update() на самом деле является действием высокого уровня, в этом случае может сработать комбинация из вышеперечисленного:
abstract class WorkItem {
public int Quantity {
get => _quantity;
protected set {
Validate(value);
_quantity = value;
}
}
// Other properties
// Single method because you cannot change quantity without
// affecting discount (and vice-versa).
UpdateOrder(int newQuantity, float discount) {
Quantity = newQuantity;
Discount = discount; // Discount validation is maybe affected by Quantity
}
private int _quantity;
}
С гипотетическим производным классом:
sealed class InternationalOrder : Workitem {
public UpdateOrder(int quantity, float discount, string address) {
Address = address;
// Address may affect discount eligibility
UpdateOrder(quantity, discount);
}
}
Конечно, это выдумка, но вы должны полагаться на существующее поведение, когда это возможно.Более того, не должно быть универсального UpdateXyz() метода (и часто даже не общедоступных сеттеров), а действий , подобных ChangeDeliveryDate(), Cancel() или Postpone().Преимущество состоит в том, чтобы абстрагировать государство от бизнес-правил.
Что, если проверка дорогостоящая или существует много внутренних зависимостей?В предыдущем примере порядок, в котором вы устанавливаете свойства, имеет значение, и это плохая вещь .Вы можете проверить один раз все вместе:
abstract class WorkItem {
public virtual void Validate() {
if (Discount > MaximumDiscount)
throw new InvalidOperationException("Discount is way to high!");
if (Quantity < MinimumQuantityForDiscount)
throw new InvalidOperationException("Order is not eligible for discount.");
}
}
sealed class OnlineOrder : WorkItem {
public override Validate() {
base.Validate();
// More validation rules, specific for OnlineOrder
}
}
Базовая проверка свойств НЕ является частью логики DDD, и IMHO должен быть на месте:
public int Quantity {
get => _quantity;
set {
if (value < 0)
throw new ArgumentOutOfRangeException("...");
_quantity = value;
}
}
Когда Validate() должно бытьназывается?
- Каждый раз при изменении свойства (внутренняя реализация механизма похожа на
INotifyPropertyChanged).Это самый простой способ, но он может повлиять на производительность, если проверка очень сложная или медленная (например, с тысячами бизнес-правил, возможно, с данными из внешних источников данных. Обратите внимание, что проверка бизнес-правил не должна быть ответственностью класса за любой нетривиальный сценарий).. - Вручную внутри вашего
UpdateTicket() метода перед вызовом SaveChanges(). Очевидным недостатком является то, что вызывающая сторона может забыть вызвать его. - Комбинация вышеперечисленного.
Я, очевидно, предпочитаю третий вариант: сделать его автоматическим и иметь механизм для временного отключения проверки при выполнении массового обновления. Подтверждение концепции:
abstract class WorkItem {
public int Quantity {
get => _quantity;
protected set {
if (value < 0)
throw new ArgumentOutOfRangeException("...");
if (_quantity != value) {
_quantity = value;
Validate();
}
}
}
public virtual void Validate() {
if (_isValidationDisabled)
return;
if (Discount > MaximumDiscount)
throw new InvalidOperationException("Discount is way to high!");
if (Quantity < MinimumQuantityForDiscount)
throw new InvalidOperationException("Order is not eligible for discount.");
}
public void BeginUpdate() {
_isValidationDisabled = true;
}
public void EndUpdate() {
_isValidationDisabled = false;
Validate();
}
private bool _isValidationDisabled;
private int _quantity;
}
Конечно, вы хотите извлечь повторно используемый метод SetBackingStore() для установки значения поля и вызвать Validate().
Он будет использоваться следующим образом:
public void UpdateTicket(int Id) {
var ticket = _context.Tickets.Where(c => c.Id == Id).SingleOrDefault();
if (ticket == null) { /* no ticket? */ }
ticket.BeginUpdate();
tickets.Status = TicketStatus.Resolved;
ticked.ResolvedAt = DateTime.UtcNow;
ticket.EndUpdate();
_context.SaveChanges();
}
Заключительная мысль: этот BeginUpdate() / EndUpdate() кортеж действительно беспокоит меня (что в случае ошибок? Что если вызывающий забудет вызвать EndUpdate()?) Создать простой класс IDisposableэто делает работу за вас (пусть это будет вложенный класс, чтобы иметь доступ к приватному состоянию), чтобы иметь возможность писать:
public void UpdateTicket(int Id) {
var ticket = _context.Tickets.Where(c => c.Id == Id).SingleOrDefault();
if (ticket == null) { /* no ticket? */ }
using (ticket.BeginUpdate()) {
tickets.Status = TicketStatus.Resolved;
ticked.ResolvedAt = DateTime.UtcNow;
}
_context.SaveChanges();
}
У нас все еще есть неприятный запах, эти сеттеры (или эквивалентные методы Java-стиль типа UpdateStatus(TicketStatus.Resolved, DateTime.Now)) - упущенный шанс ввести абстракцию:
public void UpdateTicket(int Id) {
var ticket = _context.Tickets.Where(c => c.Id == Id).SingleOrDefault();
if (ticket == null) { /* no ticket? */ }
ticket.Close();
_context.SaveChanges();
}
Метод Close может быть реализован точно так же, как и в предыдущем примере (не забудьте повторно использовать существующие поведения всякий раз, когда это возможно):
void Close() {
tickets.Status = TicketStatus.Resolved;
ticked.ResolvedAt = DateTime.UtcNow;
Validate();
}
Не всегда возможно иметь такие высокоуровневые методы (например, при редактировании свойств объекта ...), а затем сохранять это BeginUpdate() на месте для этих случаев.