Статьи

Коннасценция алгоритма

Март 28, 2018

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

Если вы  следили за этим , вы вспомните, что мой  последний рефакторинг заключался в создании нового доменного объекта для MultiBuyDiscount, тем самым облегчая неприятный случай Connascence of Position. Тесты пройдены, поэтому давайте посмотрим, что остается в этом коде:

  • Уровень 1, Connascence of Name , например, потому что различные классы знают имена методов, вызываемых друг на друга. Это доброкачественно и не исчезнет в ближайшее время.
  • Уровень 2, Connascence of Type , например, потому что тест знает, какие классы создавать. Точно так же это в значительной степени мягко в этом коде.
  • Уровень 4, Connascence of Algorithm . Здесь есть две проблемы: во-первых, знание параметров scan () сохраняется во время вызовов. Это означает, что вызывающая сторона (в данном случае тест) должна знать кое-что о том, как работает Checkout, чтобы заставить его вести себя предсказуемо. А во-вторых, сама Checkout знает, что MultiBuyDiscount имеет триггер, SKU и сумму скидки. Сам MultiBuyDiscount не имеет никакой конфиденциальности, и если дизайн этой концепции домена изменится, нам также придется изменить код в Checkout.

ша

Я планирую исправить эти два случая Connascence of Algorithm в порядке, описанном выше. Во-первых, связь между проверкой и проверкой.

Состояние экземпляра Checkout теперь зависит от значения параметров для различных вызовов функции scan (). Если я изменю специальные предложения при сканировании товаров в одной корзине, это приведет к противоречивому поведению. Например:

@Test
public void connascenceOfExecutionOrder() {
  Money priceOfA = randomPrice();
  Money firstTotal = new Checkout()
      .scan("A", priceOfA, new MultibuyDiscount("A", Money.fromPence(20), 1))
      .scan("A", priceOfA, new MultibuyDiscount("A", Money.fromPence(20), 2))
      .currentBalance();
  Money secondTotal = new Checkout()
      .scan("A", priceOfA, new MultibuyDiscount("A", Money.fromPence(20), 2))
      .scan("A", priceOfA, new MultibuyDiscount("A", Money.fromPence(20), 1))
      .currentBalance();
  assertEquals(firstTotal, secondTotal);
}

Этот тест не пройден, что, безусловно, является дефектом дизайна. К счастью, решение простое: я передаю специальное предложение в Checkout через его конструктор, а не через метод scan ():

public class Checkout {
  //...
  private MultibuyDiscount discount;
 
  public Checkout(MultibuyDiscount discount) {
    this.discount = discount;
  }
 
  public Checkout scan(String sku, Money price) {
    balance = balance.add(price);
    if (sku.equals(discount.discountSku))
      discountItems++;
    if (discountItems == discount.discountTrigger)
      balance = balance.subtract(discount.discount);
    return this;
  }
}

Это исправляет наихудший алгоритм Connascence of Algorithm, который связал тест с состоянием экземпляра Checkout. Далее я расскажу о другом экземпляре Connascence of Algorithm, между Checkout и MultiBuyDiscount. Это случай «Зависти к функциям», и я должен признать, что дважды подумал, прежде чем исправить это. Можно утверждать, что код достаточно нормален в том виде, в каком он есть, и что это связывание необходимо будет исправить только в том случае, если в будущем компания представит скидку другого типа. Тем не менее, код в его нынешнем виде эстетически неприятен, поэтому я решил продолжить.

Я перемещаю смелость алгоритма скидки из Оформления заказа:

public class Checkout {
  //...
  public Checkout scan(String sku, Money price) {
    balance = balance.add(price);
    balance = balance.subtract(discount.discountFor(sku));
    return this;
  }
}

и в MultiBuyDiscount:

public class MultibuyDiscount {
  private String watchedSku;
  private Money discount;
  private int trigger;
  private int itemsSeen = 0;
 
  public MultibuyDiscount(String sku, Money discount, int trigger) {
    this.watchedSku = sku;
    this.discount = discount;
    this.trigger = trigger;
  }
 
  public Money discountFor(String sku) {
    if (sku.equals(watchedSku))
      itemsSeen++;
    return (itemsSeen == trigger) ? discount : Money.ZERO;
  }
}

Итак, вот и все — Connascence of Algorithm все прошло. Ну, не совсем: делая это изменение, я только перемещал коннасценцию вокруг! Предположим, я создал две кассы, но у них есть общий MultibuyDiscount? Следующий тест не пройден:

@Test
public void independentCheckouts() {
  Money priceOfA = randomPrice();
  MultibuyDiscount discount = new MultibuyDiscount("A", Money.fromPence(20), 2);
  Checkout checkout1 = new Checkout(discount);
  Checkout checkout2 = new Checkout(discount);
  checkout1.scan("A", priceOfA);
  checkout2.scan("A", priceOfA);
  assertEquals(priceOfA, checkout1.currentBalance());
  assertEquals(priceOfA, checkout2.currentBalance());
}

Второй клиент получает скидку, несмотря на то, что покупает только одну букву «А»!

Объект скидки представляет состояние корзины покупателя, поскольку он запоминает, сколько элементов SKU триггера было отсканировано. Таким образом, клиент (опять же, только тест в нашем случае, но это может быть что угодно) должен помнить, что каждый раз при создании новой проверки он должен создавать новые объекты скидок. Это Коннасценция Алгоритма. Вопрос: как это исправить?

  1. Я мог бы убедиться, что каждый экземпляр Checkout создает свое собственное правило скидки, чтобы они никогда не передавались. Я должен был быть осторожным, чтобы не вводить Connascence of Value, поэтому он все равно должен быть тестом, который бы знал подробности правила.
  2. Я мог бы найти способ разделить MultiBuyDiscount на две части: одну, чтобы узнать бизнес-правила, а другую, чтобы отслеживать, сколько SKU триггеров было отсканировано в корзине текущего клиента. Checkout может содержать экземпляр последнего, в то время как тест содержит первый.

Когда я смотрю на них подробно, я вижу, что это по сути одно и то же решение. И поскольку я работаю на Java, мои возможности довольно тесно связаны. Объект правила, переданный из теста в конструктор Checkout, должен быть фабрикой, которую Checkout может использовать для создания своего личного дискаунтера. Я создаю новый фабричный класс:

public class MultibuyDiscountFactory {
  private String watchedSku;
  private Money discount;
  private int trigger;
 
  public MultibuyDiscountFactory(String sku, Money discount, int trigger) {
    this.watchedSku = sku;
    this.discount = discount;
    this.trigger = trigger;
  }
 
  public MultibuyDiscount create() {
    return new MultibuyDiscount(watchedSku, discount, trigger);
  }
}

а затем использовать его в Checkout:

public class Checkout {
  //...
  public Checkout(MultibuyDiscountFactory discount) {
    this.discount = discount.create();
  }
}

Ну, это было намного больше работы, чем я ожидал, когда я впервые заметил Connascence of Algorithm (иначе Feature Envy) еще в  предыдущем посте ! В следующий раз я вернусь к решению использовать фабрику здесь …

38
Share: