рефакторинг длинных методов с беглыми интерфейсами

Question

Мне бы хотелось узнать ваше мнение об использовании шаблона интерфейса Fluent для рефакторинга длинного метода.

http://en.wikipedia.org/wiki/Fluent_interface

Шаблон Fluent не включен в книги по рефакторингу.

например, скажем, у вас есть этот длинный метод (с длинным именем, потому что он много делает)to:

class TravelClub {

   TravelClub buy(long amount) {
    //buy stuff
    return this;
   }

   TravelClub accumulatePoints(long cardNumber) {
    //accumulate stuff
    return this;
   }

   Receipt generateReceipt() {
    return new Receipt(...);
   }


}

и называется:

Receipt myReceipt = myTravelClub.buy(543L).accumulatePoints(12345678L).generateReceipt();

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

что ты думаешь?

Answer 1

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

Если вы хотите свободный интерфейс, я быввести дополнительный класс, который аккуратно направляет код клиента в правильные действия (при условии, что все покупки происходят с картой и накапливают баллы одинаково):

class TravelClub {

   OngoingPurchase buyAmount(long amount) {
      return new OngoingPurchase(amount);
   }

   private Receipt buyAndAddPointsAndGetReceipt(long amount, long cardNumber){
      // make stuff happen
   }

   public class OngoingPurchase {
      private final long amount;
      private OngoingPurchase(long amount){
         this.amount = amount;
      }
      public Receipt withCard(long cardNumber){
         return buyAndAddPointsAndGetReceipt(long amount, cardNumber);
      }
   }

}

// Usage:
Receipt receipt = travelClub.buyAmount(543).withCard(1234567890L);

Таким образом, если вы забыли позвонить withCard, ничего не происходит.Проще заметить пропущенную транзакцию, чем неправильную, и вы не можете получить квитанцию, не выполнив полную транзакцию.

Редактировать: Кстати, смешно думать, что мыпроделайте всю эту работу, чтобы сделать методы с множеством параметров читабельными, когда, например, именованные параметры полностью устранят проблему:

Receipt r = travelClub.makePurchase(forAmount: 123, withCardNumber: 1234567890L);

Answer 2

Мой встречный вопрос: каково ожидаемое поведение, если кто-то вместо этого позвонит:

myTravelClub.accumulatePoints(10000000L);

без вызова buy?Или генерация квитанции перед покупкой?Я думаю, что свободные интерфейсы все еще должны придерживаться других соглашений ОО.Если вам действительно нужен плавный интерфейс, то метод buy() должен будет вернуть другой объект, а не сам TravelClub, а «объект покупки» с методами accumulatePoints() и generateReceipt().

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

Answer 3

Длинный метод не совпадает с методом с длинным именем . В вашем случае я бы изменил только имя метода:

public Receipt buy(long amount, long cardNumber) {
    buy(amount);
    accumulatePoints(cardNumber);
    return generateReceipt();
}

(и придумайте более описательное имя для частного buy метода), потому что все три вещи («покупка», накопление баллов и получение квитанции) всегда происходят вместе, поэтому с точки зрения вызывающего кода они могут разовая операция С точки зрения реализации, выполнение одной операции также проще. ПОЦЕЛУЙ: -)

Answer 4

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

В случае, представленном в вашем примере кода, у меня возникнет соблазн упростить имясам метод к чему-то более обобщенному:

Receipt Transaction(long amount, long cardNumber) 
{
    buy(amount);
    accumulatePoints(cardNumber);
    return generateReceipt();
}

Так что насчет этой логической проблемы, которую я упомянул?Это само по себе сводится к тому, очень ли ваш метод очень фиксирован в том, что он делает.Если можно завершить транзакцию только с использованием последовательности Buy-> Points-> Receipt, то работает более простое имя, но также более описательное имя, и разумным вариантом может быть свободный интерфейс.

А как насчет тех случаев, когда у клиента нет карты вознаграждения или он не хочет получать квитанцию?Как насчет тех ситуаций, когда несколько предметов могут быть куплены за одну транзакцию - если, конечно, метод покупки может представлять предмет покупки, а не просто сумму, рассчитанную в другом месте?Как только вы начинаете вводить вопросы / варианты в последовательность, дизайн становится немного менее очевидным, а наименование становится намного сложнее.Вы, конечно, не захотите использовать сумасшедшее длинное имя, например:

BuyAndAddPointsIfTheCustomerHasACardAndReturnAReceiptIfTheCustomerAsksForIt(...)

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

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

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

Answer 5

Пока вы используете правильные проверки, интерфейсы Fluent намного проще понять, например, это может выглядеть следующим образом:

класс TravelClub {

   TravelClub buy(long amount) {
    buy(amount);
    return this;
   }

   TravelClub accumulatePoints(long cardNumber) {
    if (!bought)
    {
        throw new BusinessException("cannot accumulate points if not bought");
    }
    accumulatePoints(cardNumber);
    return this;
   }

   Receipt generateReceipt() {
    if (!bought)
    {
       throw new BusinessException("cannot generate receipts not bought");
    }
    return new Receipt(...);
   }
}

Answer 6

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

Если единственный способ вызвать эти методы будет в той же последовательности, как в вашем первом блоке кодасохраните это как единственную функцию.Однако, если какое-либо подмножество манипуляций может быть выполнено на TravelClub до генерирования квитанции, тогда непременно используйте свободный интерфейс.Это один из лучших (если не лучший) способов преодолеть запах кода «комбинаторный взрыв».