You have multiple constructors that contain duplicate code.
Chain the constructors together to obtain the least amount of duplicate code.
 Motivation Code that's duplicated across two or more of a class's constructors is an invitation for trouble. If someone adds a new variable to a class and updates a constructor to initialize the variable, but then neglects to update the other constructors—bang, say hello to your next defect. The more constructors you have in a class, the more duplication will hurt you. It is therefore a good idea to reduce or remove duplication in your constructors. We often accomplish this refactoring with constructor chaining: specific constructors call more general-purpose constructors until a final constructor is reached. If you have one constructor at the end of every chain, it is a catch-all constructor because it handles every constructor call. A catch-all constructor often accepts more parameters than other constructors. If you find that having many constructors on your class detracts from its usability, consider applying Replace Constructors with Creation Methods (57). Mechanics - 1. Find two constructors that contain duplicate code. Determine whether one can call the other such that duplicate code can be safely (and, hopefully, easily) deleted from one of these constructors. Then make the one constructor call the other constructor such that duplicate code is reduced or eliminated.
- 2. Repeat step 1 for all constructors in the class, including ones you've already touched, in order to obtain as little duplication across all constructors as possible.
- 3. Change the visibility of any constructors that may not need to be public.
Example For this example I'll use the scenario shown in the code sketch at the beginning of this refactoring. A single Loan class has three constructors to represent different types of loans, as well as tons of bloated and ugly duplication: public Loan(float notional, float outstanding, int rating, Date expiry) { this.strategy = new TermROC(); this.notional = notional; this.outstanding = outstanding; this.rating = rating; this.expiry = expiry; } public Loan(float notional, float outstanding, int rating, Date expiry, Date maturity) { this.strategy = new RevolvingTermROC(); this.notional = notional; this.outstanding = outstanding; this.rating = rating; this.expiry = expiry; this.maturity = maturity; } public Loan(CapitalStrategy strategy, float notional, float outstanding, int rating, Date expiry, Date maturity) { this.strategy = strategy; this.notional = notional; this.outstanding = outstanding; this.rating = rating; this.expiry = expiry; this.maturity = maturity; }
Let's see what happens when I refactor this code. - 1. I study the first two constructors. They do contain duplicate code, but so does that third constructor. I consider which constructor it would be easier for the first constructor to call. I see that it could call the third constructor with a minimum amount of work. So I change the first constructor to be:
public Loan(float notional, float outstanding, int rating, Date expiry) { this(new TermROC(), notional, outstanding, rating, expiry, null); }
- 2. I repeat step 1 to remove as much duplication as possible. This leads me to the second constructor. It appears that it too can call the third constructor, as follows:
public Loan(float notional, float outstanding, int rating, Date expiry, Date maturity) { this(new RevolvingTermROC(), notional, outstanding, rating, expiry, maturity); }
I'm now aware that the third constructor is my class's catch-all constructor because it handles all of the construction details.
- 3. I check all callers of the three constructors to determine whether I can change the public visibility of any of them. In this case, I can't. (Take my word for it—you can't see the code that calls these methods.)
|
