Лучший шаблон проектирования для рефакторинга класса, который выполняет вычисления на основе многих параметров

Question

Я рефакторинг набора классов, как показано ниже, который делает некоторые расчеты цен.Расчет производится по многим параметрам.Код:

public interface IParcel {
    int SourceCode { get; set; }
    int DestinationCode { get; set; }
    int weight{get;set;}
    decimal CalculatePrice();
}

public abstract class GeneralParcel : IParcel {
    //implementation of inteface properties
    //these properties set with in SourceCode & DestinationCode 
    //and are used in CalculatePrice() inside classes that inherit from GeneralParcel 
    protected SourceProvinceCode{get; protected set;}
    protected DestinationProvinceCode{get;protected set;}

    //private variables we need for calculations
    private static ReadOnlyDictionary<int, List<int>> _States_neighboureness;
    private static ReadOnlyCollection<City> _Citieslist;
    private static ReadOnlyCollection<Province> _Provinceslist;

    protected ReadOnlyCollection<City> Citieslist {get { return _Citieslist; }}

    protected ReadOnlyCollection<Province> ProvincesList {get { return _Provinceslist; }}

    protected ReadOnlyDictionary<int, List<int>> StatesNeighboureness {get {return _States_neighboureness; }}

    //constructor code that initializes the static variables

    //implementation is in concrete classes
    public abstract decimal CalculatePrice();
}

public ExpressParcel : GeneralParcel {
    public decimal CalculatePrice() {
        //use of those three static variables in calculations
        // plus other properties & parameters
        // for calculating prices 
    }
}


public SpecialParcel : GeneralParcel {
    public decimal CalculatePrice() {
        //use of those three static variables in calculations
        // plus other properties & parameters
        // for calculating prices 
    }
}

В данный момент код эффективно использует «Шаблон стратегии» .

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

Необходим ли другой интерфейс, как показано ниже (и затем обернуть эти статические свойства внутри него?, дажесделать статическим этот класс, потому что это в основном только некоторые вычисления), тогда как подключить его к IParcel?При этом, как реализовать CalculatePrice() в SpecialParcel & ExpressParcel классов?

public interface IPriceCalculator {
    decimal CalculatePrice();
}

РЕДАКТИРОВАТЬ: вышеЭто была только общая картина всей системы, есть и другое соображение, что в комментариях мы обсуждаем их, и я пишу их здесь снова для очистки вещей.

есть BulkDiscount для всех ParcelTypes.массовая отправка происходит, когда клиент отправляет более 10 посылок (или любой порог), также существует специальная скидка, когда один клиент отправляет более 10 посылок в уникальное место назначения (есть только один получатель).теперь этот тип скидок управляется в каждом типе посылки CalculatePrice().даже есть скидки на блайнды для посылок весом до 7 кг.

также сейчас есть 3 parceltype, я показываю только 2 из них здесь.но нам нужно добавить другой тип в будущем (поддержка TNT и DHL).у каждого типа есть много услуг, которые клиент может выбрать и оплатить.например, sms service или email service и т. д.

Answer 1

Лично, в то время как другие могут сказать, что Посылка не должна знать, как рассчитать стоимость доставки, я не согласен.Ваш дизайн уже определяет, что есть три разных вида посылок с тремя разными вычислениями, поэтому, на мой взгляд (наивно?), Вполне уместно, чтобы у объекта был метод, например CalculatePrice().

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

Однако я бы рассмотрел создание статических коллекций City и Provinceоткрытые статические свойства City и Province соответственно.Это просто аккуратнее, и именно там я и ожидал бы найти их, если бы поддерживал код.StatesNeighbourliness вероятно, должен отправиться в Провинцию, или это может даже оправдать свой собственный класс.

Answer 2

Способ расчета цены для данной посылки - это ответственность, которая не должна принадлежать объекту данных.

Учитывая то, что вы мне сказали, вот как я бы это реализовал, чтобы попытаться учесть будущие соображения:

public interface IParcel {
    int SourceCode { get; set; }
    int DesinationCode { get; set; }
    int Weight { get; set; }
}

public class PricingCondition {
    //some conditions that you care about for calculations, maybe the amount of bulk or the date of the sale
    //might possibly be just an enum depending on complexity
}

public static class PricingConditions {
    public static readonly PricingCondition FourthOfJulyPricingCondition = new PricingCondition();
    public static readonly PricingCondition BulkOrderPricingCondition = new PricingCondition();
    //these could alternatively come from a database depending on your requirements
}

public interface IParcelBasePriceLookupService {
    decimal GetBasePrice(IParcel parcel);
    //probably some sort of caching
}

public class ParcelPriceCalculator {
    IParcelBasePriceLookupService _basePriceLookupService;

    decimal CalculatePrice(IParcel parcel, IEnumerable<PricingCondition> pricingConditions = new List<PricingCondition>()) {
        //do some stuff
    }
    decimal CalculatePrice(IEnumerable<IParcel> parcels, IEnumerable<PricingCondition> pricingConditions = new List<PricingCondition>()) {
        //do some stuff, probably a loop calling the first method
    }
}

Answer 3

Совет, который я бы дал, будет зависеть от того, как вы будете генерировать и использовать полиморфы Parcel.Я имею в виду, что мы не можем видеть, какие критерии используются, чтобы определить, является ли что-то «Экспресс» или «Специальным» и связаны ли эти критерии со свойствами самой посылки или каким-либо внешним фактором.

Сказав это, я думаю, что ваша интуиция хороша в том, чтобы отделить расчет цены от самого объекта Parcel.Как указал kmkemp, посылка не должна выяснять, как рассчитать цену посылки в зависимости от типа посылки.Посылка - это объект типа передачи данных / типа POCO, по крайней мере, как указано в вашем сообщении веса, источника и т. Д.

Однако я не совсем понимаю, почему вы используете эти интерфейсы.Не поймите меня неправильно - интерфейс хорош для развязки и тестируемости, но почему есть интерфейс пакета в дополнение к абстрактному базовому классу с абстрактным методом.Лично, используя только ту информацию, которая у меня есть, я бы сделал это:

public class Parcel
{
    int SourceCode { get; set; }
    int DestinationCode { get; set; }
    int weight { get; set; }
}

public abstract class GeneralCalculator
{
    //Statics go here, or you can inject them as instance variables
    //and they make sense here, since this is presumably data for price calculation
    protected static ReadOnlyDictionary<int, List<int>> _States_neighboureness;
    protected static ReadOnlyCollection<City> _Citieslist;
    protected static ReadOnlyCollection<Province> _Provinceslist;
    //.... etc

    public abstract Decimal CalculatePrice(Parcel parcel);
}

public class ExpressCalculator : GeneralCalculator
{
    public override decimal CalculatePrice(Parcel parcel)
    {
        return 0.0M;
    }
}

public class SpecialCalculator : GeneralCalculator
{
    public override decimal CalculatePrice(Parcel parcel)
    {
        return 0.0M;
    }
}

Но, опять же, я не знаю, как на самом деле обрабатываются посылки.Вам может потребоваться какое-то изменение этой концепции в зависимости от того, как вы генерируете, а затем обрабатываете посылки.Например, если тип участка зависит от значений свойств участка, вы можете определить фабричный метод или класс, который принимает участок и возвращает соответствующий экземпляр калькулятора.

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

Answer 4

IPriceCalculator будет лучшей практикой для принципа единой ответственности.

Но измените сигнатуру метода на decimal CalculatePrice(IParcel parcel); Метод вызывает метод IParcel CalculatePrice () для получения базовой цены для каждой посылки.

Answer 5

Как вы говорите, эти статические свойства на самом деле не являются частью класса GeneralParcel.Переместите их в статический класс "ListsOfThings".

Затем вы можете использовать код, который ссылается на ListsOfThings.ProvincesList и т. Д.