Как я могу улучшить этот блок кода? - PullRequest
2 голосов
/ 03 сентября 2011
public class OrderProcessor {

  public Double calculateTotalPriceWithDiscountCode(Order order,
        char discountCode) {

    Double itemTotal = order.getItemTotal();
    Double discountAmount = 0.0;

    switch (discountCode) {
    case 'A':
        discountAmount = 0.95 * itemTotal;
        break;
    case 'B':
        discountAmount = 0.15 * itemTotal;
        break;
    }
    return itemTotal - discountAmount;
}

Текущая реализация для процессора заказов закрыта для расширения и открыта для модификации для добавления новых кодов скидок, как я могу улучшить дизайн, чтобы избавиться от этого ограничения

Ответы [ 6 ]

3 голосов
/ 03 сентября 2011

В общем, наличие switch является довольно хорошей раздачей относительно того, каким должен быть класс.Итак, первый удар по нему будет выглядеть примерно так:

public interface Discounter {
  public double applyDiscount(double itemTotal);
}

public class OrderProcessor {
  private Map<Char, Discounter> discounts = new HashMap<Char, Discounter>();

  public void addDiscounter(Char discountCode, Discounter discounter) {
    discounts.put(discountCode, discounter);
  }

  public Double calculateTotalPriceWithDiscountCode(Order order, char discountCode) {
    double itemTotal = order.getItemTotal();
    double discountAmount = 0.0;

    if (discounts.hasKey(discountCode))
      discountAmount = discounter.applyDiscount(itemTotal);

    return itemTotal - discountAmount;
  }
}

Затем это будет расширено через что-то вроде этого:

processor.addDiscounter('A', new Discounter() {
  public double applyDiscount(double itemTotal) {
    return 0.95 * itemTotal;
  }
});

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

Примечание : Еслиэто то, что вы собираетесь делать в производстве. У меня есть два незапрошенных совета:

  1. Попробуйте использовать что-то вроде JBoss Drools вместо того, чтобы обрабатывать вашу бизнес-логикуэтого;он намного более мощный и гибкий.

  2. Пожалуйста, не используйте double для реальных финансовых расчетов.:)

2 голосов
/ 03 сентября 2011

Поскольку вы указали, что код скидки необходимо изменить, лучше всего отделить действительные коды скидки от кода. Сохраните коды скидок в xml или файле настроек и отложите загрузку значений в словарь / хэш-набор перед их использованием.

Например, ваш xml может выглядеть так:

<Discounts>
  <Discount code="A" value="0.05"/>
  <Discount code="B" value="0.10"/>
  <Discount code="C" value="0.15"/>
</Discounts>

В вашем calculateTotalPriceWithDiscountCode заполните словарь значениями, прочитанными из этого xml.

if(discountsDictionary == null)
{    
    discountsDictionary = LoadValuesFromXml(xmlFilePath);
}

Если ваши скидки xml могут измениться во время выполнения программы, тогда выполните операцию Load, когда вы хотите получить скидку (по сравнению с загрузкой один раз во фрагменте кода выше).

Затем получите доступ к коду (ключу), чтобы получить скидку (значение),

if(discountsDictionary.ContainsKey(discountCode))
{
    discount = discountsDictionary[discountCode];
    discountedItemPrice = itemPrice * ( 1 - discount);
}
else
{
     // Invalid discount code...
}
1 голос
/ 03 сентября 2011

Потяните логику для вычисления итогов на основе заказа в свой собственный класс / интерфейс:

class OrderProcessor {
    // Probably inject this or load it some other way than a static factory
    private Collection<TotalCalculator> calculators = TotalCalculatorFactory.getTotalCalculators();

    public Double calculateTotalPriceWithDiscountCode(Order order, char discountCode) {
        for (TotalCalculator calculator : calculators) {
            if (calculator.supports(discountCode)) {
                return calculator.calculateTotal(order);
            }
        }
        return order.getItemTotal();
    }
}

class TotalCalculator {
    private char discountCode;
    private double discountRatio;

    public TotalCalculator(char discountCode, double discountRatio) {
        this.discountCode = discountCode;
        this.discountRatio = discountRatio;
    }

