SOLID, избегая анемичных доменов, внедрение зависимости?

33

Хотя это может быть независимый от языка программирования вопрос, мне интересны ответы, нацеленные на экосистему .NET.

Это сценарий: предположим, нам нужно разработать простое консольное приложение для публичного администрирования. Приложение о транспортном налоге. У них (только) есть следующие бизнес-правила:

1.a) Если транспортное средство является автомобилем и последний раз его владелец оплачивал налог 30 дней назад, то владелец должен заплатить снова.

1.b) Если транспортное средство является мотоциклом, и последний раз, когда его владелец платил налог, был 60 дней назад, то владелец должен заплатить снова.

Другими словами, если у вас есть машина, которую вы должны платить каждые 30 дней, или если у вас есть мотоцикл, вы должны платить каждые 60 дней.

Для каждого транспортного средства в системе приложение должно проверить эти правила и распечатать те транспортные средства (номерной знак и информация о владельце), которые не удовлетворяют им.

Что я хочу это:

2.a) Соответствовать принципам SOLID (особенно принципу Open / Closed).

Чего я не хочу, так это (я думаю):

2.b) Анемичная область, поэтому бизнес-логика должна идти внутри бизнес-объектов.

Я начал с этого:

public class Person
// You wanted a banana but what you got was a gorilla holding the banana and the entire jungle.
{
    public string Name { get; set; }

    public string Surname { get; set; }
}

public abstract class Vehicle
{
    public string PlateNumber { get; set; }

    public Person Owner { get; set; }

    public DateTime LastPaidTime { get; set; }

    public abstract bool HasToPay();
}

public class Car : Vehicle
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 30;
    }
}

public class Motorbike : Vehicle
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 60;
    }
}

public class PublicAdministration
{
    public IEnumerable<Vehicle> GetVehiclesThatHaveToPay()
    {
        return this.GetAllVehicles().Where(vehicle => vehicle.HasToPay());
    }

    private IEnumerable<Vehicle> GetAllVehicles()
    {
        throw new NotImplementedException();
    }
}

class Program
{
    static void Main(string[] args)
    {
        PublicAdministration administration = new PublicAdministration();
        foreach (var vehicle in administration.GetVehiclesThatHaveToPay())
        {
            Console.WriteLine("Plate number: {0}\tOwner: {1}, {2}", vehicle.PlateNumber, vehicle.Owner.Surname, vehicle.Owner.Name);
        }
    }
}

2.a: принцип Open / closed гарантирован; если они хотят налог на велосипед, который вы просто наследуете от Vehicle, тогда вы переопределяете метод HasToPay, и все готово. Открытый / закрытый принцип удовлетворяется посредством простого наследования.

«Проблемы»:

3.а) Почему транспортное средство должно знать, должно ли оно платить? Не является ли проблема общественной администрации? Если правила государственной администрации об изменении налога на автомобиль, почему автомобиль должен измениться? Я думаю, что вы должны спросить: «Тогда почему вы поместили метод HasToPay в Vehicle?», И ответ таков: потому что я не хочу проверять тип транспортного средства (typeof) в PublicAdministration. Вы видите лучшую альтернативу?

3.b) Может быть, в конце концов, уплата налогов - это проблема транспортного средства, или, может быть, мой первоначальный дизайн Person-Vehicle-Car-Car-Motorbike-PublicAdministration просто ошибочен, или мне нужен философ и лучший бизнес-аналитик. Решение может быть следующим: переместить метод HasToPay в другой класс, мы можем назвать его TaxPayment. Затем мы создаем два производных класса TaxPayment; CarTaxPayment и MotorbikeTaxPayment. Затем мы добавляем абстрактное свойство Payment (типа TaxPayment) в класс Vehicle и возвращаем правильный экземпляр TaxPayment из классов Car и Motorbike:

public abstract class TaxPayment
{
    public abstract bool HasToPay();
}

public class CarTaxPayment : TaxPayment
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 30;
    }
}

public class MotorbikeTaxPayment : TaxPayment
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 60;
    }
}

public abstract class Vehicle
{
    public string PlateNumber { get; set; }

    public Person Owner { get; set; }

    public DateTime LastPaidTime { get; set; }

    public abstract TaxPayment Payment { get; }
}

public class Car : Vehicle
{
    private CarTaxPayment payment = new CarTaxPayment();
    public override TaxPayment Payment
    {
        get { return this.payment; }
    }
}

public class Motorbike : Vehicle
{
    private MotorbikeTaxPayment payment = new MotorbikeTaxPayment();
    public override TaxPayment Payment
    {
        get { return this.payment; }
    }
}

