Это третья часть серии, в которой я тестирую классическое кодовое ката и использую только правила коннасценции, чтобы сказать мне, что нужно реорганизовать. В прошлый раз я продолжал работать над ката на кассе и исправил некоторое Connascence of Value, введя новый класс для представления доменной концепции Money. Сегодня я хочу немного продвинуть этот же код, чтобы исследовать, что происходит, когда я продолжаю просто реагировать на смущение, которое вижу.
Вот код на данный момент:
public class CheckoutTests {
@Test
public void basicPrices() {
Money priceOfA = randomPrice();
Checkout checkout = new Checkout();
Money priceOfB = randomPrice();
checkout.scan("A", priceOfA).scan("B", priceOfB);
assertEquals(priceOfA.add(priceOfB), checkout.currentBalance());
}
private Money randomPrice() {
int pence = new Random().nextInt(1000);
return Money.fromPence(pence);
}
}
public class Checkout {
private Money balance = Money.ZERO;
public Checkout scan(String sku, Money price) {
balance = balance.add(price);
return this;
}
public Money currentBalance() {
return balance;
}
}
Пришло время добавлять специальные предложения. Я добавляю тест, который говорит, что мы получаем скидку в 20 пунктов, если наша корзина содержит два элемента «А»:
@Test
public void discountForTwoAs() {
Money priceOfA = randomPrice();
Checkout checkout = new Checkout();
checkout.scan("A", priceOfA).scan("A", priceOfA);
assertEquals(priceOfA.add(priceOfA).subtract(Money.fromPence(20)), checkout.currentBalance());
}
Я делаю это наивно, проверяя текущий отсканированный элемент по сравнению с его предшественником:
public class Checkout {
private String previousSku;
//...
public Checkout scan(String sku, Money price) {
balance = balance.add(price);
if (sku.equals(previousSku))
balance = balance.subtract(Money.fromPence(20));
previousSku = sku;
return this;
}
}
Это похоже на огромный взлом и достаточно много кода для написания одного провального теста. Но, с другой стороны, тест проходит успешно, поэтому я готов подготовиться к рефакторингу. Как и раньше, я ввел некоторую Connascence of Value, потому что и Checkout, и тест знают об этом -20 для суммы скидки. Это уровень 8 (из 9) коннасценции, поэтому мне лучше удалить его, прежде чем делать что-либо еще Как и раньше, я могу быстро это исправить, передав из проверки в Checkout:
@Test
public void discountForTwoAs() {
Money priceOfA = randomPrice();
Money discount = Money.fromPence(20);
Checkout checkout = new Checkout();
checkout.scan("A", priceOfA, discount)
.scan("A", priceOfA, discount);
Money expectedTotal = priceOfA.add(priceOfA).subtract(discount);
assertEquals(expectedTotal, checkout.currentBalance());
}
Вкратце, я должен упомянуть, что этот код мне ужасен: все мои инстинкты и опыт кричат о том, что эти значения Money должны были быть переданы в конструктор Checkout, а не через метод scan (), как описано выше. Я подозреваю, что это потому, что я занимаюсь моделированием предметной области в своей голове, пока я пишу код. Для этих сообщений в блоге я сознательно «следую правилам»; Я понятия не имею, приведет ли это к коду, который мне нравится, и я думаю, что это половина удовольствия. Во всяком случае, allons-y …
Этот последний рефакторинг устранил Connascence of Value, что хорошо. Но теперь вместо этого у меня есть Connascence of Position. Это связано с тем, что тест и Checkout должны согласовать порядок, в котором эти два значения Money передаются в метод scan (). Проблема в том, что компилятор не может помочь, поэтому для вызывающей стороны было бы очень просто передать их в неправильном порядке. Хотя это не большая проблема для теста, это может быть финансово катастрофическим, если любой другой клиент Checkout передаст значения в неправильном порядке.
(Connascence of Position — уровень 5 — более серьезный, чем Connascence of Value, но менее серьезный, чем Connascence of Value. Одна из вещей, которые я нахожу наиболее привлекательными в отношении connascence, — это строгая иерархия: я всегда знаю, какую связь следует решать дальше.)
Я могу придумать несколько способов справиться с этим Connascence of Position:
- Переместите любой параметр Money в конструктор Checkout. Это исправит Connascence of Position, потому что больше не будет единственного метода с двумя параметрами Money.
- Оберните SKU и его цену в одном объекте. Это привлекательно, потому что он имеет ранний запах PriceList. Это может как-то успокоить модельера домена в моей голове.
- Передайте скидку с помощью нового метода на Checkout. Это устранит Connascence of Position, но введет «сеттер» — метод, который настраивает Checkout после того, как он был построен. Это было бы ужасной идеей, потому что это заставляет пользователя объекта помнить порядок, в котором эти методы должны быть вызваны. Это будет Connascence of Execution Order — уровень 6 по шкале 9 ..
- Оберните скидку в новый объект SpecialOffer. Заворачивание одного объекта в другой, просто чтобы избежать Connascence of Position, кажется излишним.
- Оберните два значения Money в словаре. Одним из основных способов устранения CoP является использование именованных параметров. Я сегодня работаю на Java, так что это не вариант. Я мог бы подделать его, создав Map <String, Money>, но эти два значения не чувствуются, поскольку они принадлежат одной и той же структуре данных.
Теперь я вижу эти варианты, я понимаю, что все это преждевременно. Я мог бы взять код в любом количестве различных направлений на данный момент, и у меня нет достаточно информации, чтобы решить, что будет наиболее подходящим. Поэтому мне нужен следующий тест, чтобы принять это решение в контексте. Я решил переработать тест, добавив другой элемент между двумя As:
@Test
public void discountForTwoAs() {
Money priceOfA = randomPrice();
Money priceOfB = randomPrice();
Money discount = Money.fromPence(20);
Checkout checkout = new Checkout();
checkout.scan("A", priceOfA, discount)
.scan("B", priceOfB, discount);
.scan("A", priceOfA, discount);
Money expectedTotal = priceOfA.add(priceOfA)
.add(priceOfB)
.subtract(discount);
assertEquals(expectedTotal, checkout.currentBalance());
}
Как всегда, я делаю это, не задумываясь о эстетике дизайна:
public class Checkout {
//...
private int countOfAs = 0;
public Checkout scan(String sku, Money price, Money discount) {
balance = balance.add(price);
if (sku.equals("A"))
countOfAs++;
if (countOfAs == 2)
balance = balance.subtract(discount);
return this;
}
}
Кажется, мне всегда удается представить Connascence of Value! На этот раз и тест, и Checkout знают, на какой товар действует скидка, и сколько товаров вызывают эту скидку. Как и раньше, я исправляю это, передавая дублированные знания через параметр:
@Test
public void discountForTwoAs() {
Money priceOfA = randomPrice();
Money priceOfB = randomPrice();
Checkout checkout = new Checkout();
Money discount = Money.fromPence(20);
checkout.scan("A", priceOfA, "A", discount, 2)
.scan("B", priceOfB, "A", discount, 2)
.scan("A", priceOfA, "A", discount, 2);
Money expectedTotal = priceOfA.add(priceOfA)
.add(priceOfB)
.subtract(discount);
assertEquals(expectedTotal, checkout.currentBalance());
}
На данный момент я могу подвести итоги. Я сделал Connascence of Position хуже, потому что теперь вызывающий абонент также должен знать, какой из этих двух параметров SKU был отсканирован, а какой является индикатором специального предложения. Я также создал другое смущение; Подробнее об этом в следующем посте. На данный момент Connascence of Position по-прежнему является моей самой большой проблемой, и мои варианты ее исправления неожиданно стали намного яснее: три параметра, связанных со скидками, представляют аспекты одной концепции домена: MultiBuyDiscount. Я могу победить Connascence of Position, обернув эти значения в новый класс:
@Test
public void discountForTwoAs() {
Money priceOfA = randomPrice();
Money priceOfB = randomPrice();
Checkout checkout = new Checkout();
MultibuyDiscount discount = new MultibuyDiscount("A", Money.fromPence(20), 2);
checkout.scan("A", priceOfA, discount)
.scan("B", priceOfB, discount)
.scan("A", priceOfA, discount);
Money expectedTotal = priceOfA.add(priceOfA)
.add(priceOfB)
.subtract(Money.fromPence(20));
assertEquals(expectedTotal, checkout.currentBalance());
}
public class Checkout {
//...
public Checkout scan(String sku, Money price, MultibuyDiscount discount) {
balance = balance.add(price);
if (sku.equals(discount.discountSku))
discountItems++;
if (discountItems == discount.discountTrigger)
balance = balance.subtract(discount.discount);
return this;
}
}
Как и прежде, исправляя некоторое смущение, я обнаружил новый предметный объект и дал ему имя и реальное существование. Но пока что этот новый объект является просто контейнером данных: у него есть три открытых поля, и вся обработка, связанная с ними, все еще находится в Checkout. Эта ситуация нарушает сильное предложение Деметры (и это выглядит довольно вонючим). Мне нужно исправить это в ближайшее время.
Однако этот пост вырос гораздо дольше, чем мне нравится, поэтому я на этом остановлюсь. В следующий раз я рассмотрю этот код для большего удобства …