refactoring long methods with fluent interfaces

Question

I'd like to know your opinion about using the fluent interface pattern to refactor a long method.

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

The fluent pattern is not included in the refactoring books.

for example, say you have this long method (with a long name as it does many things)

class TravelClub {

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

   }

   void buy(int amount) {...}

   void accumlatePoints(int cardNumber) {...}

   void generateRecepit() {...}

}

called as:

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

That could be refactored to:

class TravelClub {

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

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

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


}

and called as:

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

from my point of view this is a quite good manner to decompose a long method and also to decompose its name.

what do you think?

Answer 1

It has a problem in that you have to remember both to accumulate the points and perform the purchase (and generate the receipt, which is less of a problem as I assume that action has no side effects). In my mind, point accumulation should come automatically when performing a purchase. It's also rather natural that you get a receipt when performing a purchase, so in a way, your initial method was fine, except that it doesn't read very well.

If you want a fluent interface I'd introduce an extra class which gently guides the client code into doing the right thing (assuming that all purchases happen with a card and accumulate points the same way):

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);

This way, if you forgot to call withCard, nothing happens. It's easier to spot a missing transaction than an incorrect transaction, and you can't get a receipt without performing a complete transaction.

Edit: As an aside, it's funny to think that we do all this work to make methods with many parameters readable, when for example named parameters would make the problem go away completely:

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