И мы вызываем старый процесс следующим образом:

    public IEnumerable<Vehicle> GetVehiclesThatHaveToPay()
    {
        return this.GetAllVehicles().Where(vehicle => vehicle.Payment.HasToPay());
    }

Но теперь этот код не будет компилироваться, потому что внутри CarTaxPayment / MotorbikeTaxPayment / TaxPayment нет члена LastPaidTime. Я начинаю видеть, что CarTaxPayment и MotorbikeTaxPayment больше похожи на «реализации алгоритмов», а не на бизнес-объекты. Каким-то образом теперь этим «алгоритмам» нужно значение LastPaidTime. Конечно, мы можем передать значение конструктору TaxPayment, но это нарушит инкапсуляцию транспортного средства, или обязанности, или как бы это ни назвали эти евангелисты ООП, не так ли?

3.c) Предположим, у нас уже есть Entity Framework ObjectContext (который использует объекты нашего домена; Person, Vehicle, Car, Motorbike, PublicAdministration). Как бы вы сделали, чтобы получить ссылку ObjectContext из метода PublicAdministration.GetAllVehicles и, следовательно, реализовать функциональность?

3.d) Если нам также нужна та же ссылка ObjectContext для CarTaxPayment.HasToPay, но другая для MotorbikeTaxPayment.HasToPay, кто отвечает за «инъекцию» или как вы передадите эти ссылки в классы платежей? В этом случае, избегая анемичного домена (и, следовательно, никаких сервисных объектов), не приведет ли нас к катастрофе?

Каковы ваши варианты дизайна для этого простого сценария? Ясно, что примеры, которые я показал, «слишком сложны» для простой задачи, но в то же время идея состоит в том, чтобы придерживаться принципов SOLID и предотвращать анемичные домены (из которых было поручено уничтожить).

Дэвид Роберт Джонс
источник

Ответы:

15

Я бы сказал, что ни человек, ни транспортное средство не должны знать, подлежит ли оплата.

пересмотр проблемной области

Если вы посмотрите на проблемную область «люди, имеющие автомобили»: для того, чтобы купить автомобиль, у вас есть какой-то договор, который передает право собственности от одного человека другому, при этом ни автомобиль, ни человек не меняются. Вашей модели этого совершенно не хватает.

мысленный эксперимент

Предполагая, что есть супружеская пара, мужчине принадлежит машина, и он платит налоги. Теперь мужчина умирает, его жена наследует машину и будет платить налоги. Тем не менее, ваши тарелки останутся прежними (по крайней мере, в моей стране они будут). Это все та же машина! Вы должны быть в состоянии смоделировать это в вашем домене.

=> Собственность

Автомобиль, который никому не принадлежит, все еще является автомобилем, но никто не будет платить за него налоги. На самом деле вы платите не налоги за автомобиль, а за привилегию владения автомобилем . Таким образом, мое предложение будет:

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

    public string Surname { get; set; }

    public List<Ownership> getOwnerships();

    public List<Vehicle> getVehiclesOwned();
}

public abstract class Vehicle
{
    public string PlateNumber { get; set; }

    public Ownership currentOwnership { get; set; }

}

public class Car : Vehicle {}

public class Motorbike : Vehicle {}

public abstract class Ownership
{
    public Ownership(Vehicle vehicle, Owner owner);

    public Vehicle Vehicle { get;}

    public Owner CurrentOwner { get; set; }

    public abstract bool HasToPay();

    public DateTime LastPaidTime { get; set; }

   //Could model things like payment history or owner history here as well
}

public class CarOwnership : Ownership
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 30;
    }
}

public class MotorbikeOwnership : Ownership
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 60;
    }
}

public class PublicAdministration
{
    public IEnumerable<Ownership> GetVehiclesThatHaveToPay()
    {
        return this.GetAllOwnerships().Where(HasToPay());
    }

}

Это далеко от совершенства и, возможно, даже не корректно удаленно корректирует код C #, но я надеюсь, что вы поняли идею (и кто-то может это исправить). Единственным недостатком является то, что вам, вероятно, понадобится фабрика или что-то, чтобы создать правильное CarOwnershipмежду а CarиPerson

