Refactoring long methods with smooth interfaces

Question

I would like to know your opinion about using a free interface to refactor a long method.

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

The free picture is not included in refactoring books.

for example, let's say you have this long method (with a long name, as it does a lot of 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:

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

It can be reorganized into:

 class TravelClub { TravelClub buy(long amount) { //buy stuff return this; } TravelClub accumulatePoints(long cardNumber) { //accumulate stuff return this; } Receipt generateReceipt() { return new Receipt(...); } } 

and is called:

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

from my point of view, this is a pretty good way to decompose a long method and also to decompose its name.

what do you think?

Answer 1

You have a problem in that you must remember how to accumulate points and make a purchase (and generate a receipt, which is a lesser problem, since I believe that the action has no side effects). In my opinion, the accumulation of points should occur automatically when making a purchase. It is also quite natural that you receive a receipt when making a purchase, so in a sense, your original method was perfect, except that it does not read very well.

If you need a free interface, I would add an additional class that accurately directs the client code to the correct position (assuming that all purchases occur with the card and points accumulate 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); 

Thus, if you forget to call withCard , nothing will happen. It is easier to detect a missing transaction than an incorrect transaction, and you cannot receive a receipt without completing a complete transaction.

Edit:. Oddly enough, it’s ridiculous to think that we are doing all this work to make methods with many parameters readable when, for example, named parameters would completely fix the problem:

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

Answer 2

Then my counter question is what is the expected behavior if someone instead calls:

 myTravelClub.accumulatePoints(10000000L); 

without calling a purchase? Or getting a receipt before buying? I think that for free communication, you still need to adhere to other OO conventions. If you really need a fluid interface, then the buy() method would have to return another object, not TravelClub itself, but a "purchase object" that has accumulatePoints() and generateReceipt() methods.

Maybe I read a lot in the semantics of your example, but there is a reason why there are methods in the wikipedia example that can be logically called in any order. I believe another good example is the Hibernate API.

Answer 3

The long method does not match the method with a long name. In your case, the only thing I would change is the name of the method:

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

(and think of a more descriptive name for the private buy method), because all three things (“buying”, accumulating points and receiving receipts) always happen together, so from the point of view of the call code they can be a single job. In terms of implementation, one operation is simpler. KISS :-)

Answer 4

The advantage of using one method is that the same sequence is always called. for example, you cannot skip accumulatePoints as you could in the interface example below.

If the only way to call these methods is in the same sequence as in your first block of code, save it as one function. If, however, any subset of the manipulations can be done on TravelClub before receiving the receipt, then, in any case, use the free interface. This is one of the best (if not the best) ways to overcome the smell of the Raman Explosion code.

Answer 5

As long as you use the correct checks, Fluent interfaces are much easier to understand, for example, this could be the following:

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

It seems to me that part of the difficulty here lies in choosing a good descriptive name that covers everything that the method does. Naturally, the problem is that sometimes you have a lot of complex logic that you cannot easily describe with a simple name.

In the case presented in your code example, I will be tempted to simplify the name of the method itself to something more generalized:

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

What about this logical problem that I talked about? This in itself boils down to whether your method is really fixed in what it does. If a transaction can only be performed using the Buy-> Points-> Receipt sequence, then a simpler name will be used, but also a more reasonable name and convenient interface.

How about when the client does not have a reward card or does not want to receive a receipt? How about situations where several items can be purchased in one transaction - unless, of course, the purchase method can be a purchase, and not just the total amount that was calculated elsewhere? Once you start introducing questions / options into the sequence, the design becomes a little less obvious and the naming is much more complicated. Of course, you would not want to use a crazy long name, for example:

 BuyAndAddPointsIfTheCustomerHasACardAndReturnAReceiptIfTheCustomerAsksForIt(...) 

Of course, he definitely talks about what he is doing, but also points out a potential problem in that the method may be responsible for too many things or that it may hide the more complex smell of the code behind one of the methods that it calls. Similarly, a simple method name, such as “Transaction,” can simplify a complex problem that needs to be better understood.

A free interface can be very beneficial if it helps the developer make smart decisions on how to apply the quick methods called. If the call sequence is important, you need to limit the return data types to the next selection in the sequence. If the calling sequence is less important, you can use the return type with a more generalized interface that allows you to select methods in any sequence.

As for whether to really use a free interface, I do not think that this should be solved only as a means for decomposing hard-to-determine methods. You make the choice of design with which you will have to live throughout the life of the product, and from the point of view of service, I found that smooth interfaces can make design more difficult to visualize and organize and maintain in your code. Ultimately, you need to decide if this could be something you can live with as a compromise with the benefits it gives you. For me, I usually start by asking whether the use case combinations are random and simple, or if they are relatively infinite. If the latter, a free interface can help keep the code cleaner and easier to use in multiple scenarios. I would also like to consider whether the code belongs to a more generalized layer, for example an API, for example, when a free interface can work well or something more specialized.