refactoring: improving the design of existing...

638, Refactoring—Imporving the Design of Existing Code1

Refactoring: Improving the Design of Existing CodeMartin FowlerChief Scientist, [email protected]


What We Will Cover

• An example of refactoring– Blow by blow example of changes– Steps for illustrated refactorings

• Background of refactoring– Where it came from– Tools– Why and When

Fowler, Refactoring: Improving the Design of Existing Code,

Addison-Wesley, 1999


What Is Refactoring?

• Verify no change in external behavior by– Testing– Formal code analysis by tool

➨ In practice good tests are essential

A series of small steps, each of which changes the program’s internal structure without changing its external behavior

A series of small steps, each of which changes the program’s internal structure without changing its external behavior


Starting Class Diagram

priceCode: int

Movie

daysRented: int

Rental

statement()

Customer✻1

✻ 1


Sample Output

Rental Record for Dinsdale Pirhana

Monty Python and the Holy Grail 3.5

Ran2

Star Trek 27 6

Star Wars 3.2 3

Wallace and Gromit 6

Amount owed is 20.5

You earned 6 frequent renter points


Class Movie

public class Movie {public class Movie {public class Movie {public class Movie {public static final int CHILDRENS = 2;public static final int CHILDRENS = 2;public static final int CHILDRENS = 2;public static final int CHILDRENS = 2;public static final int REGULAR = 0;public static final int REGULAR = 0;public static final int REGULAR = 0;public static final int REGULAR = 0;public static final int NEW_RELEASE = 1;public static final int NEW_RELEASE = 1;public static final int NEW_RELEASE = 1;public static final int NEW_RELEASE = 1;

private String _title;private String _title;private String _title;private String _title;private int _priceCode;private int _priceCode;private int _priceCode;private int _priceCode;

public Movie(String title, int priceCode) {public Movie(String title, int priceCode) {public Movie(String title, int priceCode) {public Movie(String title, int priceCode) {_title = title;_title = title;_title = title;_title = title;_priceCode = priceCode;_priceCode = priceCode;_priceCode = priceCode;_priceCode = priceCode;

}}}}public int getPriceCode() {public int getPriceCode() {public int getPriceCode() {public int getPriceCode() {

return _priceCode;return _priceCode;return _priceCode;return _priceCode;}}}}public void setPriceCode(int arg) {public void setPriceCode(int arg) {public void setPriceCode(int arg) {public void setPriceCode(int arg) {

_priceCode = arg;_priceCode = arg;_priceCode = arg;_priceCode = arg;}}}}public String getTitle () {public String getTitle () {public String getTitle () {public String getTitle () {

return _title; return _title; return _title; return _title; };};};};

}}}}


Class Rental

class Rental {class Rental {class Rental {class Rental {private Movie _movie;private Movie _movie;private Movie _movie;private Movie _movie;privateprivateprivateprivate intintintint ____daysRenteddaysRenteddaysRenteddaysRented;;;;

public Rental(Movie movie,public Rental(Movie movie,public Rental(Movie movie,public Rental(Movie movie, int daysRentedint daysRentedint daysRentedint daysRented) {) {) {) {_movie = movie;_movie = movie;_movie = movie;_movie = movie;____daysRenteddaysRenteddaysRenteddaysRented ==== daysRenteddaysRenteddaysRenteddaysRented;;;;

}}}}publicpublicpublicpublic int getDaysRentedint getDaysRentedint getDaysRentedint getDaysRented() {() {() {() {

return _return _return _return _daysRenteddaysRenteddaysRenteddaysRented;;;;}}}}public Moviepublic Moviepublic Moviepublic Movie getMoviegetMoviegetMoviegetMovie() {() {() {() {

return _movie;return _movie;return _movie;return _movie;}}}}

}}}}