sebastiangeiger
источник
Я думаю, что само транспортное средство должно регистрировать уплаченные за него налоги, а физические лица должны регистрировать налоги, уплаченные за все свои транспортные средства. Налоговый кодекс может быть написан так, чтобы, если налог на автомобиль был уплачен 1 июня, а он был продан 10 июня, новый владелец мог бы быть ответственным за налог 10 июня, 1 июля или 10 июля (и даже если он в настоящее время одним способом это может измениться). Если правило 30 дней остается неизменным для автомобиля даже при смене владельца, но автомобили, которые никому не должны платить, не могут платить налоги, то я хотел бы предложить, чтобы при покупке автомобиля, который оставался в собственности в течение 30 или более дней, уплата налога ...
суперкат
... получить запись от вымышленного лица, получающего налоговую скидку на неоплаченное транспортное средство, чтобы на момент продажи налоговое обязательство составляло либо ноль, либо тридцать дней, независимо от того, как долго автомобиль не был приобретен.
суперкат
4

Я думаю, что в такой ситуации, когда вы хотите, чтобы бизнес-правила существовали вне целевых объектов, тестирование на тип более чем приемлемо.

Конечно, во многих ситуациях вы должны использовать паттерн Стратегии и позволить объектам самим обрабатывать вещи, но это не всегда желательно.

В реальном мире, клерк будет смотреть на тип транспортного средства и искать свои налоговые данные на основе этого. Почему ваше программное обеспечение не должно следовать тому же правилу busienss?

Эрик Фанкенбуш
источник
Хорошо, скажем, вы убедили меня, и с помощью метода GetVehiclesThatHaveToPay мы проверяем тип транспортного средства. Кто должен нести ответственность за LastPaidTime? Является ли LastPaidTime проблемой транспортного средства или государственной администрацией? Потому что, если это Автомобиль, разве эта бизнес-логика не должна находиться в Транспортном средстве? Что вы можете сказать мне по поводу вопроса 3.с?
@DavidRobertJones - В идеале я бы посмотрел последний платеж в журнале истории платежей, основываясь на лицензии на транспортное средство. Налоговый платеж - это налоговое беспокойство, а не транспортное средство.
Эрик Фанкенбуш
5
@DavidRobertJones Я думаю, вы путаете себя с собственной метафорой. То, что у вас есть, не является действительным транспортным средством, которое должно делать все то, что делает транспортное средство. Вместо этого вы для простоты назвали TaxableVehicle (или что-то подобное) транспортным средством. Весь ваш домен - это налоговые платежи. Не беспокойтесь о том, что делают настоящие автомобили. И, если необходимо, отбросьте метафору и предложите более подходящую.
3

Чего я не хочу, так это (я думаю):

2.b) Анемичная область, что означает бизнес-логику внутри бизнес-объектов.

У вас есть это задом наперед. Анемичная область - это область, логика которой находится за пределами бизнес-объектов. Есть много причин, почему использование дополнительных классов для реализации логики - плохая идея; и только пара, почему вы должны это сделать.

Отсюда и причина анемии: в классе ничего нет ..

Что бы я сделал:

  1. Измените свой абстрактный класс Vehicle на интерфейс.
  2. Добавьте интерфейс ITaxPayment, который должны реализовать транспортные средства. Пусть ваш HasToPay зависнет отсюда.
  3. Удалите классы MotorbikeTaxPayment, CarTaxPayment, TaxPayment. Это ненужные осложнения / беспорядок.
  4. Каждый тип транспортного средства (автомобиль, мотоцикл, дом на колесах) должен реализовываться на минимальном транспортном средстве и, если они облагаются налогом, должен также реализовать ITaxPayment. Таким образом, вы можете провести различие между теми транспортными средствами, которые не облагаются налогом, и теми, которые ... Предполагая, что такая ситуация даже существует.
  5. Каждый тип транспортного средства должен реализовывать в границах самого класса методы, определенные в ваших интерфейсах.
  6. Также добавьте дату последнего платежа в ваш интерфейс ITaxPayment.
Не я
источник
Вы правы. Первоначально вопрос не был сформулирован таким образом, я удалил часть, и значение изменилось. Это сейчас исправлено. Благодарность!
2

Почему транспортное средство должно знать, должно ли оно платить? Не является ли проблема общественной администрации?

Нет. Как я понимаю, все ваше приложение о налогах. Таким образом, забота о том, следует ли облагать налогом транспортное средство, больше не заботит PublicAdministration, чем что-либо еще во всей программе. Там нет причин, чтобы положить его в другом месте, кроме здесь.

public class Car : Vehicle
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 30;
    }
}

public class Motorbike : Vehicle
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 60;
    }
}

Эти два фрагмента кода отличаются только константой. Вместо переопределения всей функции HasToPay () переопределяемint DaysBetweenPayments() функцию. Назовите это вместо использования константы. Если это все, чем они на самом деле отличаются, я бы бросил занятия.

