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

Question

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

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

Свободный образец не включен в книги по рефакторингу.

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

class TravelClub {

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

   }

   void buy(int amount) {...}

   void accumlatePoints(int cardNumber) {...}

   void generateRecepit() {...}

}

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

Receipt myReceipt = myTravelClub.buyAndAddPointsAndGetReceipt(543L,12345678L);

Это может быть изменено на:

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() метод должен был бы вернуть другой объект, не сам 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();
}

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

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

BuyAndAddPointsIfTheCustomerHasACardAndReturnAReceiptIfTheCustomerAsksForIt(...)

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

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

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

Answer 5

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

class 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 как вы могли бы в приведенном вами примере интерфейса пользователя.

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