    public boolean supports(char discountCode) {
        return this.discountCode == discountCode;
    }

    public Double calculateTotal(Order order) {
        return order.getItemTotal() - order.getItemTotal() * discountRatio;
    }
}

class TotalCalculatorFactory {
    public static Collection<TotalCalculator> getTotalCalculators() {
        return Arrays.asList(
                new TotalCalculator('A', 0.95),
                new TotalCalculator('B', 0.15)
        );
    }
}

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

Это решение также очень гибкое, позволяя вам создавать ваши TotalCalculators (или реализации в виде интерфейса), используя другие механизмы, чем их ручное кодирование взавод.Вы можете использовать IoC-контейнер для их создания и внедрения или использовать, например, ServiceLoader для их поиска и загрузки.

0 голосов
/ 03 сентября 2011

Вы можете использовать отдельный XML-файл для хранения кодов, а также их механизмов расчета.

Это снимет ограничение на невозможность добавить новый код скидки.

Файл XML: discount.xml

<discount-codes>
    <discount-code>
       <code>A</code>
       <val>0.15</val>
    </discount-code>
    <discount-code>
        <code>B</code>        
        <val>0.95</val>
    </discount-code>
</discount-codes>

Примечание. Код операции (что я собираюсь делать со значениями?) В настоящее время не реализован. Вы можете реализовать то же самое на своем конце.

Использовать класс синтаксического анализатора XML:

Класс: DiscountModel.java (Этот класс является моделью для хранения кодов скидок)

public class DiscountModel {
    char code;
    Double val;

    // Implement getters and setters
}

Класс: DiscountParser.java (это позволит проанализировать файл discount.xml и сохранить коды в списке)

public class DiscountParser {
    List<DiscountModel> discountsList;

    // Getters and Setters for discountsList

    // Parser Code
    public void parseDiscounts() {
         // Code here
    }

    // Add new discount
    public void addDiscount() {
        // Code 
    }

    // Remove discount
    public void removeDiscount () {
       // Code
    }
}

Класс: OrderProcessor.java (Это приведет к дисконтированному значению после расчета)

/**
 *  Call this class when calculations need to be done.
 */
public class OrderProcessor {
    // Declare instance of DocumentParser
    DocumentParser dc1;

    // Getter and setter for dc1

    public Double calculateTotalPriceWithDiscountCode(Order order, char discountCode) {
        // Find the corresponding discountcode and 
        // value from the list of values in the 
        // Class DocumentParser          

        // Use the corresponding values to calculate 
        // the discount and return the value
    }
}  

Всякий раз, когда требуется добавить новый код, вы можете вставить его в файл XML. То же самое применимо, если необходимо удалить код скидки.

Надеюсь, что выше поможет.

0 голосов
/ 03 сентября 2011

Моя идея - сохранять скидки отдельно.

public class OrderProcessor {

  public Double calculateTotalPriceWithDiscountCode(Order order,
    char discountCode) {

   Double itemTotal = order.getItemTotal();
 return itemTotal - (itemTotal* Discounts.GetDiscounts(discountCode));
  }
}

////////////////

class Discounts
{
 public static double GetDiscounts(char discountCode)
    {
       switch (discountCode) {
case 'A':
    return 0.95d;

case 'B':
    return 0.15d;

default:
      return 0.00d;
  }
 }
}
0 голосов
/ 03 сентября 2011

В этой функции calculateTotalPriceWithDiscountCode не рекомендуется передавать код скидки в качестве параметра. Ну, третий человек, который просматривает ваш код, не поймет, не поймет, что это означает, кроме вас, своего рода запах кода.

Одно из предложений заключается в том, что вам нужно создать другой класс с именем Discount, передать объект Discount в качестве параметра и получить внутреннее значение из его открытого метода.

т.е.

public class Discount {
//Use a hash map to store your Code associate with your discountAmount

    public Discount(char discountCode){
        this.discountCode = discountCode
    }

    public int getDiscountAmount(){
        ....
       ....
    }
}

Прямо сейчас вам нужно изменить только класс Discount, и вашему calculateTotalPriceWithDiscountCode не нужно будет заботиться.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...