Я также предложил бы, чтобы мотоцикл / автомобиль был свойством категории транспортного средства, а не подклассами. Это дает вам больше гибкости. Например, это позволяет легко изменить категорию мотоцикла или автомобиля. Конечно, велосипеды вряд ли превратятся в автомобили в реальной жизни. Но вы знаете, что автомобиль будет введен неправильно, и его нужно будет заменить позже.

Конечно, мы можем передать значение конструктору TaxPayment, но это нарушит инкапсуляцию транспортного средства, или обязанности, или как это называют евангелисты ООП, не так ли?

На самом деле, нет. Объект, который намеренно передает свои данные другому объекту в качестве параметра, не является большой проблемой инкапсуляции. Реальное беспокойство вызывают другие объекты, достигающие внутри этого объекта для извлечения данных или манипулирования данными внутри объекта.

Уинстон Эверт
источник
1

Вы можете быть на правильном пути. Проблема, как вы заметили, в том, что внутри TaxPayment нет члена LastPaidTime . Это именно то, где он принадлежит, хотя это вообще не нужно публично раскрывать:

public interface ITaxPayment
{
    // Your base TaxPayment class will know the 
    // next due date. But you can easily create
    // one which uses (for example) fixed dates
    bool IsPaymentDue();
}

public interface IVehicle
{
    string Plate { get; }
    Person Owner { get; }
    ITaxPayment LastTaxPayment { get; }
} 

Каждый раз, когда вы получаете платеж, вы создаете новый экземпляр в ITaxPaymentсоответствии с текущими политиками, который может иметь совершенно другие правила, чем предыдущий.

Гру
источник
Дело в том, что срок оплаты зависит от того, когда владелец последний раз заплатил (разные транспортные средства, разные сроки). Как ITaxPayment узнает, когда в последний раз автомобиль (или владелец) платил для расчета?
1

Я согласен с ответом @sebastiangeiger о том, что проблема на самом деле заключается в владельцах транспортных средств, а не в транспортных средствах. Я собираюсь предложить использовать его ответ, но с некоторыми изменениями. Я бы сделал Ownershipкласс универсальным классом:

public class Vehicle {}

public abstract class Ownership<T>
{
    public T Item { get; set; }

    public DateTime LastPaidTime { get; set; }

    public abstract TimeSpan PaymentInterval { get; }

    public virtual bool HasToPay
    {
        get { return (DateTime.Today - this.LastPaidTime) >= this.PaymentInterval; }
    }
}

И используйте наследование, чтобы указать интервал оплаты для каждого типа транспортного средства. Я также предлагаю использовать строго типизированный TimeSpanкласс вместо простых целых чисел:

public class CarOwnership : Ownership<Vehicle>
{
    public override TimeSpan PaymentInterval
    {
        get { return new TimeSpan(30, 0, 0, 0); }
    }
}

public class MotorbikeOwnership : Ownership<Vehicle>
{
    public override TimeSpan PaymentInterval
    {
        get { return new TimeSpan(60, 0, 0, 0); }
    }
}

Теперь ваш реальный код должен иметь дело только с одним классом Ownership<Vehicle>.

public class PublicAdministration
{
    List<Ownership<Vehicle>> VehicleOwnerships { get; set; }

    public IEnumerable<Vehicle> GetVehiclesThatHaveToPay()
    {
        return VehicleOwnerships.Where(x => x.HasToPay).Select(x => x.Item);
    }
}
user74130
источник
0

Я ненавижу нарушать это для вас, но у вас есть анемичный домен , то есть бизнес-логика вне объектов. Если это то, к чему вы идете, это нормально, но вы должны прочитать о причинах, чтобы избежать этого.

Старайтесь не моделировать проблемную область, а моделируйте решение. Вот почему вы получаете параллельные иерархии наследования и более сложную, чем вам нужно.

Что-то вроде этого - все, что вам нужно для этого примера:

public class Vehicle
{
    public Boolean isMotorBike()
    public string PlateNumber { get; set; }
    public String Owner { get; set; }
    public DateTime LastPaidTime { get; set; }
}

public class TaxPaymentCalculator
{
    public List<Vehicle> GetVehiclesThatHaveToPay(List<Vehicle> all)
}

Если вы хотите следовать SOLID, это здорово, но, как сказал сам дядя Боб, это всего лишь инженерные принципы . Они не предназначены для строгого применения, но являются руководящими принципами, которые следует учитывать.