Class Customer (Almost)class Customer {class Customer {class Customer {class Customer {

private String _name;private String _name;private String _name;private String _name;

private Vector _rentals = new Vector();private Vector _rentals = new Vector();private Vector _rentals = new Vector();private Vector _rentals = new Vector();

public Customer (String name)public Customer (String name)public Customer (String name)public Customer (String name) {{{{

_name = name;_name = name;_name = name;_name = name;

};};};};

public voidpublic voidpublic voidpublic void addRentaladdRentaladdRentaladdRental(Rental(Rental(Rental(Rental argargargarg) {) {) {) {

_rentals._rentals._rentals._rentals.addElementaddElementaddElementaddElement((((argargargarg););););

}}}}

public Stringpublic Stringpublic Stringpublic String getNamegetNamegetNamegetName ()()()() {{{{

return _name;return _name;return _name;return _name;

};};};};

public String statement() public String statement() public String statement() public String statement() // see next slide// see next slide// see next slide// see next slide


Customer.statement() Part 1public String statement() {public String statement() {public String statement() {public String statement() {

double double double double totalAmounttotalAmounttotalAmounttotalAmount = 0;= 0;= 0;= 0;int frequentRenterPointsint frequentRenterPointsint frequentRenterPointsint frequentRenterPoints = 0;= 0;= 0;= 0;Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();String result = "Rental Record for " +String result = "Rental Record for " +String result = "Rental Record for " +String result = "Rental Record for " + getNamegetNamegetNamegetName() + "() + "() + "() + "\\\\n";n";n";n";while (rentals.while (rentals.while (rentals.while (rentals.hasMoreElementshasMoreElementshasMoreElementshasMoreElements()) {()) {()) {()) {

double double double double thisAmountthisAmountthisAmountthisAmount = 0;= 0;= 0;= 0;Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.nextElementnextElementnextElementnextElement();();();();

//determine amounts for each line//determine amounts for each line//determine amounts for each line//determine amounts for each lineswitch (each.switch (each.switch (each.switch (each.getMoviegetMoviegetMoviegetMovie().().().().getPriceCodegetPriceCodegetPriceCodegetPriceCode()) {()) {()) {()) {

case Movie.REGULAR:case Movie.REGULAR:case Movie.REGULAR:case Movie.REGULAR:thisAmountthisAmountthisAmountthisAmount += 2;+= 2;+= 2;+= 2;if (each.if (each.if (each.if (each.getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() > 2)() > 2)() > 2)() > 2)

thisAmountthisAmountthisAmountthisAmount += (each.+= (each.+= (each.+= (each.getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() () () () ---- 2) * 1.5;2) * 1.5;2) * 1.5;2) * 1.5;break;break;break;break;

case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:thisAmountthisAmountthisAmountthisAmount += each.+= each.+= each.+= each.getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() * 3;() * 3;() * 3;() * 3;break;break;break;break;

case Movie.CHILDRENS:case Movie.CHILDRENS:case Movie.CHILDRENS:case Movie.CHILDRENS:thisAmountthisAmountthisAmountthisAmount += 1.5;+= 1.5;+= 1.5;+= 1.5;if (each.if (each.if (each.if (each.getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() > 3)() > 3)() > 3)() > 3)


} //} //} //} //continues on next slidecontinues on next slidecontinues on next slidecontinues on next slide


Customer.statement() Part 2

// add frequent renter points// add frequent renter points// add frequent renter points// add frequent renter pointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPoints ++;++;++;++;// add bonus for a two day new release rental// add bonus for a two day new release rental// add bonus for a two day new release rental// add bonus for a two day new release rentalif ((each.if ((each.if ((each.if ((each.getMoviegetMoviegetMoviegetMovie().().().().getPriceCodegetPriceCodegetPriceCodegetPriceCode() == Movie.NEW_RELEASE) && () == Movie.NEW_RELEASE) && () == Movie.NEW_RELEASE) && () == Movie.NEW_RELEASE) && each.each.each.each.getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() > 1)() > 1)() > 1)() > 1) frequentRenterPointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPoints ++;++;++;++;

//show figures for this rental//show figures for this rental//show figures for this rental//show figures for this rentalresult += "result += "result += "result += "\\\\t" + each.t" + each.t" + each.t" + each.getMoviegetMoviegetMoviegetMovie().().().().getTitlegetTitlegetTitlegetTitle()+ "()+ "()+ "()+ "\\\\t" + t" + t" + t" + String.String.String.String.valueOfvalueOfvalueOfvalueOf((((thisAmountthisAmountthisAmountthisAmount) + ") + ") + ") + "\\\\n";n";n";n";totalAmounttotalAmounttotalAmounttotalAmount +=+=+=+= thisAmountthisAmountthisAmountthisAmount;;;;

}}}}//add footer lines//add footer lines//add footer lines//add footer linesresult += "Amount owed is " + String.result += "Amount owed is " + String.result += "Amount owed is " + String.result += "Amount owed is " + String.valueOfvalueOfvalueOfvalueOf((((totalAmounttotalAmounttotalAmounttotalAmount) + ) + ) + ) +

""""\\\\n";n";n";n";result += "You earned " + String.result += "You earned " + String.result += "You earned " + String.result += "You earned " + String.valueOfvalueOfvalueOfvalueOf((((frequentRenterPointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPoints) + ) + ) + ) +

" frequent renter points";" frequent renter points";" frequent renter points";" frequent renter points";return result;return result;return result;return result;

}}}}


Interactions For Statement

aCustomer aRental aMovie

getMovie

* [for all rentals]

getPriceCode

getDaysRented

statement


Requirements Changes

• Produce an html version of the statement• The movie classifications will soon change

– Together with the rules for charging and for frequent renter points


Extract Method

��

��

��

��

��

��

��

��

��

��

void printOwing() {printBanner();

//print detailsSystem.out.println ("name:" + _name);System.out.println ("amount" + getOutstanding());

}

��

��

��

��

��

��

��

��

��

��

��

��

��

void printOwing() {printBanner();printDetails(getOutstanding());

}

void printDetails (double outstanding) {System.out.println ("name:" + _name);System.out.println ("amount" + outstanding);

}

��

��

��

��

��

��

��

��

��

��

��

��

��

��

��

��

��

��

You have a code fragment that can be grouped togetherTurn the fragment into a method whose name explains

the purpose of the method


Candidate Extractionpublic String statement() {public String statement() {public String statement() {public String statement() {

double double double double totalAmounttotalAmounttotalAmounttotalAmount = 0;= 0;= 0;= 0;int frequentRenterPointsint frequentRenterPointsint frequentRenterPointsint frequentRenterPoints = 0;= 0;= 0;= 0;Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();String result = "Rental Record for " +String result = "Rental Record for " +String result = "Rental Record for " +String result = "Rental Record for " + getNamegetNamegetNamegetName() + "() + "() + "() + "\\\\n";n";n";n";while (rentals.while (rentals.while (rentals.while (rentals.hasMoreElementshasMoreElementshasMoreElementshasMoreElements()) {()) {()) {()) {

double double double double thisAmountthisAmountthisAmountthisAmount = 0;= 0;= 0;= 0;Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.nextElementnextElementnextElementnextElement();();();();

//determine amounts for each line//determine amounts for each line//determine amounts for each line//determine amounts for each lineswitch (each.switch (each.switch (each.switch (each.getMoviegetMoviegetMoviegetMovie().().().().getPriceCodegetPriceCodegetPriceCodegetPriceCode()) {()) {()) {()) {






} //} //} //} //[snip][snip][snip][snip]


Steps for Extract Method

• Create method named after intention of code• Copy extracted code• Look for local variables and parameters

– Turn into parameter– Turn into return value– Declare within method

• Compile• Replace code fragment with call to new method• Compile and test


Extracting the Amount Calculation privateprivateprivateprivate int amountForint amountForint amountForint amountFor(Rental each) {(Rental each) {(Rental each) {(Rental each) {

int thisAmountint thisAmountint thisAmountint thisAmount = 0;= 0;= 0;= 0;switch (each.switch (each.switch (each.switch (each.getMoviegetMoviegetMoviegetMovie().().().().getPriceCodegetPriceCodegetPriceCodegetPriceCode()) {()) {()) {()) {


thisAmountthisAmountthisAmountthisAmount += (each.+= (each.+= (each.+= (each.getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() () () () ---- 2) * 2) * 2) * 2) * 1.5;1.5;1.5;1.5;

break;break;break;break;case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:

thisAmountthisAmountthisAmountthisAmount += each.+= each.+= each.+= each.getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() * 3;() * 3;() * 3;() * 3;break;break;break;break;


thisAmountthisAmountthisAmountthisAmount += (each.+= (each.+= (each.+= (each.getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() () () () ---- 3) * 3) * 3) * 3) * 1.5;1.5;1.5;1.5;

break;break;break;break;}}}}returnreturnreturnreturn thisAmountthisAmountthisAmountthisAmount;;;;

}}}}


Statement() After Extractionpublic String statement() {public String statement() {public String statement() {public String statement() {

doubledoubledoubledouble totalAmounttotalAmounttotalAmounttotalAmount = 0;= 0;= 0;= 0;int frequentRenterPointsint frequentRenterPointsint frequentRenterPointsint frequentRenterPoints = 0;= 0;= 0;= 0;Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();String result = "Rental Record for " +String result = "Rental Record for " +String result = "Rental Record for " +String result = "Rental Record for " + getNamegetNamegetNamegetName() + "() + "() + "() + "\\\\n";n";n";n";while (rentals.while (rentals.while (rentals.while (rentals.hasMoreElementshasMoreElementshasMoreElementshasMoreElements()) {()) {()) {()) {

doubledoubledoubledouble thisAmountthisAmountthisAmountthisAmount = 0;= 0;= 0;= 0;Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.nextElementnextElementnextElementnextElement();();();();

thisAmountthisAmountthisAmountthisAmount ==== amountForamountForamountForamountFor(each);(each);(each);(each);

// add frequent renter points// add frequent renter points// add frequent renter points// add frequent renter pointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPoints ++;++;++;++;// add bonus for a two day new release rental// add bonus for a two day new release rental// add bonus for a two day new release rental// add bonus for a two day new release rentalif ((each.if ((each.if ((each.if ((each.getMoviegetMoviegetMoviegetMovie().().().().getPriceCodegetPriceCodegetPriceCodegetPriceCode() == Movie.NEW_RELEASE) && () == Movie.NEW_RELEASE) && () == Movie.NEW_RELEASE) && () == Movie.NEW_RELEASE) && each.each.each.each.getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() > 1)() > 1)() > 1)() > 1) frequentRenterPointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPoints ++;++;++;++;

//show figures for this rental//show figures for this rental//show figures for this rental//show figures for this rentalresult += "result += "result += "result += "\\\\t" + each.t" + each.t" + each.t" + each.getMoviegetMoviegetMoviegetMovie().().().().getTitlegetTitlegetTitlegetTitle()+ "()+ "()+ "()+ "\\\\t" + t" + t" + t" + String.String.String.String.valueOfvalueOfvalueOfvalueOf((((thisAmountthisAmountthisAmountthisAmount) + ") + ") + ") + "\\\\n";n";n";n";totalAmounttotalAmounttotalAmounttotalAmount +=+=+=+= thisAmountthisAmountthisAmountthisAmount;;;;

}}}}//add footer lines//add footer lines//add footer lines//add footer linesresult += "Amount owed is " + String.result += "Amount owed is " + String.result += "Amount owed is " + String.result += "Amount owed is " + String.valueOfvalueOfvalueOfvalueOf((((totalAmounttotalAmounttotalAmounttotalAmount) + ") + ") + ") + "\\\\n";n";n";n";result += "You earned " + String.result += "You earned " + String.result += "You earned " + String.result += "You earned " + String.valueOfvalueOfvalueOfvalueOf((((frequentRenterPointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPoints) + ) + ) + ) + " frequent renter points";" frequent renter points";" frequent renter points";" frequent renter points";

return result;return result;return result;return result;

}}}}}}}}


Extracting the Amount Calculation (2)

private double private double private double private double amountForamountForamountForamountFor(Rental each) {(Rental each) {(Rental each) {(Rental each) {double double double double thisAmountthisAmountthisAmountthisAmount = 0;= 0;= 0;= 0;switch (each.switch (each.switch (each.switch (each.getMoviegetMoviegetMoviegetMovie().().().().getPriceCodegetPriceCodegetPriceCodegetPriceCode()) {()) {()) {()) {






}}}}returnreturnreturnreturn thisAmountthisAmountthisAmountthisAmount;;;;

}}}}


Change Names of Variables

private doubleprivate doubleprivate doubleprivate double amountForamountForamountForamountFor(Rental(Rental(Rental(Rental aRentalaRentalaRentalaRental) {) {) {) {double double double double resultresultresultresult = 0;= 0;= 0;= 0;switch (switch (switch (switch (aRentalaRentalaRentalaRental....getMoviegetMoviegetMoviegetMovie().().().().getPriceCodegetPriceCodegetPriceCodegetPriceCode()) {()) {()) {()) {

case Movie.REGULAR:case Movie.REGULAR:case Movie.REGULAR:case Movie.REGULAR:resultresultresultresult += 2;+= 2;+= 2;+= 2;if (if (if (if (aRentalaRentalaRentalaRental....getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() > 2)() > 2)() > 2)() > 2)

resultresultresultresult += (+= (+= (+= (aRentalaRentalaRentalaRental....getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() () () () ---- 2) * 1.5;2) * 1.5;2) * 1.5;2) * 1.5;break;break;break;break;

case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:resultresultresultresult +=+=+=+= aRentalaRentalaRentalaRental....getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() * 3;() * 3;() * 3;() * 3;break;break;break;break;

case Movie.CHILDRENS:case Movie.CHILDRENS:case Movie.CHILDRENS:case Movie.CHILDRENS:result += 1.5;result += 1.5;result += 1.5;result += 1.5;if (if (if (if (aRentalaRentalaRentalaRental....getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() > 3)() > 3)() > 3)() > 3)

resultresultresultresult += (+= (+= (+= (aRentalaRentalaRentalaRental....getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() () () () ---- 3) * 1.5;3) * 1.5;3) * 1.5;3) * 1.5;break;break;break;break;

}}}}return return return return resultresultresultresult;;;;

}}}}


Move Method

aMethod()

Class 1

Class 2

Class 1

aMethod()

Class 2

A method is, or will be, using or used by more features of another class than the class it is defined on.

Create a new method with a similar body in the class it uses most. Either turn the old method into a simple delegation, or remove it altogether.


Steps for Move Method

• Declare method in target class• Copy and fit code• Set up a reference from the source object

to the target• Turn the original method into a delegating method

– amountOf(Rental each) {return each.charge()}– Check for overriding methods

• Compile and test• Find all users of the method

– Adjust them to call method on target• Remove original method• Compile and test


Moving Amount() to Rentalclass Rentalclass Rentalclass Rentalclass Rental ............

doubledoubledoubledouble getChargegetChargegetChargegetCharge() {() {() {() {double result = 0;double result = 0;double result = 0;double result = 0;switch (switch (switch (switch (getMoviegetMoviegetMoviegetMovie().().().().getPriceCodegetPriceCodegetPriceCodegetPriceCode()) {()) {()) {()) {

case Movie.REGULAR:case Movie.REGULAR:case Movie.REGULAR:case Movie.REGULAR:result += 2;result += 2;result += 2;result += 2;if (if (if (if (getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() > 2)() > 2)() > 2)() > 2)

result += (result += (result += (result += (getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() () () () ---- 2) * 1.5;2) * 1.5;2) * 1.5;2) * 1.5;break;break;break;break;

case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:result +=result +=result +=result += getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() * 3;() * 3;() * 3;() * 3;break;break;break;break;

case Movie.CHILDRENS:case Movie.CHILDRENS:case Movie.CHILDRENS:case Movie.CHILDRENS:result += 1.5;result += 1.5;result += 1.5;result += 1.5;if (if (if (if (getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() > 3)() > 3)() > 3)() > 3)

result += (result += (result += (result += (getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() () () () ---- 3) * 1.5;3) * 1.5;3) * 1.5;3) * 1.5;break;break;break;break;

}}}}return result;return result;return result;return result;

}}}}

1 ✻

statement()

Customer

getCharge()

daysRented: int

Rental

priceCode: int

Movie


Altered Statementclass Customer... class Customer... class Customer... class Customer...

public String statement() {public String statement() {public String statement() {public String statement() {double totalAmount = 0;double totalAmount = 0;double totalAmount = 0;double totalAmount = 0;int frequentRenterPoints = 0;int frequentRenterPoints = 0;int frequentRenterPoints = 0;int frequentRenterPoints = 0;Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();String result = "Rental Record for " + getName() + "String result = "Rental Record for " + getName() + "String result = "Rental Record for " + getName() + "String result = "Rental Record for " + getName() + "\\\\n";n";n";n";while (rentals.hasMoreElements()) {while (rentals.hasMoreElements()) {while (rentals.hasMoreElements()) {while (rentals.hasMoreElements()) {

double thisAmount = 0;double thisAmount = 0;double thisAmount = 0;double thisAmount = 0;Rental each = (Rental) rentals.nextElement();Rental each = (Rental) rentals.nextElement();Rental each = (Rental) rentals.nextElement();Rental each = (Rental) rentals.nextElement();

thisAmountthisAmountthisAmountthisAmount = = = = each.each.each.each.getChargegetChargegetChargegetCharge();();();();// add frequent renter points// add frequent renter points// add frequent renter points// add frequent renter pointsfrequentRenterPoints ++;frequentRenterPoints ++;frequentRenterPoints ++;frequentRenterPoints ++;// add bonus for a two day new release rental// add bonus for a two day new release rental// add bonus for a two day new release rental// add bonus for a two day new release rentalif ((each.getMovie().getPriceCode() == Movie.NEW_RELif ((each.getMovie().getPriceCode() == Movie.NEW_RELif ((each.getMovie().getPriceCode() == Movie.NEW_RELif ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && EASE) && EASE) && EASE) &&

each.getDaysRented() > 1) frequentRenterPoints ++;each.getDaysRented() > 1) frequentRenterPoints ++;each.getDaysRented() > 1) frequentRenterPoints ++;each.getDaysRented() > 1) frequentRenterPoints ++;

//show figures for this rental//show figures for this rental//show figures for this rental//show figures for this rentalresult += "result += "result += "result += "\\\\t" + each.getMovie().getTitle()+ "t" + each.getMovie().getTitle()+ "t" + each.getMovie().getTitle()+ "t" + each.getMovie().getTitle()+ "\\\\t" + t" + t" + t" +

String.valueOf(thisAmount) + "String.valueOf(thisAmount) + "String.valueOf(thisAmount) + "String.valueOf(thisAmount) + "\\\\n";n";n";n";totalAmount += thisAmount;totalAmount += thisAmount;totalAmount += thisAmount;totalAmount += thisAmount;

}}}}//add footer lines//add footer lines//add footer lines//add footer linesresult += "Amount owed is " + String.valueOf(totalAmounresult += "Amount owed is " + String.valueOf(totalAmounresult += "Amount owed is " + String.valueOf(totalAmounresult += "Amount owed is " + String.valueOf(totalAmount) + "t) + "t) + "t) + "\\\\n";n";n";n";result += "You earned " + String.valueOf(frequentRenterPresult += "You earned " + String.valueOf(frequentRenterPresult += "You earned " + String.valueOf(frequentRenterPresult += "You earned " + String.valueOf(frequentRenterPoints) + oints) + oints) + oints) +


}}}}


Problems With Tempsclass Customer... class Customer... class Customer... class Customer...

public String statement() {public String statement() {public String statement() {public String statement() {double totalAmount = 0;double totalAmount = 0;double totalAmount = 0;double totalAmount = 0;int frequentRenterPoints = 0;int frequentRenterPoints = 0;int frequentRenterPoints = 0;int frequentRenterPoints = 0;Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();String result = "Rental Record for " + getName() + "String result = "Rental Record for " + getName() + "String result = "Rental Record for " + getName() + "String result = "Rental Record for " + getName() + "\\\\n";n";n";n";while (rentals.hasMoreElements()) {while (rentals.hasMoreElements()) {while (rentals.hasMoreElements()) {while (rentals.hasMoreElements()) {

double thisAmount = 0;double thisAmount = 0;double thisAmount = 0;double thisAmount = 0;Rental each = (Rental) rentals.nextElement();Rental each = (Rental) rentals.nextElement();Rental each = (Rental) rentals.nextElement();Rental each = (Rental) rentals.nextElement();

thisAmountthisAmountthisAmountthisAmount = each.= each.= each.= each.getChargegetChargegetChargegetCharge();();();();// add frequent renter points// add frequent renter points// add frequent renter points// add frequent renter pointsfrequentRenterPoints ++;frequentRenterPoints ++;frequentRenterPoints ++;frequentRenterPoints ++;// add bonus for a two day new release rental// add bonus for a two day new release rental// add bonus for a two day new release rental// add bonus for a two day new release rentalif ((each.getMovie().getPriceCode() == Movie.NEW_RELif ((each.getMovie().getPriceCode() == Movie.NEW_RELif ((each.getMovie().getPriceCode() == Movie.NEW_RELif ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) && EASE) && EASE) && EASE) &&

each.getDaysRented() > 1) frequentRenterPoints ++;each.getDaysRented() > 1) frequentRenterPoints ++;each.getDaysRented() > 1) frequentRenterPoints ++;each.getDaysRented() > 1) frequentRenterPoints ++;

//show figures for this rental//show figures for this rental//show figures for this rental//show figures for this rentalresult += "result += "result += "result += "\\\\t" + each.getMovie().getTitle()+ "t" + each.getMovie().getTitle()+ "t" + each.getMovie().getTitle()+ "t" + each.getMovie().getTitle()+ "\\\\t" + t" + t" + t" +

String.valueOf(String.valueOf(String.valueOf(String.valueOf(thisAmountthisAmountthisAmountthisAmount) + ") + ") + ") + "\\\\n";n";n";n";totalAmount += totalAmount += totalAmount += totalAmount += thisAmountthisAmountthisAmountthisAmount;;;;

}}}}//add footer lines//add footer lines//add footer lines//add footer linesresult += "Amount owed is " + String.valueOf(totalAmounresult += "Amount owed is " + String.valueOf(totalAmounresult += "Amount owed is " + String.valueOf(totalAmounresult += "Amount owed is " + String.valueOf(totalAmount) + "t) + "t) + "t) + "\\\\n";n";n";n";result += "You earned " + String.valueOf(frequentRenterPresult += "You earned " + String.valueOf(frequentRenterPresult += "You earned " + String.valueOf(frequentRenterPresult += "You earned " + String.valueOf(frequentRenterPoints) + oints) + oints) + oints) +


}}}}


A Word About Performance

• Most of a program’s time is taken in a small part of the code

• Profile a running program to find these “hot spots”– You won’t be able to find them by eye

• Optimize the hot spots, and measure the improvement

The best way to optimize performance is to first write a well factored program, then optimize it.The best way to optimize performance is to first write a well factored program, then optimize it.

McConnell Steve, Code Complete: A Practical Handbook of Software

Construction, Microsoft Press, 1993


Replace Temp With Query

You are using a temporary variable to hold the result of an expression

Extract the expression into a method. Replace all references to the temp with

the expression. The new method can then be used in other methods


Steps for Replace Temp With Query• Find temp with a single assignment• Extract Right Hand Side of assignment• Replace all references of temp

with new method• Remove declaration and assignment

of temp• Compile and test


thisAmount Removedpublic String statement() {public String statement() {public String statement() {public String statement() {


Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.nextElementnextElementnextElementnextElement();();();();

// add frequent renter points// add frequent renter points// add frequent renter points// add frequent renter pointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPoints ++;++;++;++;// add bonus for a two day new release rental// add bonus for a two day new release rental// add bonus for a two day new release rental// add bonus for a two day new release rentalif ((each.if ((each.if ((each.if ((each.getMoviegetMoviegetMoviegetMovie().().().().getPriceCodegetPriceCodegetPriceCodegetPriceCode() == Movie.NEW_RELEASE) &&() == Movie.NEW_RELEASE) &&() == Movie.NEW_RELEASE) &&() == Movie.NEW_RELEASE) &&each.each.each.each.getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() > 1)() > 1)() > 1)() > 1) frequentRenterPointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPoints ++;++;++;++;

//show figures for this rental//show figures for this rental//show figures for this rental//show figures for this rentalresult += "result += "result += "result += "\\\\t" + each.t" + each.t" + each.t" + each.getMoviegetMoviegetMoviegetMovie().().().().getTitlegetTitlegetTitlegetTitle()+ "()+ "()+ "()+ "\\\\t" + t" + t" + t" + String.String.String.String.valueOfvalueOfvalueOfvalueOf((((each.each.each.each.getChargegetChargegetChargegetCharge())())())()) + "+ "+ "+ "\\\\n";n";n";n";totalAmounttotalAmounttotalAmounttotalAmount += += += += each.each.each.each.getChargegetChargegetChargegetCharge();();();();

}}}}//add footer lines//add footer lines//add footer lines//add footer linesresult += "Amount owed is " + String.result += "Amount owed is " + String.result += "Amount owed is " + String.result += "Amount owed is " + String.valueOfvalueOfvalueOfvalueOf((((totalAmounttotalAmounttotalAmounttotalAmount) + ") + ") + ") + "\\\\n";n";n";n";result += "You earned " + String.result += "You earned " + String.result += "You earned " + String.result += "You earned " + String.valueOfvalueOfvalueOfvalueOf((((frequentRenterPointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPoints) + ) + ) + ) +


}}}}}}}}


Extract and Move frequentRenterPoints()

class Customer...class Customer...class Customer...class Customer...public String statement() {public String statement() {public String statement() {public String statement() {


Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.nextElementnextElementnextElementnextElement();();();();frequentRenterPointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPoints += each.+= each.+= each.+= each.getFrequentRenterPointsgetFrequentRenterPointsgetFrequentRenterPointsgetFrequentRenterPoints();();();();

//show figures for this rental//show figures for this rental//show figures for this rental//show figures for this rentalresult += "result += "result += "result += "\\\\t" + each.t" + each.t" + each.t" + each.getMoviegetMoviegetMoviegetMovie().().().().getTitlegetTitlegetTitlegetTitle()+ "()+ "()+ "()+ "\\\\t" +t" +t" +t" +

String.String.String.String.valueOfvalueOfvalueOfvalueOf(each.(each.(each.(each.getChargegetChargegetChargegetCharge()) + "()) + "()) + "()) + "\\\\n";n";n";n";totalAmounttotalAmounttotalAmounttotalAmount += each.+= each.+= each.+= each.getChargegetChargegetChargegetCharge();();();();

}}}}

//add footer lines//add footer lines//add footer lines//add footer linesresult += "Amount owed is " + String.result += "Amount owed is " + String.result += "Amount owed is " + String.result += "Amount owed is " + String.valueOfvalueOfvalueOfvalueOf((((totalAmounttotalAmounttotalAmounttotalAmount) + ") + ") + ") + "\\\\n";n";n";n";result += "You earned " + String.result += "You earned " + String.result += "You earned " + String.result += "You earned " + String.valueOfvalueOfvalueOfvalueOf((((frequentRenterPointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPoints) +) +) +) +


}}}}


After Moving Charge and Frequent Renter Points

✻1statement()

Customer

getCharge()getFrequentRenterPoints()

daysRented: int

Rental

priceCode: int

Movie


getCharge

* [for all rentals]

getPriceCode

statement

getFrequentRenterPointsgetPriceCode


More Temps to Killclass Customer...class Customer...class Customer...class Customer...

public String statement() {public String statement() {public String statement() {public String statement() {doubledoubledoubledouble totalAmounttotalAmounttotalAmounttotalAmount = 0;= 0;= 0;= 0;int frequentRenterPointsint frequentRenterPointsint frequentRenterPointsint frequentRenterPoints = 0;= 0;= 0;= 0;Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();String result = "Rental Record for " +String result = "Rental Record for " +String result = "Rental Record for " +String result = "Rental Record for " + getNamegetNamegetNamegetName() + "() + "() + "() + "\\\\n";n";n";n";while (rentals.while (rentals.while (rentals.while (rentals.hasMoreElementshasMoreElementshasMoreElementshasMoreElements()) {()) {()) {()) {

Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.nextElementnextElementnextElementnextElement();();();();frequentRenterPointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPoints += each.+= each.+= each.+= each.getFrequentRenterPointsgetFrequentRenterPointsgetFrequentRenterPointsgetFrequentRenterPoints();();();();

//show figures for this rental//show figures for this rental//show figures for this rental//show figures for this rentalresult += "result += "result += "result += "\\\\t" + each.t" + each.t" + each.t" + each.getMoviegetMoviegetMoviegetMovie().().().().getTitlegetTitlegetTitlegetTitle()+ "()+ "()+ "()+ "\\\\t" +t" +t" +t" +

String.String.String.String.valueOfvalueOfvalueOfvalueOf(each.(each.(each.(each.getChargegetChargegetChargegetCharge()) + "()) + "()) + "()) + "\\\\n";n";n";n";totalAmounttotalAmounttotalAmounttotalAmount += each.+= each.+= each.+= each.getChargegetChargegetChargegetCharge();();();();

}}}}

//add footer lines//add footer lines//add footer lines//add footer linesresult += "Amount owed is " + String.result += "Amount owed is " + String.result += "Amount owed is " + String.result += "Amount owed is " + String.valueOfvalueOfvalueOfvalueOf((((totalAmounttotalAmounttotalAmounttotalAmount) + ") + ") + ") + "\\\\n";n";n";n";result += "You earned " + String.result += "You earned " + String.result += "You earned " + String.result += "You earned " + String.valueOfvalueOfvalueOfvalueOf((((frequentRenterPointsfrequentRenterPointsfrequentRenterPointsfrequentRenterPoints) +) +) +) +


}}}}


The New Methods

class Customer…class Customer…class Customer…class Customer…

private doubleprivate doubleprivate doubleprivate double getTotalChargegetTotalChargegetTotalChargegetTotalCharge() {() {() {() {double result = 0;double result = 0;double result = 0;double result = 0;Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();while (rentals.while (rentals.while (rentals.while (rentals.hasMoreElementshasMoreElementshasMoreElementshasMoreElements()) {()) {()) {()) {

Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.nextElementnextElementnextElementnextElement();();();();result += each.result += each.result += each.result += each.getChargegetChargegetChargegetCharge();();();();


}}}}

privateprivateprivateprivate int getTotalFrequentRenterPointsint getTotalFrequentRenterPointsint getTotalFrequentRenterPointsint getTotalFrequentRenterPoints(){(){(){(){intintintint result = 0;result = 0;result = 0;result = 0;Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();while (rentals.while (rentals.while (rentals.while (rentals.hasMoreElementshasMoreElementshasMoreElementshasMoreElements()) {()) {()) {()) {

Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.nextElementnextElementnextElementnextElement();();();();result += each.result += each.result += each.result += each.getFrequentRenterPointsgetFrequentRenterPointsgetFrequentRenterPointsgetFrequentRenterPoints();();();();


}}}}


The Temps Removed

public String statement() {public String statement() {public String statement() {public String statement() {Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();String result = "Rental Record for " +String result = "Rental Record for " +String result = "Rental Record for " +String result = "Rental Record for " + getNamegetNamegetNamegetName() + "() + "() + "() + "\\\\n";n";n";n";while (rentals.while (rentals.while (rentals.while (rentals.hasMoreElementshasMoreElementshasMoreElementshasMoreElements()) {()) {()) {()) {


//show figures for this rental//show figures for this rental//show figures for this rental//show figures for this rentalresult += "result += "result += "result += "\\\\t" + each.t" + each.t" + each.t" + each.getMoviegetMoviegetMoviegetMovie().().().().getTitlegetTitlegetTitlegetTitle()+ "()+ "()+ "()+ "\\\\t" + t" + t" + t" +

String.String.String.String.valueOfvalueOfvalueOfvalueOf(each.(each.(each.(each.getChargegetChargegetChargegetCharge()) + "()) + "()) + "()) + "\\\\n";n";n";n";}}}}

//add footer lines//add footer lines//add footer lines//add footer linesresult += "Amount owed is " + String.result += "Amount owed is " + String.result += "Amount owed is " + String.result += "Amount owed is " + String.valueOfvalueOfvalueOfvalueOf((((getTotalChargegetTotalChargegetTotalChargegetTotalCharge()()()()) + ") + ") + ") + "\\\\n";n";n";n";result += "You earned " + result += "You earned " + result += "You earned " + result += "You earned " +

String.String.String.String.valueOfvalueOfvalueOfvalueOf((((getTotalFrequentRenterPointsgetTotalFrequentRenterPointsgetTotalFrequentRenterPointsgetTotalFrequentRenterPoints()()()()) + ) + ) + ) + " frequent renter points";" frequent renter points";" frequent renter points";" frequent renter points";

return result;return result;return result;return result;}}}}


After Replacing the Totals

✻1statement()getTotalCharge()getTotalFrequentRenterPoints()

Customer

getCharge()getFrequentRenterPoints()

daysRented: int

Rental

priceCode: int

Movie


* [for all rentals] getCharge

getTotalCharge

getPriceCode

statement

* [for all rentals] getFrequentRenterPoints

getPriceCode

getTotalFrequentRenterPoints


htmlStatement()

public Stringpublic Stringpublic Stringpublic String htmlStatementhtmlStatementhtmlStatementhtmlStatement() {() {() {() {Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();String result = "<H1>Rentals for <EM>" +String result = "<H1>Rentals for <EM>" +String result = "<H1>Rentals for <EM>" +String result = "<H1>Rentals for <EM>" + getNamegetNamegetNamegetName() + "</EM></H1><P>() + "</EM></H1><P>() + "</EM></H1><P>() + "</EM></H1><P>\\\\n";n";n";n";while (rentals.while (rentals.while (rentals.while (rentals.hasMoreElementshasMoreElementshasMoreElementshasMoreElements()) {()) {()) {()) {

Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.nextElementnextElementnextElementnextElement();();();();//show figures for each rental//show figures for each rental//show figures for each rental//show figures for each rentalresult += each.result += each.result += each.result += each.getMoviegetMoviegetMoviegetMovie().().().().getTitlegetTitlegetTitlegetTitle()+ ": " +()+ ": " +()+ ": " +()+ ": " +

String.String.String.String.valueOfvalueOfvalueOfvalueOf(each.(each.(each.(each.getChargegetChargegetChargegetCharge()) + "<BR>()) + "<BR>()) + "<BR>()) + "<BR>\\\\n";n";n";n";}}}}//add footer lines//add footer lines//add footer lines//add footer linesresult += "<P>You owe <EM>" + String.result += "<P>You owe <EM>" + String.result += "<P>You owe <EM>" + String.result += "<P>You owe <EM>" + String.valueOfvalueOfvalueOfvalueOf((((getTotalChargegetTotalChargegetTotalChargegetTotalCharge()) +()) +()) +()) +

"</EM><P>"</EM><P>"</EM><P>"</EM><P>\\\\n";n";n";n";result += "On this rental you earned <EM>" +result += "On this rental you earned <EM>" +result += "On this rental you earned <EM>" +result += "On this rental you earned <EM>" +

String.String.String.String.valueOfvalueOfvalueOfvalueOf((((getTotalFrequentRenterPointsgetTotalFrequentRenterPointsgetTotalFrequentRenterPointsgetTotalFrequentRenterPoints()) +()) +()) +()) +"</EM> frequent renter points<P>";"</EM> frequent renter points<P>";"</EM> frequent renter points<P>";"</EM> frequent renter points<P>";

return result;return result;return result;return result;} } } }


The Current getCharge Methodclass Rentalclass Rentalclass Rentalclass Rental ............

doubledoubledoubledouble getChargegetChargegetChargegetCharge() {() {() {() {

double result = 0;double result = 0;double result = 0;double result = 0;

switch (switch (switch (switch (getMoviegetMoviegetMoviegetMovie().().().().getPriceCodegetPriceCodegetPriceCodegetPriceCode()) {()) {()) {()) {

case Movie.REGULAR:case Movie.REGULAR:case Movie.REGULAR:case Movie.REGULAR:

result += 2;result += 2;result += 2;result += 2;

if (if (if (if (getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() > 2)() > 2)() > 2)() > 2)

result += (result += (result += (result += (getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() () () () ---- 2) * 1.5;2) * 1.5;2) * 1.5;2) * 1.5;

break;break;break;break;

case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:

result +=result +=result +=result += getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() * 3;() * 3;() * 3;() * 3;


case Movie.CHILDRENS:case Movie.CHILDRENS:case Movie.CHILDRENS:case Movie.CHILDRENS:

result += 1.5;result += 1.5;result += 1.5;result += 1.5;

if (if (if (if (getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() > 3)() > 3)() > 3)() > 3)

result += (result += (result += (result += (getDaysRentedgetDaysRentedgetDaysRentedgetDaysRented() () () () ---- 3) * 1.5;3) * 1.5;3) * 1.5;3) * 1.5;


}}}}


}}}}


getCharge Moved to Movieclass Rental...class Rental...class Rental...class Rental...

double getCharge() {double getCharge() {double getCharge() {double getCharge() {return _movie.getCharge(_daysRented);return _movie.getCharge(_daysRented);return _movie.getCharge(_daysRented);return _movie.getCharge(_daysRented);

}}}}

class Movie … class Movie … class Movie … class Movie … doubledoubledoubledouble getChargegetChargegetChargegetCharge((((int daysRentedint daysRentedint daysRentedint daysRented) {) {) {) {

double result = 0;double result = 0;double result = 0;double result = 0;switch (switch (switch (switch (getPriceCodegetPriceCodegetPriceCodegetPriceCode()) {()) {()) {()) {

case Movie.REGULAR:case Movie.REGULAR:case Movie.REGULAR:case Movie.REGULAR:result += 2;result += 2;result += 2;result += 2;if (if (if (if (daysRenteddaysRenteddaysRenteddaysRented > 2)> 2)> 2)> 2)

result += (result += (result += (result += (daysRenteddaysRenteddaysRenteddaysRented ---- 2) * 1.5;2) * 1.5;2) * 1.5;2) * 1.5;break;break;break;break;

case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:result +=result +=result +=result += daysRenteddaysRenteddaysRenteddaysRented * 3;* 3;* 3;* 3;break;break;break;break;

case Movie.CHILDRENS:case Movie.CHILDRENS:case Movie.CHILDRENS:case Movie.CHILDRENS:result += 1.5;result += 1.5;result += 1.5;result += 1.5;if (if (if (if (daysRenteddaysRenteddaysRenteddaysRented > 3)> 3)> 3)> 3)

result += (result += (result += (result += (daysRenteddaysRenteddaysRenteddaysRented ---- 3) * 1.5;3) * 1.5;3) * 1.5;3) * 1.5;break;break;break;break;


}}}}

• Do the same with frequentRenterPoints()


Consider Inheritance

How’s This?

getCharge

Movie

getCharge

Regular Movie

getCharge

Childrens Movie

getCharge

New Release Movie


Using the State Pattern

getCharge

Price

getCharge

Regular Price

getCharge

Childrens Price

getCharge

New Release Price

getCharge

Movie

1

return price.getCharge


Replace Type Code With State/Strategy

ENGINEER : intSALESMAN : inttype : int

Employee

Employee Type

Engineer Salesman

Employee1

You have a type code which affects the behavior of a class but you cannot use subclassing.

Replace the type code with a state object.


Steps for Replace Type Code With State/Strategy• Create a new state class for the type code• Add subclasses of the state object, one for each

type code• Create an abstract query in the superclass to

return the type code. Override in subclasses to return correct type code

• Compile• Create field in old class for the state object• Change the type code query to delegate to the

state object• Change the type code setting methods to assign

an instance of the subclass• Compile and test


Price Codes on the Price Hierarchyabstract class Price {abstract class Price {abstract class Price {abstract class Price {

abstract int getPriceCode();abstract int getPriceCode();abstract int getPriceCode();abstract int getPriceCode();

}}}}

class ChildrensPrice extends Price {class ChildrensPrice extends Price {class ChildrensPrice extends Price {class ChildrensPrice extends Price {

int getPriceCode() {int getPriceCode() {int getPriceCode() {int getPriceCode() {

return Movie.CHILDRENS;return Movie.CHILDRENS;return Movie.CHILDRENS;return Movie.CHILDRENS;

}}}}

}}}}

class NewReleasePrice extends Price {class NewReleasePrice extends Price {class NewReleasePrice extends Price {class NewReleasePrice extends Price {


return Movie.NEW_RELEASE;return Movie.NEW_RELEASE;return Movie.NEW_RELEASE;return Movie.NEW_RELEASE;

}}}}

}}}}

class RegularPrice extends Price {class RegularPrice extends Price {class RegularPrice extends Price {class RegularPrice extends Price {


return Movie.REGULAR;return Movie.REGULAR;return Movie.REGULAR;return Movie.REGULAR;

}}}}

}}}}


Change Accessors on Moviepublic int getPriceCode() {public int getPriceCode() {public int getPriceCode() {public int getPriceCode() {

return _price.getPriceCode();return _price.getPriceCode();return _price.getPriceCode();return _price.getPriceCode();}}}}public void setPriceCodepublic void setPriceCodepublic void setPriceCodepublic void setPriceCode

(int arg) {(int arg) {(int arg) {(int arg) {switch (arg) {switch (arg) {switch (arg) {switch (arg) {

case REGULAR:case REGULAR:case REGULAR:case REGULAR:_price = new RegularPrice();_price = new RegularPrice();_price = new RegularPrice();_price = new RegularPrice();break;break;break;break;

case CHILDRENS:case CHILDRENS:case CHILDRENS:case CHILDRENS:_price = new ChildrensPrice();_price = new ChildrensPrice();_price = new ChildrensPrice();_price = new ChildrensPrice();break;break;break;break;

case NEW_RELEASE:case NEW_RELEASE:case NEW_RELEASE:case NEW_RELEASE:_price = new NewReleasePrice();_price = new NewReleasePrice();_price = new NewReleasePrice();_price = new NewReleasePrice();break;break;break;break;

default:default:default:default:throw new throw new throw new throw new

IllegalArgumentExceptionIllegalArgumentExceptionIllegalArgumentExceptionIllegalArgumentException("Incorrect Price Code");("Incorrect Price Code");("Incorrect Price Code");("Incorrect Price Code");

}}}}}}}}private Price _price;private Price _price;private Price _price;private Price _price;

public int public int public int public int getPriceCode() {getPriceCode() {getPriceCode() {getPriceCode() {

return _priceCode;return _priceCode;return _priceCode;return _priceCode;}}}}public setPriceCode public setPriceCode public setPriceCode public setPriceCode (int arg) {(int arg) {(int arg) {(int arg) {

_priceCode = arg;_priceCode = arg;_priceCode = arg;_priceCode = arg;}}}}private int private int private int private int _priceCode;_priceCode;_priceCode;_priceCode;


Replace Conditional With Polymorphsim

getSpeed

Bird

getSpeed

European

getSpeed

African

getSpeed

Norweigian Blue

��

��

��

��

��

��

��

��

��

��

��

��

double getSpeed() {switch (_type) {

case EUROPEAN:return getBaseSpeed();

case AFRICAN:return getBaseSpeed() - getLoadFactor() * _numberOfCoconuts;

case NORWEIGIAN_BLUE:return (_isNailed) ? 0 : getBaseSpeed(_voltage);

}throw new RuntimeException ("Should be unreachable");

}

You have a conditional that chooses different behavior depending on the type of an object

Move each leg of the conditional to an overriding method in a subclass. Make the original method abstract


Steps for Replace Conditional With Polymorphism• Move switch to superclass of

inheritance structure• Copy one leg of case statement

into subclass• Compile and test• Repeat for all other legs• Replace case statement

with abstract method


Move getCharge To Priceclass Movie…class Movie…class Movie…class Movie…double getCharge(int daysRented) {double getCharge(int daysRented) {double getCharge(int daysRented) {double getCharge(int daysRented) {

return _price.getCharge(daysRented);return _price.getCharge(daysRented);return _price.getCharge(daysRented);return _price.getCharge(daysRented);}}}}

class Price… class Price… class Price… class Price… double getCharge(int daysRented) {double getCharge(int daysRented) {double getCharge(int daysRented) {double getCharge(int daysRented) {

double result = 0;double result = 0;double result = 0;double result = 0;switch (getPriceCode()) {switch (getPriceCode()) {switch (getPriceCode()) {switch (getPriceCode()) {

case Movie.REGULAR:case Movie.REGULAR:case Movie.REGULAR:case Movie.REGULAR:result += 2;result += 2;result += 2;result += 2;if (daysRented > 2)if (daysRented > 2)if (daysRented > 2)if (daysRented > 2)

result += (daysRented result += (daysRented result += (daysRented result += (daysRented ---- 2) * 1.5;2) * 1.5;2) * 1.5;2) * 1.5;break;break;break;break;

case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:case Movie.NEW_RELEASE:result += daysRented * 3;result += daysRented * 3;result += daysRented * 3;result += daysRented * 3;break;break;break;break;

case Movie.CHILDRENS:case Movie.CHILDRENS:case Movie.CHILDRENS:case Movie.CHILDRENS:result += 1.5;result += 1.5;result += 1.5;result += 1.5;if (daysRented > 3)if (daysRented > 3)if (daysRented > 3)if (daysRented > 3)

result += (daysRented result += (daysRented result += (daysRented result += (daysRented ---- 3) * 1.5;3) * 1.5;3) * 1.5;3) * 1.5;break;break;break;break;


}}}}


Override getCharge in the Subclasses

Class RegularPrice…Class RegularPrice…Class RegularPrice…Class RegularPrice…double getCharge(int daysRented){double getCharge(int daysRented){double getCharge(int daysRented){double getCharge(int daysRented){

double result = 2;double result = 2;double result = 2;double result = 2;if (daysRented > 2)if (daysRented > 2)if (daysRented > 2)if (daysRented > 2)

result += (daysRented result += (daysRented result += (daysRented result += (daysRented ---- 2) * 1.5;2) * 1.5;2) * 1.5;2) * 1.5;return result;return result;return result;return result;

}}}}Class ChildrensPriceClass ChildrensPriceClass ChildrensPriceClass ChildrensPrice

double getCharge(int daysRented){double getCharge(int daysRented){double getCharge(int daysRented){double getCharge(int daysRented){double result = 1.5;double result = 1.5;double result = 1.5;double result = 1.5;if (daysRented > 3)if (daysRented > 3)if (daysRented > 3)if (daysRented > 3)

result += (daysRented result += (daysRented result += (daysRented result += (daysRented ---- 3) * 1.5;3) * 1.5;3) * 1.5;3) * 1.5;return result;return result;return result;return result;

}}}}

Class NewReleasePrice…Class NewReleasePrice…Class NewReleasePrice…Class NewReleasePrice…double getCharge(int daysRented){double getCharge(int daysRented){double getCharge(int daysRented){double getCharge(int daysRented){

return daysRented * 3;return daysRented * 3;return daysRented * 3;return daysRented * 3;}}}}

CCCClass Price…lass Price…lass Price…lass Price…

abstract double getCharge(int daysRented);abstract double getCharge(int daysRented);abstract double getCharge(int daysRented);abstract double getCharge(int daysRented);

Do each leg one at a time then...


Similar Statement Methodspublic String statement() {public String statement() {public String statement() {public String statement() {Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();String result = "Rental Record for " +String result = "Rental Record for " +String result = "Rental Record for " +String result = "Rental Record for " + getNamegetNamegetNamegetName() + "() + "() + "() + "\\\\n";n";n";n";while (rentals.while (rentals.while (rentals.while (rentals.hasMoreElementshasMoreElementshasMoreElementshasMoreElements()) {()) {()) {()) {

Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.nextElementnextElementnextElementnextElement();();();();result += "result += "result += "result += "\\\\t" + each.t" + each.t" + each.t" + each.getMoviegetMoviegetMoviegetMovie().().().().getTitlegetTitlegetTitlegetTitle()+ "()+ "()+ "()+ "\\\\t" + t" + t" + t" +

String.String.String.String.valueOfvalueOfvalueOfvalueOf(each.(each.(each.(each.getChargegetChargegetChargegetCharge()) + "()) + "()) + "()) + "\\\\n";n";n";n";}}}}

result += "Amount owed is " + String.result += "Amount owed is " + String.result += "Amount owed is " + String.result += "Amount owed is " + String.valueOfvalueOfvalueOfvalueOf((((getTotalChargegetTotalChargegetTotalChargegetTotalCharge()) + "()) + "()) + "()) + "\\\\n";n";n";n";result += "You earned " + String.result += "You earned " + String.result += "You earned " + String.result += "You earned " + String.valueOfvalueOfvalueOfvalueOf((((getTotalFrequentRenterPointsgetTotalFrequentRenterPointsgetTotalFrequentRenterPointsgetTotalFrequentRenterPoints()) + ()) + ()) + ()) +


}}}}

public Stringpublic Stringpublic Stringpublic String htmlStatementhtmlStatementhtmlStatementhtmlStatement() {() {() {() {Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();Enumeration rentals = _rentals.elements();String result = "<H1>Rentals for <EM>" +String result = "<H1>Rentals for <EM>" +String result = "<H1>Rentals for <EM>" +String result = "<H1>Rentals for <EM>" + getNamegetNamegetNamegetName() + "</EM></H1><P>() + "</EM></H1><P>() + "</EM></H1><P>() + "</EM></H1><P>\\\\n";n";n";n";while (rentals.while (rentals.while (rentals.while (rentals.hasMoreElementshasMoreElementshasMoreElementshasMoreElements()) {()) {()) {()) {

Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.Rental each = (Rental) rentals.nextElementnextElementnextElementnextElement();();();();result += each.result += each.result += each.result += each.getMoviegetMoviegetMoviegetMovie().().().().getTitlegetTitlegetTitlegetTitle()+ ": " +()+ ": " +()+ ": " +()+ ": " +String.String.String.String.valueOfvalueOfvalueOfvalueOf(each.(each.(each.(each.getChargegetChargegetChargegetCharge()) + "<BR>()) + "<BR>()) + "<BR>()) + "<BR>\\\\n";n";n";n";

}}}}result += "<P>You owe <EM>" + result += "<P>You owe <EM>" + result += "<P>You owe <EM>" + result += "<P>You owe <EM>" +

String.String.String.String.valueOfvalueOfvalueOfvalueOf((((getTotalChargegetTotalChargegetTotalChargegetTotalCharge()) + "</EM><P>()) + "</EM><P>()) + "</EM><P>()) + "</EM><P>\\\\n";n";n";n";result += "On this rental you earned <EM>" +result += "On this rental you earned <EM>" +result += "On this rental you earned <EM>" +result += "On this rental you earned <EM>" +

String.String.String.String.valueOfvalueOfvalueOfvalueOf((((getTotalFrequentRenterPointsgetTotalFrequentRenterPointsgetTotalFrequentRenterPointsgetTotalFrequentRenterPoints()) +()) +()) +()) +"</EM> frequent renter points<P>";"</EM> frequent renter points<P>";"</EM> frequent renter points<P>";"</EM> frequent renter points<P>";

return result;return result;return result;return result;} } } }


Form Template Method

You have two methods in subclasses that carry out similar steps in the same

order, yet the steps are different

Give each step into methods with the same signature, so that the original

methods become the same. Then you can pull them up.


Steps for Form Template Method• Take two methods with similar overall structure

but varying pieces– Use subclasses of current class, or create a strategy

and move the methods to the strategy • At each point of variation extract methods from

each source with the the same signature but different body

• Declare signature of extracted method in superclass and place varying bodies in subclasses

• When all points of variation have been removed, move one source method to superclass and remove the other


Create a Statement Strategyclass Customer …class Customer …class Customer …class Customer …

public String statement() {public String statement() {public String statement() {public String statement() {

return newreturn newreturn newreturn new TextStatementTextStatementTextStatementTextStatement().value(this);().value(this);().value(this);().value(this);

}}}}

class class class class TextStatementTextStatementTextStatementTextStatement {{{{

public String value(Customerpublic String value(Customerpublic String value(Customerpublic String value(Customer aCustomeraCustomeraCustomeraCustomer) {) {) {) {

Enumeration rentals =Enumeration rentals =Enumeration rentals =Enumeration rentals = aCustomeraCustomeraCustomeraCustomer....getRentalsgetRentalsgetRentalsgetRentals();();();();

String result = "Rental Record for " +String result = "Rental Record for " +String result = "Rental Record for " +String result = "Rental Record for " + aCustomeraCustomeraCustomeraCustomer....getNamegetNamegetNamegetName() + "() + "() + "() + "\\\\n";n";n";n";

while (rentals.while (rentals.while (rentals.while (rentals.hasMoreElementshasMoreElementshasMoreElementshasMoreElements()) {()) {()) {()) {


result += "result += "result += "result += "\\\\t" + each.t" + each.t" + each.t" + each.getMoviegetMoviegetMoviegetMovie().().().().getTitlegetTitlegetTitlegetTitle()+ "()+ "()+ "()+ "\\\\t" + t" + t" + t" +

String.String.String.String.valueOfvalueOfvalueOfvalueOf(each.(each.(each.(each.getChargegetChargegetChargegetCharge()) + "()) + "()) + "()) + "\\\\n";n";n";n";

}}}}

result += "Amount owed is " + result += "Amount owed is " + result += "Amount owed is " + result += "Amount owed is " +

String.String.String.String.valueOfvalueOfvalueOfvalueOf((((aCustomeraCustomeraCustomeraCustomer....getTotalChargegetTotalChargegetTotalChargegetTotalCharge()) + "()) + "()) + "()) + "\\\\n";n";n";n";

result += "You earned " + result += "You earned " + result += "You earned " + result += "You earned " +

String.String.String.String.valueOfvalueOfvalueOfvalueOf((((aCustomeraCustomeraCustomeraCustomer....getTotalFrequentRenterPointsgetTotalFrequentRenterPointsgetTotalFrequentRenterPointsgetTotalFrequentRenterPoints()) + ()) + ()) + ()) +

" frequent renter points";" frequent renter points";" frequent renter points";" frequent renter points";


}}}}

• Do the same with htmlStatement()


Extract Differencesclassclassclassclass TextStatementTextStatementTextStatementTextStatement............

public String value(Customerpublic String value(Customerpublic String value(Customerpublic String value(Customer aCustomeraCustomeraCustomeraCustomer) {) {) {) {

Enumeration rentals =Enumeration rentals =Enumeration rentals =Enumeration rentals = aCustomeraCustomeraCustomeraCustomer....getRentalsgetRentalsgetRentalsgetRentals();();();();

String result =String result =String result =String result = headerStringheaderStringheaderStringheaderString((((aCustomeraCustomeraCustomeraCustomer););););

while (rentals.while (rentals.while (rentals.while (rentals.hasMoreElementshasMoreElementshasMoreElementshasMoreElements()) {()) {()) {()) {


result += "result += "result += "result += "\\\\t" + each.t" + each.t" + each.t" + each.getMoviegetMoviegetMoviegetMovie().().().().getTitlegetTitlegetTitlegetTitle()+ "()+ "()+ "()+ "\\\\t" + t" + t" + t" +

String.String.String.String.valueOfvalueOfvalueOfvalueOf(each.(each.(each.(each.getChargegetChargegetChargegetCharge()) + "()) + "()) + "()) + "\\\\n";n";n";n";

}}}}

result += "Amount owed is " + result += "Amount owed is " + result += "Amount owed is " + result += "Amount owed is " +

String.String.String.String.valueOfvalueOfvalueOfvalueOf((((aCustomeraCustomeraCustomeraCustomer....getTotalChargegetTotalChargegetTotalChargegetTotalCharge()) + "()) + "()) + "()) + "\\\\n";n";n";n";

result += "You earned " + result += "You earned " + result += "You earned " + result += "You earned " +

String.String.String.String.valueOfvalueOfvalueOfvalueOf((((aCustomeraCustomeraCustomeraCustomer....getTotalFrequentRenterPointsgetTotalFrequentRenterPointsgetTotalFrequentRenterPointsgetTotalFrequentRenterPoints()) + ()) + ()) + ()) +



}}}}

StringStringStringString headerStringheaderStringheaderStringheaderString(Customer(Customer(Customer(Customer aCustomeraCustomeraCustomeraCustomer) {) {) {) {

return "Rental Record for " +return "Rental Record for " +return "Rental Record for " +return "Rental Record for " + aCustomeraCustomeraCustomeraCustomer....getNamegetNamegetNamegetName() + "() + "() + "() + "\\\\n";n";n";n";

}}}}

class class class class HtmlStatementHtmlStatementHtmlStatementHtmlStatement............StringStringStringString headerStringheaderStringheaderStringheaderString(Customer(Customer(Customer(Customer aCustomeraCustomeraCustomeraCustomer) {) {) {) {

return "<H1>Rentals for <EM>" +return "<H1>Rentals for <EM>" +return "<H1>Rentals for <EM>" +return "<H1>Rentals for <EM>" + aCustomeraCustomeraCustomeraCustomer....getNamegetNamegetNamegetName() + "</EM></H1><P>() + "</EM></H1><P>() + "</EM></H1><P>() + "</EM></H1><P>\\\\n";n";n";n";}}}}


Continue Extractingclass TextStatement …class TextStatement …class TextStatement …class TextStatement …

public String value(Customer aCustomer) {public String value(Customer aCustomer) {public String value(Customer aCustomer) {public String value(Customer aCustomer) {

Enumeration rentals = aCustomer.getRentals();Enumeration rentals = aCustomer.getRentals();Enumeration rentals = aCustomer.getRentals();Enumeration rentals = aCustomer.getRentals();

String result = headerString(aCustomer);String result = headerString(aCustomer);String result = headerString(aCustomer);String result = headerString(aCustomer);

while (rentals.hasMoreElements()) {while (rentals.hasMoreElements()) {while (rentals.hasMoreElements()) {while (rentals.hasMoreElements()) {

Rental each = (Rental) rentals.nextElement();Rental each = (Rental) rentals.nextElement();Rental each = (Rental) rentals.nextElement();Rental each = (Rental) rentals.nextElement();

result += eachRentalString(each);result += eachRentalString(each);result += eachRentalString(each);result += eachRentalString(each);

}}}}

result += footerString(aCustomer);result += footerString(aCustomer);result += footerString(aCustomer);result += footerString(aCustomer);


}}}}

String eachRentalString (Rental aRental) {String eachRentalString (Rental aRental) {String eachRentalString (Rental aRental) {String eachRentalString (Rental aRental) {

return "return "return "return "\\\\t" + aRental.getMovie().getTitle()+ "t" + aRental.getMovie().getTitle()+ "t" + aRental.getMovie().getTitle()+ "t" + aRental.getMovie().getTitle()+ "\\\\t" + t" + t" + t" +

String.valueOf(aRental.getCharge()) + "String.valueOf(aRental.getCharge()) + "String.valueOf(aRental.getCharge()) + "String.valueOf(aRental.getCharge()) + "\\\\n";n";n";n";

}}}}

String footerString (Customer aCustomer) {String footerString (Customer aCustomer) {String footerString (Customer aCustomer) {String footerString (Customer aCustomer) {

return "Amount owed is " + return "Amount owed is " + return "Amount owed is " + return "Amount owed is " +

String.valueOf(aCustomer.getTotalCharge()) + "String.valueOf(aCustomer.getTotalCharge()) + "String.valueOf(aCustomer.getTotalCharge()) + "String.valueOf(aCustomer.getTotalCharge()) + "\\\\n" +n" +n" +n" +

"You earned " + "You earned " + "You earned " + "You earned " +

String.valueOf(aCustomer.getTotalFrequentRenterPoints()) + String.valueOf(aCustomer.getTotalFrequentRenterPoints()) + String.valueOf(aCustomer.getTotalFrequentRenterPoints()) + String.valueOf(aCustomer.getTotalFrequentRenterPoints()) +


}}}}


Pull Up the Value Method

class Statement...class Statement...class Statement...class Statement...

public String value(Customer aCustomer) {public String value(Customer aCustomer) {public String value(Customer aCustomer) {public String value(Customer aCustomer) {

Enumeration rentals = aCustomer.getRentals();Enumeration rentals = aCustomer.getRentals();Enumeration rentals = aCustomer.getRentals();Enumeration rentals = aCustomer.getRentals();

String result = headerString(aCustomer);String result = headerString(aCustomer);String result = headerString(aCustomer);String result = headerString(aCustomer);

while (rentals.hasMoreElements()) {while (rentals.hasMoreElements()) {while (rentals.hasMoreElements()) {while (rentals.hasMoreElements()) {

Rental each = (Rental) rentals.nextElement();Rental each = (Rental) rentals.nextElement();Rental each = (Rental) rentals.nextElement();Rental each = (Rental) rentals.nextElement();

result += eachRentalString(each);result += eachRentalString(each);result += eachRentalString(each);result += eachRentalString(each);

}}}}

result += footerString(aCustomer);result += footerString(aCustomer);result += footerString(aCustomer);result += footerString(aCustomer);


}}}}

abstract String headerString(Customer aCustomer);abstract String headerString(Customer aCustomer);abstract String headerString(Customer aCustomer);abstract String headerString(Customer aCustomer);

abstract String eachRentalString (Rental aRental);abstract String eachRentalString (Rental aRental);abstract String eachRentalString (Rental aRental);abstract String eachRentalString (Rental aRental);

abstract String footerString (Customer aCustomer);abstract String footerString (Customer aCustomer);abstract String footerString (Customer aCustomer);abstract String footerString (Customer aCustomer);


State of Classes

statement()htmlStatement()

Customervalue(Customer)headerString(Customer)eachRentalString(Rental)footerString(Customer)

Statement

headerString(Customer)eachRentalString(Rental)footerString(Customer)

Text Statement

headerString(Customer)eachRentalString(Rental)footerString(Customer)

Html Statement


In This Example• We saw a poorly factored program improved

– Easier to add new services on customer– Easier to add new types of movie

• No debugging during refactoring– Appropriate steps reduce chance of bugs – Small steps make bugs easy to find

• Illustrated several refactorings– Extract Method– Move Method– Replace Temp with Query– Replace Type Code with State/Strategy– Replace Switch with Polymorphism– Form Template Method


Definitions of Refactoring

• Loose Usage– Reorganize a program (or something)

• As a noun– A change made to the internal structure of some

software to make it easier to understand and cheaper to modify, without changing the observable behavior of that software

• As a verb– The activity of restructuring software by applying a

series of refactorings without changing the observable behavior of that software.


Where Refactoring Came From• Ward Cunningham and Kent Beck

–Smalltalk style• Ralph Johnson at University of Illinois at

Urbana-Champaign• Bill Opdyke’s Thesis

ftp://st.cs.uiuc.edu/pub/papers/refactoring/opdyke-thesis.ps.Z

• John Brant and Don Roberts: The Refactoring Browser


Refactoring Tools• Based on provable transformations

– Build parse tree of programs– Mathematic proof that refactoring does not

change semantics– Embed refactoring in tool

• Speeds up refactoring– Extract method: select code, type in method name– No need for tests (unless dynamic reflection)– Big speed improvement

• Not Science Fiction– Available for Smalltalkhttp://st-www.cs.uiuc.edu/~brant/RefactoringBrowser


The Importance of Tests

• Even with a tool, testing is important– Not all refactorings can be proven

• Write tests as you write the code• Make the test self-checking

– Return “OK” if good, errors if not• Run a suite with a single command• Test with every compile

www.xprogramming.com/software


The Two Hats

Adding Function

• Add new capabilities to the system

• Adds new tests• Get the test working

Refactoring

• Does not add anynew features

• Does not add tests (but may change some)

• Restructure the code to remove redundancy

Swap frequently between the hats, but only wear one at a time


Why Refactor

• To improve the software design– Combat’s “bit rot”– Makes the program easier to change

• To make the software easier to understand– Write for people, not the compiler– Understand unfamiliar code

• To help find bugs– Refactor while debugging to clarify the code

Refactoring helps you program faster!


When Should You Refactor?

• To add new functionality– Refactor existing code until you understand it– Refactor the design to make it easy to add

• To find bugs– Refactor to understand the code

• For code reviews– Rmmediate effect of code review– Allows for higher level suggestions

Don’t set aside time for refactoring, include it in your normal activities


What Do You Tell Your Manager

• If the manager is really concerned about quality– Then stress the quality aspects

• Otherwise you need to develop as fast as possible– You’re the professional, so you know to do

what makes you go faster


Problems With Refactoring

• We don’t know what they are yet• Database Migration

– Insulate persistent database structure from your objects

– With OO databases, migrate frequently• Published Interfaces

– Publish only when you need to– Don’t publish within a development team

• Without working tests– Don’t bother


Design Decisions

• In the moment– Consider current needs– Patch code when new needs appear

• Planned design– Consider current needs and possible future needs– Design to minimize change with future needs– Patch code if unforeseen need appears

• Evolutionary design– Consider current needs and possible future needs– Trade off cost of current flexibility versus cost of later

refactoring– Refactor as changes appear


Extreme Programming

• Methodology developed by Kent Beck• Designed to adapt to changes• Key Practices

– Iterative Development– Self Testing Code– Refactoring– Pair Programming

• Leverages refactoring to encourage evolutionary design

Beck, K. Extreme Programming Explained, Addison-Wesley


Team Techniques

• Encourage refactoring culture– Nobody gets things right first time– Nobody can write clear code without reviews– Refactoring is forward progress

• Provide sound testing base– Tests are essential for refactoring– Build system and run tests daily

• Pair Programming– Two programmers working together can be quicker

than working separately– Refactor with the class writer and a class user


Creating Your Own Refactorings• Consider a change to a program• Should it change the external behavior

of the system• Break down the change into small steps

– Look for points where you can compile and test• Carry out the change, note what you do

– If a problem occurs, consider how to eliminate it in future

• Carry it out again, follow and refine the notes• After two or three times you have a workable

refactoring


Final Thoughts

• The one benefit of objects is that they make it easier to change.

• Refactoring allows you to improve the design after the code is written

• Up front design is still important, but not so critical

• Refactoring is an immature subject: not much written and very few tools

refactoring: improving the design of existing...

Documents