Гарретт Холл
источник
4
Я думаю, что ваше решение не особенно масштабируемо. Вам нужно будет добавить новое логическое значение для каждого типа транспортного средства, которое вы добавляете. На мой взгляд, это плохой выбор.
Эрик Фанкенбуш
@Garrett Hall, мне понравилось, что ты удалил класс Person из моего "домена". Во-первых, у меня не было бы этого класса, так что извините за это. Но isMotorBike не имеет смысла. Какой будет реализация?
1
@DavidRobertJones: Я согласен с Mystere, добавление isMotorBikeметода не является хорошим выбором ООП дизайна. Это похоже на замену полиморфизма кодом типа (хотя и логическим).
Groo
@MystereMan, можно легко бросить bools и использоватьenum Vehicles
radarbob
+1. Старайтесь не моделировать проблемную область, а моделируйте решение . Pithy . Памятно сказано. И принципы комментируют. Я вижу слишком много попыток строгой буквальной интерпретации ; ерунда.
радар Боб
0

Я думаю, что вы правы, что период оплаты не является собственностью реального автомобиля / мотоцикла. Но это атрибут класса автомобиля.

Несмотря на то, что вы можете пойти по пути наличия / выбора для объекта типа, это не очень масштабируемо. Если позже вы добавите грузовик и сегвей, а также толкнете велосипед, автобус и самолет (это может показаться маловероятным, но вы никогда не узнаете об этом с общественными службами), тогда состояние станет больше. И если вы хотите изменить правила для грузовика, вы должны найти его в этом списке.

Так почему бы не сделать это, в буквальном смысле, атрибутом и использовать отражение, чтобы вытащить его? Что-то вроде этого:

[DaysBetweenPayments(30)]
public class Car : Vehicle
{
    // actual properties of the physical vehicle
}

[AttributeUsage(AttributeTargets.Class, Inherited = false)]
public class DaysBetweenPaymentsAttribute : Attribute, IPaymentRequiredCalculator
{
    private int _days;

    public DaysBetweenPaymentAttribute(int days)
    {
        _days = days;
    }

    public bool HasToPay(Vehicle vehicle)
    {
        return vehicle.LastPaid.AddDays(_days) <= DateTime.Now;
    }
}

Вы также можете использовать статическую переменную, если это более удобно.

Недостатки

  • что это будет немного медленнее, и я предполагаю, что вам придется обрабатывать много машин. Если это проблема, то я мог бы взять более прагматичный взгляд и придерживаться абстрактного свойства или интерфейсного подхода. (Хотя я бы тоже рассмотрел кеширование по типу.)

  • При запуске вам нужно будет проверить, что у каждого подкласса Vehicle есть атрибут IPaymentRequiredCalculator (или разрешить значение по умолчанию), потому что вы не хотите, чтобы он обрабатывал 1000 автомобилей, а только аварийно, потому что у мотоцикла нет PaymentPeriod.

Положительным моментом является то, что если позже вы встретите календарный месяц или годовой платеж, вы можете просто написать новый класс атрибутов. Это само определение OCP.

прецизионный самописец
источник
Основная проблема заключается в том, что если вы хотите изменить количество дней между платежами за транспортное средство, вам нужно будет перестроить и повторно развернуть его, и если частота платежей изменяется в зависимости от свойства объекта (например, размера двигателя), это будет трудно получить элегантное решение для этого.
Эд Джеймс
@EdWoodcock: я думаю, что первая проблема верна для большинства решений, и это действительно не беспокоит меня. Если они захотят этот уровень гибкости позже, я бы дал им приложение БД. По второму пункту я полностью не согласен. В этот момент вы просто добавляете Vehicle как необязательный аргумент в HasToPay () и игнорируете его там, где он вам не нужен.
фунтовые
Я предполагаю, что это зависит от долгосрочных планов, которые у вас есть для приложения, а также от конкретных требований клиента, относительно того, стоит ли подключать БД, если ее еще нет (я склонен полагать, что почти все программное обеспечение является БД -вождение, которое я знаю не всегда так). Однако, по второму пункту, важный классификатор элегантен ; Я бы не сказал, что добавление дополнительного параметра к методу атрибута, который затем берет экземпляр класса, к которому прикреплен атрибут, является особенно элегантным решением. Но, опять же, это зависит от требований вашего клиента.
Эд Джеймс
@EdWoodcock: На самом деле, я просто думал об этом, и аргумент lastPaid уже должен быть собственностью Vehicle. Имея в виду ваш комментарий, возможно, стоит заменить этот аргумент сейчас, а не позже - он будет редактироваться.
фунтовые