refactoring this lecture is divided into and introduction to refactoring and then several lessons....

Refactoring

This lecture is divided into and introduction to refactoring and then several lessons.

The intent is not to teach you all the refactorings but to expose you to just a few…

Refactoring

Introduction Outline

• A. What is Refactoring?

• B. Why do we Refactor?

• C. How do we Refactor?

• D. When do we Refactor?

Refactoring

A. What is Refactoring?

Refactoring is a technique which identifies bad code (code that smells) and allows promotes the re-structuring of that bad code into classes and methods that are more readable, maintainable, and generally sparse in code. Refactoring yields a “better” design of both your classes and methods.

Refactoring

B. Why do we Refactor?

Refactoring allows building of complex systems by exploiting sophisticated object-oriented techniques that yield such systems catagorized as frameworks, beans, or other reusable software components.

Refactoring

Why do you refactor

Enable sharing of logic.

Explain intention and implementation seperately.

Isolate change.

Encode conditional logic.

Refactoring

C. How do we Refactor?

Refactoring is accomplished by

1) building, decomposing, and moving methods2) building and decomposing classes3) replacing code with patterns, and 4) replacing code with other techniques.

Refactoring

D. When do we Refactor?

Refactoring is done when

1) methods or classes are too LARGE, 2) code, control flow, or data structures are DUPLICATED, 3) attributes or methods are MISPLACED,4) When parameters can make code reusable5) When design is poor (the code smells).

Refactoring

When do you refactorRefactor when you add functions.

Refactor as you do a code review.

Refactor when you fix a bug.

Refactoring

Benefits of RefactoringWithout refactoring, the design of the program

decays.

Refactoring helps you to develop code more quickly.

Refactoring helps you to find bugs.

Refactoring makes software easier to understand.

Refactoring improves the design of software.

Refactoring

An example- Class Diagram

Movie

getPriceCode)_setPriceCode()getTitle()

pricecode: intchildren = 2regular = 0new_release=1title: String

Rental

getMovie()getDaysRented)_

daysRented:int

Customer

Statement()addRental(rental)getName():

name: Stringrentals: vector

0..*

1..1 0..*

1..1

Refactoring

MovieRentalCustomerstatement

forallrentals

Rental: getMovie()

getPriceCode()

getDaysRented()

An Example – Sequence Diagram

Refactoring

public class Movie { public static final int CHILDRENS = 2; // type movie for children public static final int REGULAR = 0; // type of regular movie public static final int NEW_RELEASE = 1; // new release movie

private String _title; // title of the movie private int _priceCode; // price code for movie

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

}// end constructor

public int getPriceCode() { return priceCode; } public void setPriceCode (int agr) { _priceCode = arg; } public String getTitle () { return _Title; }

} // end Movie

Code -- page 3

Constructor

Movie



Refactoring

class Rental { private Movie _movie; // movie rented private int _daysRented; // number of days rental public Rental(Movie movie, int daysRented) { _movie = movie; _daysREnted = daysRented; } // end Rental

public int getDaysRented() { return _daysRented; } public Movie getMovie() { return _movie; }

}// end Rental

Code (con’t)-- page 3

Constructor

Rental


daysRented:int

Refactoring

class Customer { private String _name; // name of customer private Vector _rentals = new Vector (); // vector of list of

rentals by the customer public Customer (String name) { _name = name; }

public void addRental (Rental arg) { _rentals.addElement(arg) }

public String getName () { return _name}


Constructor

Customer



Refactoring

public String statement() {

double totalAmount = 0; // total of the statement int frequentRenterPoints = 0; // number of frequent

rental points of a rental Enumeration rentals = _rentals.elements(); // list of

rentals String result = “Rental Record for “ + getName()

+”/n”

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

// continued on next page


Refactoring

// determine amounts for each line // regular 2.00 for two days 1.50 extra

// new release 3.00 per day // children 1.50 for three days

switch (each.getMovie().getPriceCode()) {

case Movie.REGULAR: // statements for a regular moviethisAmount +=2;if (each.getDaysRented() >2)

thisAmount +=(each.getDaysRented()-2)*1.5;Break;

case Movie.NEW_RELEASE: // statements for a new release type movie

thisAmount +=each.getDaysRented()*3;Break;

case Movie_CHILDREN: // statements for a children moviethisAmount +=1.5;if (each.getDaysRented() >3) thisAmount

+=(each.getDaysRented()-3)*1.5;Break;

} // end switch

Code (Con’t) page 5

Refactoring // add frequent renter points add 1 frequent renter point if NEW RELEASE rented > one day frequentRenterPoints ++;

// add bonus for a two day new release rental if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) &&

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

// show figures for this rental result +=“/t” + each.getMovie().getTitle*(+”\t” +

String.valueOf(thisAmount) + “\n”; totalAmount +=thisAmount; }

// add footer linesresult +=“Amount owed is “ + String.valueOf(totalAmount) + “\

n”;result += “You earned “ + String.valueOf(frequentRenterPoints) +

“ frequent renter points”;return result; } // end statement// end customer

Code (Con’t) page 5

Refactoring

• This method, called statement, is TOO LARGE.

• This statement method should not be inside customer.

RefactoringRefactoring Opportunities

// determine amounts for each line

switch (each.getMovie().getPriceCode()) { case Movie.REGULAR:

thisAmount +=2;if (each.getDaysRented() >2) thisAmount


case Movie.NEW_RELEASE:thisAmount +=each.getDaysRented()*3;Break;

case Movie_CHILDREN:thisAmount +=1.5;if (each.getDaysRented() >3) thisAmount


} // end switch

a code cluster is setting one variableEXTRACT it as a METHOD

returning the variable

RefactoringEXTRACT METHOD page 11

private double amountFor(Rental each)

double thisAmount = 0.0; switch (each.getMovie().getPriceCode()) {

case Movie.REGULAR: thisAmount +=2;if (each.getDaysRented() >2) thisAmount


case Movie.NEW_RELEASE:thisAmount +=each.getDaysRented()*3;Break;

case Movie_CHILDREN:thisAmount +=1.5;if (each.getDaysRented() >3) thisAmount


} // end switch return thisAmount; } // end amountFor

Refactoring

Original Code - Customer Class page 18

Calls the new method

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

+”/n” while (rentals.hasMoreElements() { double thisAmount =0; Rental each = (Rental) rentals.nextElement();

thisAmount = amountFor(each);

Refactoring

Refactoring Opportunities

Should NOT be in customer classShould be in the rental



thisAmount = amountFor(each);

a variable resides in the wrong classMOVE the METHOD

to the class where it should reside

RefactoringMOVE METHOD and rename

private double getCharge(Rental each)

double result = 0.0; switch (each.getMovie().getPriceCode()) { case Movie.REGULAR:

result +=2;if (each.getDaysRented() >2) result


case Movie.NEW_RELEASE:result +=each.getDaysRented()*3;Break;

case Movie_CHILDREN:result +=1.5;if (each.getDaysRented() >3) result


} // end switch return result; } // end getCharge

Rename the method and result

Refactoring

Original Code - Customer Class page 19

Calls the new method In the rental class



thisAmount = each.getCharge(each);

Refactoring


forallrentals

getPriceCode()

getDaysRented()


Rental: getMovie()amount: getCharge()

Refactoring

// add frequent renter points add 1 frequent renter point if NEW RELEASE rented > one day

frequentRenterPoints ++; // add bonus for a two day new release rental if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) &&

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

// show figures for this rental result +=“/t” + each.getMovie().getTitle*(+”\t” +

String.valueOf(thisAmount) + “\n”; totalAmount +=thisAmount; }

// add footer linesresult +=“Amount owed is “ + String.valueOf(totalAmount) + “\n”;result += “You earned “ + String.valueOf(frequentRenterPoints) + “ frequent renter

points”;return result; } // end statement

Refactoring Opportunitiesa code cluster is setting one variable

EXTRACT it as a METHODreturning the variable

}

Refactoring

// add frequent renter points add 1 frequent renter point if NEW RELEASE rented > one day

int getFrequentRenterPoints() { if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) &&

each.getDaysRented() > 1) return 2; else return 1;

EXTRACT METHOD

RefactoringRefactoring Opportunities

Should NOT be in customer classShould be in the rental

public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = “Rental Record for “ + getName() +”/n” while (rentals.hasMoreElements() { double thisAmount =0; Rental each = (Rental) rentals.nextElement();

frequentRenterPoints += getFrequentRenterPoints();

a variable resides in the wrong classMOVE the METHOD

to the class where it should reside

Refactoring

class Rental…..….. // add frequent renter points add 1 frequent renter point if NEW

RELEASE rented > one day int getFrequentRenterPoints() {

if ((each.getMovie().getPriceCode() == Movie.NEW_RELEASE) &&

each.getDaysRented() > 1) return 2; else return 1;

MOVE METHOD

RefactoringOriginal Code - Customer Class

Calls the new methods In the rental class

public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = “Rental Record for “ + getName() +”/n” while (rentals.hasMoreElements() { double thisAmount =0; Rental each = (Rental) rentals.nextElement(); thisAmount = each.getCharge();

frequentRenterPoints += each.getFrequentRenterPoints();

Refactoring

public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = “Rental Record for “ + getName() +”/n” while (rentals.hasMoreElements() { double thisAmount =0; Rental each = (Rental) rentals.nextElement(); thisAmount = each.getCharge(); frequentRenterPoints += getFrequentRenterPoints(); // show figures for this rental result +=“/t” + each.getMovie().getTitle*(+”\t” +

String.valueOf(thisAmount) + “\n”; totalAmount +=this.Amount; } // end while

Refactoring Opportunitiesa variable is used temporarily

REPLACE TEMP with QUERYeliminating the temp

RefactoringREPLACE TEMP with QUERY page 21

public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = “Rental Record for “ + getName() +”/n” while (rentals.hasMoreElements() { double thisAmount =0; Rental each = (Rental) rentals.nextElement(); thisAmount = each.getCharge(); frequentRenterPoints += getFrequentRenterPoints(); // show figures for this rental result +=“/t” + each.getMovie().getTitle*(+”\t” +

String.valueOf(thisAmount each.getCharge()) + “\n”; totalAmount += this.Amount each.getCharge() ; } // end while

Refactoring

public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = “Rental Record for “ + getName() +”/n” while (rentals.hasMoreElements() { Rental each = (Rental) rentals.nextElement(); frequentRenterPoints += getFrequentRenterPoints(); // show figures for this rental result +=“/t” + each.getMovie().getTitle*(+”\t” +

String.valueOf(each.getCharge()) + “\n”; totalAmount += each.getCharge(); } // end loop// add footer linesresult +=“Amount owed is “ + String.valueOf(totalAmount) + “\n”;result += “You earned “ + String.valueOf(frequentRenterPoints) + “

frequent renter points”;return result; } // end statement



Problem – temp in loop

Refactoring

private double getTotalCharge() { double result = 0; Enumeration rentals = _rentals.elements(); while (rentals.hasMoreElements() { Rental each = (Rental) rentals.nextElement(); result += each.getCharge(); } // end loop return result;} // end getTotalCharge

EXTRACT it as a METHOD

Refactoring

public String statement() { double totalAmount = 0; int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = “Rental Record for “ + getName() +”/n” while (rentals.hasMoreElements() { Rental each = (Rental) rentals.nextElement(); frequentRenterPoints += getFrequentRenterPoints();// show figures for this rental result +=“/t” + each.getMovie().getTitle*(+”\t” +

String.valueOf(each.getCharge()) + “\n”; totalAmount += each.getCharge(); }// add footer linesresult +=“Amount owed is “ + String.valueOf( totalAmount getTotalCharge()) + “\

n”;result += “You earned “ + String.valueOf(frequentRenterPoints) + “ frequent renter


REPLACE TEMP with QUERY page 27

Yes, we are looping twice

Refactoring

public String statement() { int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = “Rental Record for “ + getName() +”/n” while (rentals.hasMoreElements() { Rental each = (Rental) rentals.nextElement(); frequentRenterPoints += getFrequentRenterPoints(); // show figures for this rental result +=“/t” + each.getMovie().getTitle*(+”\t” +

String.valueOf(each.getCharge()) + “\n”; } // end loop// add footer linesresult +=“Amount owed is “ + String.valueOf(getTotalCharge) + “\n”;result += “You earned “ + String.valueOf(frequentRenterPoints) + “ frequent renter




Problem – temp in loop

Refactoring

private double getTotalFrequentRentalPoints() { int result = 0; Enumeration rentals = _rentals.elements(); while (rentals.hasMoreElements() { Rental each = (Rental) rentals.nextElement(); result += each.getFrequentRentalPoints(); } // end loop return result;} // end getTotalFrequentRentalPoints

EXTRACT it as a METHOD

Refactoring

public String statement() { int frequentRenterPoints = 0; Enumeration rentals = _rentals.elements(); String result = “Rental Record for “ + getName() +”/n” while (rentals.hasMoreElements() { Rental each = (Rental) rentals.nextElement(); frequentRenterPoints += getFrequentRenterPoints();// show figures for this rental result +=“/t” + each.getMovie().getTitle*(+”\t” +

String.valueOf(each.getCharge()) + “\n”; }// add footer linesresult +=“Amount owed is “ + String.valueOf( getTotalCharge()) + “\n”;result += “You earned “ + String.valueOf(frequentRenterPoints

getTotalFrequentRentalPoints ) + “ frequent renter points”;return result; } // end statement

REPLACE TEMP with QUERY page 29

Yes, we are looping thrice

Refactoring


getTotalCharge

amount: getCharge()getPriceCode()

getFrequentRenterPoints()


getPriceCode()

Refactoring

Refactoring Opportunitiesconditional code exist for sub-types

Use POLYMORPHISMreplacing conditional logic

private double getCharge(Rental each)double result = 0.0;

switch (getMovie().getPriceCode()) { case Movie.REGULAR:

result +=2;if (getDaysRented() >2) result +=(getDaysRented()-2)*1.5;Break;

case Movie.NEW_RELEASE:result +=getDaysRented()*3;Break;

case Movie_CHILDREN:result +=1.5;if (getDaysRented() >3) result +=(getDaysRented()-3)*1.5;Break;


Cannot make subclasses of movie Movies can change classifications

Refactoring

Refactoring Opportunitiesobject can change states in its lifetime

Use the STATE Patternfor the variables which change values

private double getCharge(Rental each)double result = 0.0;







Refactoring

Original- Class Diagram

Movie



Rental


daysRented:int

Customer



0..*

1..1 0..*

1..1

Refactoring

State Pattern- Class Diagram

Movie

getPriceCode)_setPriceCode()getTitle()getCharge()


Price

getCharge()

1..1

1..1

Regular Price

getCharge()

Childrens Price

getCharge()

New Release Price

getCharge()

Refactoring

Refactoring Opportunitiesstate variables not encapsulated

SELF ENCAPSULATE FIELDSadd get and set methods

private double getCharge(int daysRented)double result = 0.0;







Refactoring

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

private String _title; private int _priceCode;


}// end constructor

public int getPriceCode() { return priceCode; }

public void setPriceCode (int agr) { _priceCode = arg; }

public String getTitle () { return _Title; }

} // end Movie

Original Code -- page 3

} Sub-types

Self Encapsulate

Refactoring

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

private String _title; private int _priceCode; setPriceCode(priceCode);


}// end constructor

public int getPriceCode() { return priceCode; }

public void setPriceCode (int agr) { _priceCode = arg; }

public String getTitle () { return _Title; }

} // end Movie

Self Encapsulating Movie Code -- page 41

Refactoring

class Price { abstract int getPriceCode(); } // end Price

class ChildrensPrice extends Price { int getPriceCode () { return Movie.CHILDRENS; }} // end ChildrensPrice

class NewReleasePrice extends Price { int getPriceCode () { return Movie.NEW_RELEASE; }} // end NewReleasePrice

class RegularPrice extends Price { int getPriceCode () { return Movie.REGULAR; }} // end RegularPrice

Movie Price Sub-Classes -- page 41

Refactoring

MOVE METHOD page 45

class Price private double getCharge(int daysRented)

double result = 0.0; switch (getMovie().getPriceCode()) { case Movie.REGULAR:




} // end switch return result; } // end getCharge}// end price

Once it is moved, we can replace conditional with polymorphism

Refactoring

Use POLYMORPHISM page 47

class RegularPrice double getCharge(int daysRented)

double result = 2;if (getDaysRented() >2) result +=(getDaysRented()-2)*1.5;

return result; } // end getCharge} // end regularPrice

class ChildrensPrice double getCharge (int daysRented) { double result = 1.5;

if (getDaysRented() >3) result +=(getDaysRented()-3)*1.5;return result

} // end getCharge} // end ChildrensPrice

class NewRelease double getCharge (int daysRented() { return daysRented * 3; }

Take one leg of case statement at a time.

Refactoring


class Price abstract double getCharge (int daysRented);

Create an overiding method for the getCharge method.

Refactoring


class Movie….. int getFrequentRenterPoints (int daysRented) { if ((getPriceCode() == Movie.NEW_RELEASE) && daysRented >1) return 2; else return 1; }

We can do the same thing with getFrequentRenterPoints

Refactoring

Final Thoughts page 52

Placing a state pattern in code is quite an effortThe gain is that if I

change any of the price’s behavioradd new pricesadd extra price dependent behavior

The rest of the application does not know about the use of the state pattern.For this tiny bit of behavior, it does not seem worthwhile.

These changes lead to better distributed responsibilities and code1that is easier to maintain.

It does look different than the regular procedural code.

One important lesson in this example is the rhythm of refactgoring, change a littletest a little

Refactoring

We will look at a catalogue of refactoring methods.

Each refactoring method is described by the following:

name : noun (with a page number of the text)

summary : narrative description

motivation : why you would use the technique

example : code using the technique

mechanics : how you would use the technique

Catalogue of Refactoring

Refactoring

We will look at several types of refactoring. These include the refactoring of:

methods generalization

classes data

calls conditional expressions

And some other BIG refactoring.

Refactoring

Extract Methods from code

Inline Methods to code

Replace Temp with Query

Introduce Explaining Variables

Split Temporary Variables

Remove Assignments to Parameters

Replace Method with Method Objects

Substitute Algorithm

Refactoring and Composing Methods

Refactoring

Move Method

Move Field

Extract Class

Inline Class

Hide Delegate

Remove Middle Man

Introduce Foreign Method

Introduce Local Extension

Refactoring by Moving Features Between Objects

Refactoring

Self Encapsulate Field Encapsulate Field

Replace Data Value with Object Encapsulate Collection

Change Value to Reference Replace Record with Data Class

Change Reference to Value Replace Type with Data Class

Replace Array with Object Replace Type Code with Subclasss

Duplicate Observed Data Replace Type Code with State/Strategy

Change Unidirectional Direction to Bidirectional Replace Subclass

Change Bidirectional Direction to Unidirectional with Field

Replace Magic Number with Symbolic Constant

Refactoring by Organizing Data

Refactoring

Decompose Conditional

Consolidate Conditional Expression

Consolidate Duplicate Conditional Fragments

Remove Control Flag

Replace nested Conditional with Guard Clauses

Replace Conditional with Polymorphism

Introduce Null Object

Introduce Assertion

Refactoring by Simplifying Conditional Expressions

Refactoring

Rename Method Introduce Parameter Object

Add Parameter Remove Setting Method

Remove Parameter Hide Method

Separate Query from Modifier Replace Constructor with Factory

Parameterize Method Encapsulate Downcast

Replace Parameter with Explicit Methods

Preserve Whole Object Replace Error Code with Exception

Replace Parameter with Method Replace Exception with Test

Refactoring by Making Method Calls Simpler

Refactoring

Pull Up Field Extract Interface

Pull Up Method Collapse Hierarchy

Pull Up Constructor Body Form Template Method

Push Down Method Replace Inheritance with

Push Down Field Delegation

Extract Subclass Replace Delegation with

Extract Superclass Inheritance

Refactoring by Dealing with Generalization

Refactoring

Tease Apart Inheritance

Convert Procedural Design to Objects

Separate Domain from Presentation

Extract Hierarchy

Refactoring with Big Refactoring

Refactoring

It if difficult to know how to approach refactoring the code you have written. And unfortunately, we have not progressed in teaching programming to notice needed refactorings.

You can approach it using a few simple guidelines.

Because there are SOOOO many factorings

Refactoring

First: Make your code Self-DocumentingSecond: Encapsulate your classesThird: Assure Constants and Variables are coded correctlyFourth: Make sure you have GOOD MethodsFifth: Make sure your Conditionals are coded correctlySixth: Assure your Classes are coded correctlySeventh: Assure proper InheritanceEighth: Apply needed Patterns

Refactoring Topics

Refactoring

import java.awt.*;import java.awt.event.*;import java.awt.image.*;import java.net.*;import java.applet.*;

/** * A Simple TicTacToe applet. * A Tic Tac Toe applet. * A very simple, and mostly brain-dead * implementation of your favorite game! */

Refactoring

/** * * In this game a position is represented by a white and black * bitmask. A bit is set if a position is occupied. There are * 9 squares so there are 1<<9 possible positions for each * side. An array of 1<<9 Booleans is created, it marks * all the winning positions. * * @version 1.2, 13 Oct 1995 * @author Arthur van Hoff * @modified 96/04/23 Jim Hagen : winning sounds * @modified 03/07/21 Sara Stoecklin : updated java version */

Refactoring

publicclass TTTV0 extends Applet implements MouseListener { /** * White's current position. The computer is white. */ int white;

/** * Black's current position. The user is black. */ int black;

Refactoring

/** * The squares in order of importance... */ final static int moves[] = {4, 0, 2, 6, 8, 1, 3, 5, 7};

/** * The winning positions. */ static boolean won[] = new boolean[1 << 9]; static final int DONE = (1 << 9) - 1; static final int OK = 0; static final int WIN = 1; static final int LOSE = 2; static final int STALEMATE = 3;

Refactoring

/** * Mark all positions with these bits set as winning. */ static void isWon(int pos) {

for (int i = 0 ; i < DONE ; i++) { if ((i & pos) == pos) {

won[i] = true; }}

}

Refactoring

/** * Initialize all winning positions. */ static {

isWon((1 << 0) | (1 << 1) | (1 << 2));isWon((1 << 3) | (1 << 4) | (1 << 5));isWon((1 << 6) | (1 << 7) | (1 << 8));isWon((1 << 0) | (1 << 3) | (1 << 6));isWon((1 << 1) | (1 << 4) | (1 << 7));isWon((1 << 2) | (1 << 5) | (1 << 8));isWon((1 << 0) | (1 << 4) | (1 << 8));isWon((1 << 2) | (1 << 4) | (1 << 6));

}

Refactoring

/** * Compute the best move for white. * @return the square to take */ int bestMove(int white, int black) {

int bestmove = -1;

Refactoring

loop:for (int i = 0 ; i < 9 ; i++) { int mw = moves[i]; if (((white & (1 << mw)) == 0) &&

((black & (1 << mw)) == 0)) {int pw = white | (1 << mw);if (won[pw]) { // white wins, take it! return mw;}

Refactoring

loop:…….

for (int mb = 0 ; mb < 9 ; mb++) { if (((pw & (1 << mb)) == 0) &&

((black & (1 << mb)) == 0)) {int pb = black | (1 << mb);if (won[pb]) { // black wins, take another continue loop;}

}}

Refactoring

loop:…………..

// Neither white nor black can win in one move, this will do.if (bestmove == -1) { bestmove = mw;}

}}if (bestmove != -1) { return bestmove;}

Refactoring

loop: ……

…………

// No move is totally satisfactory, try the first one that is openfor (int i = 0 ; i < 9 ; i++) { int mw = moves[i]; if (((white & (1 << mw)) == 0) &&

((black & (1 << mw)) == 0)) {return mw;

}}

// No more movesreturn -1;

}

Refactoring

/** * User move. * @return true if legal */ boolean yourMove(int m) {

if ((m < 0) || (m > 8)) { return false;}if (((black | white) & (1 << m)) != 0) { return false;}black |= 1 << m;return true;

}

Refactoring

/** * Computer move. * @return true if legal */ boolean myMove() {

if ((black | white) == DONE) { return false;}int best = bestMove(white, black);white |= 1 << best;return true;

}

Refactoring

/** * Figure what the status of the game is. */ int status() {

if (won[white]) { return WIN;}if (won[black]) { return LOSE;}if ((black | white) == DONE) { return STALEMATE;}return OK;

}

Refactoring

/** * Who goes first in the next game? */ boolean first = true;

/** * The image for white. */ Image notImage;

/** * The image for black. */ Image crossImage;

Refactoring

/** * Initialize the applet. Resize and load images. */ public void init() {

notImage = getImage(getCodeBase(), "oimage.gif");crossImage = getImage(getCodeBase(), "ximage.gif");

addMouseListener(this); }

public void destroy() { removeMouseListener(this); }

Refactoring

/** * Paint it. */ public void paint(Graphics g) {

Dimension d = getSize();g.setColor(Color.black);int xoff = d.width / 3;int yoff = d.height / 3;g.drawLine(xoff, 0, xoff, d.height);g.drawLine(2*xoff, 0, 2*xoff, d.height);g.drawLine(0, yoff, d.width, yoff);g.drawLine(0, 2*yoff, d.width, 2*yoff);

Refactoring

int i = 0;for (int r = 0 ; r < 3 ; r++) { for (int c = 0 ; c < 3 ; c++, i++) {

if ((white & (1 << i)) != 0) { g.drawImage(notImage, c*xoff + 1, r*yoff + 1, this);} else if ((black & (1 << i)) != 0) { g.drawImage(crossImage, c*xoff + 1, r*yoff + 1, this);}

}}

}

Refactoring

/** * The user has clicked in the applet. Figure out where * and see if a legal move is possible. If it is a legal * move, respond with a legal move (if possible). */ public void mouseReleased(MouseEvent e) {

int x = e.getX();int y = e.getY();

Refactoring

switch (status()) { case WIN: case LOSE: case STALEMATE: play(getCodeBase(), "audio/return.au"); white = black = 0; if (first) {

white |= 1 << (int)(Math.random() * 9); } first = !first; repaint(); return;}

Refactoring

// Figure out the row/columnDimension d = getSize();int c = (x * 3) / d.width;int r = (y * 3) / d.height;if (yourMove(c + r * 3)) { repaint();

Refactoring

switch (status()) { case WIN:

play(getCodeBase(), "audio/yahoo1.au");break;

case LOSE:play(getCodeBase(), "audio/yahoo2.au");break;

case STALEMATE:break;

Refactoring

default:if (myMove()) { repaint(); switch (status()) { case WIN:

play(getCodeBase(), "audio/yahoo1.au");break;

case LOSE:play(getCodeBase(), "audio/yahoo2.au");break;

case STALEMATE:break;

default:play(getCodeBase(), "audio/ding.au");

}

Refactoring

} else { play(getCodeBase(), "audio/beep.au");}

}} else { play(getCodeBase(), "audio/beep.au");}

}

Refactoring

public void mousePressed(MouseEvent e) { } public void mouseClicked(MouseEvent e) { } public void mouseEntered(MouseEvent e) { } public void mouseExited(MouseEvent e) { }

public String getAppletInfo() {return "TicTacToe by Arthur van Hoff";

}}

Refactoring

Lesson One: Self-Documenting Code and Functional Testing

Refactoring

Process1. Read, review and understand2. Format and comment3. Perform documenting refactorings

Refactoring

1. Read, review and understand

A. Read existing code Not part of refactoring but necessary.

B. Review it for understandabilityDo the variables have meaningful names?Are their enough commentsDo the methods have meaningful names?

Refactoring

2. Format and comment

• A. Format the code according to supplied standardsFormat the code by some standard to aid in

readability

Comment code for understandingAfter reading the code add any needed

comments to increase understandability

Refactoring

import java.awt.*;import java.awt.event.*;import java.awt.image.*;import java.net.*;import java.applet.*;

/** * A Simple TicTacToe applet. A Tic Tac Toe applet. * A very simple, and mostly brain-dead * implementation of your favorite game! */

My format- get rid of extra white space to show on screens

Refactoring

/** * A bitmask is used for the two players denoting positions occupied. * Each bit represents a square on the board 0..8. * The bit is 0 if not occupied and 1 if occupied. * Winning is determined by comparing the bitmask with * an array of Booleans that marks all the winning positions. * @version 1.2, 13 Oct 1995 * @author Arthur van Hoff * @modified 96/04/23 Jim Hagen : winning sounds * @modified 03/07/21 Sara Stoecklin : updated java version */

Rewrite comments to make them clearer

Refactoring

publicclass TTTV11 extends Applet implements MouseListener {

static final int DONE = (1 << 9) - 1; // sentinal for square loop static final int LOSE = 2; // status for user wins static final int OK = 0; // status for game continues static final int STALEMATE = 3; // status for a tie static final int WIN = 1; // status for computer wins

Place all class constants first in alphabetical order

Refactoring

int black; // user bitmask denotes user squares occupied Image crossImage; // user image boolean first = true; // who goes first next game

// The squares in order of importance... positions 0..8 final static int moves[] = {4, 0, 2, 6, 8, 1, 3, 5, 7}; Image notImage; // computer image int white; // computer bitmask denotes squares occupied static boolean won[] = new boolean[1 << 9]; // winning states

Place all class variables next in alphabetical order

Refactoring

static void isWon(int pos) { // mark winning squares as true win for (int i = 0 ; i < DONE ; i++) { if ((i & pos) == pos) {

won[i] = true; } // end if (i & pos)} // end for

} // end isWon

I move comments to save real estate

Refactoring

static { // initialize winning squares by shifting a one n bits isWon((1 << 0) | (1 << 1) | (1 << 2)); // row one winisWon((1 << 3) | (1 << 4) | (1 << 5)); // row two winisWon((1 << 6) | (1 << 7) | (1 << 8)); // row three winisWon((1 << 0) | (1 << 3) | (1 << 6)); // col one winisWon((1 << 1) | (1 << 4) | (1 << 7)); // col two winisWon((1 << 2) | (1 << 5) | (1 << 8)); // col three winisWon((1 << 0) | (1 << 4) | (1 << 8)); // dia right winisWon((1 << 2) | (1 << 4) | (1 << 6)); // dia left win

} // end static

Document all possible statements

Refactoring

4. Perform Refactorings

1. Rename Method2. Rename Constants

3. Rename Variables

Refactoring

Summary:

The name of a method does not reveal its purpose

Change the name of the method.

Rename Method

Refactoring

Rename Method:

Motivation:

Methods should be named in a way the communicates their intension. Think what the comment for the method would be and turn that comment into the name of the method.

Refactoring

Rename Method:

Customer____________getinvcrelmt ()

Customer____________getInvoiceCreditLimit()

Refactoring

Rename Method:

Mechanics:

Check if method signature is implemented by a super or sub class. If so perform these steps for each implementation.

Declare new method with new name and copy old body into new method.

Change body of old so it calls the new one.

Find references to old and change them to refer to new one.

Remove old method.

Compile and test.

Refactoring

CONSTANTS

static final int CONTINUE = 0; // OLD OK status for game continuesstatic final int ENDINGSTATE=(1<< 9)-1; //OLD DONE 111 111 111

Change these two constants to make code clearer……

TEST BETWEEN CHANGES.

Rename constants, variables methods

Refactoring

if (won[black]) { return LOSE; } if ((black | white) == DONE) { return STALEMATE; } return OK; BECOMES if (won[black]) { return LOSE; } if ((black | white) == DONE) { return STALEMATE; } return CONTINUE;

TEST BETWEEN CHANGES.

Refactoring

VARIABLES

Image crossImage; // user image Image notImage; // computer image BECOMES computerImage = getImage(getCodeBase(), "oimage.gif"); userImage = getImage(getCodeBase(), "ximage.gif");

int white; // White's current position. The computer is white. int black; Black's current position. The user is black. BECOMES int userStatus; //user bitmask denotes user squares occupied OLD BLACK int computerStatus; //computer bitmask denotes squares occupied WHITE


Refactoring

if ((white & (1 << i)) != 0) { g.drawImage(notImage, c*xoff + 1, r*yoff + 1, this);} else if ((black & (1 << i)) != 0) {

g.drawImage(crossImage, c*xoff + 1, r*yoff + 1, this);

BECOMES

if ((computerStatus & (1 << i)) != 0) { // if computer square taken g.drawImage(computerImage, c*xoff + 1, r*yoff + 1, this);

} else if ((userStatus & (1 << i)) != 0) { // if user square taken g.drawImage(userImage, c*xoff + 1, r*yoff + 1, this);

Refactoring

VARIABLES

int bestMove(int white, int black)BECOMES

int bestMove(int computerStatus,int userStatus) { //compute best move

int pw = white | (1 << mw);BECOMES

int pw = computerStatus | (1 << mw);

int pb = black | (1 << mb);BECOMES

int pb = userStatus | (1 << mb);

Refactoring

white = black = 0; BECOMEScomputerStatus = userStatus = 0;

white |= 1 << (int)(Math.random() * 9); BECOMEScomputerStatus |= 1 << (int)(Math.random() * 9);

white |= 1 << best;BECOMEScomputerStatus |= 1 << best;

Refactoring

int best = bestMove(white, black); BECOMESint best = bestMove(computerStatus, userStatus);

if (((black|white)&(1<< m))!= 0) black |= 1 << m; BECOMESif (((userStatus|computerStatus)&(1<< m))!= 0) userStatus |= 1 << m;

if (((white&(1<< mw))== 0) &&((black&(1<<mw))== 0)) BECOMESif (((computerStatus&(1<< mw))== 0) &&((userStatus&(1<<mw))== 0))

Refactoring

if ((black | white) == DONE) BECOMES

if ((userStatus | computerStatus) == DONE)

if (won[white]) return WIN;BECOMES

if (won[computerStatus]) return WIN;

if (won[black]) return LOSE;BECOMES

if (won[userStatus]) return LOSE;

if ((black | white) == DONE) BECOMES

if ((userStatus | computerStatus) == DONE)

Refactoring

MORE VARIABLES

static int moves[] = {4,0,2,6,8,1,3,5,7}; BECOMESstatic int mostStrategicMove[] = {4,0,2,6,8,1,3,5,7};

int mw = moves[i]; BECOMES int mw = mostStrategicMove[i];


Refactoring

won[] = new boolean[1 << 9]; BECOMESwinningState[] = new boolean[1 << 9]; // winning states of game

if (won[white]) BECOMES if (winningState[computerStatus])

if (won[black]) BECOMES if (winningState[userStatus])

if (won[pw]) BECOMES if (winningState[pw])

Refactoring

METHODS

boolean myMove(int m) { boolean yourMove() { BECOMESboolean legalComputerMove() { boolean legalUserMove(int m) {

if (myMove()) if (yourMove(c + r * 3)) BECOMESif (legalComputerMove()) if (legalUserMove(c + r * 3))


Refactoring

OTHERSmw BECOMESint potentialComputerMove = mostStrategicMove[i];

pb BECOMESint potentialUserStatus = userStatus | (1 << j);

c and r BECOMES row and col


Refactoring

loop:for (int i = 0 ; i < 9 ; i++) { int mw = moves[i]; if (((white & (1 << mw)) == 0) && ((black & (1 << mw)) == 0)) { int pw = white | (1 << mw); if (won[pw]) {return mw; } // white wins, take it! for (int mb = 0 ; mb < 9 ; mb++) {

if (((pw & (1 << mb)) == 0) && ((black & (1 << mb)) == 0)) { int pb = black | (1 << mb); if (won[pb]) {continue loop; } // black wins, take another

} }

}

A big difference BEFORE

Refactoring

loop: for (int i = 0 ; i < 9 ; i++) { // for square = 0 to < 9 int potentialComputerMove = mostStrategicMove[i]; if (((computerStatus & (1 << potentialComputerMove)) == 0) && ((userStatus & (1 << potentialComputerMove)) == 0)) { int potentialComputerStatus = computerStatus | (1 << potentialComputerMove); if (winningState[potentialComputerStatus]) { // computer wins return potentialComputerMove; } /// end if (not user taken ) && ( not computer taken )

A big difference AFTER

Refactoring

// the computer did not find a winning move for (int j = 0 ; j < 9 ; j++) { // for square = 0 to < 9 if (((potentialComputerStatus & (1 << j)) == 0) && ((userStatus & (1 << j)) == 0)) { int potentialUserStatus = userStatus | (1 << j); if (winningState[potentialUserStatus]) { // user wins, take another continue loop; } // end if won } // end if &&} // end for

// found a move but user would win

A big difference

Refactoring

// computer has not found a winning move if (bestmove == -1) { // neither can win, this move will do bestmove = potentialComputerMove; } // end if } // end if && } // end for

if (bestmove != -1) { // if no move found return the best one return bestmove; } // end if bestmove

A big difference

Refactoring

// there is no winning or good move – take mostStrategic movefor (int i = 0 ; i < 9 ; i++) { // no great move, try first one open int firstAvailableComputerMove = mostStrategicMove[i]; if (((computerStatus&(1<< firstAvailableComputerMove))== 0) && (userStatus & (1 << firstAvailableComputerMove)) == 0)) {

return firstAvailableComputerMove; } // end if && } // end for

return -1; // return no more moves } // end best move

A big difference

Refactoring

Encapsulate

Refactoring

Process1. Encapsulate ALL class variables2. Unit test3. Functionally test

Refactoring

1. Encapsulate ALL class variables

A. Write get and set methods for ALL class variablesModify ALL references to class variables to use get and set methods.Test methods.

Refactoring

public int getComputerStatus () { return computerStatus; } public Image getUserImage (){return userImage;} public boolean getFirst () { return first; } public Image getComputerImage () { return computerImage;} public void setComputerStatus (int computerStatus) { this.computerStatus = computerStatus; } public void setUserImage (Image userImage) { this.userImage = userImage;} public void setFirst (boolean first) { this.first = first; } public void setCommputerImage (Image computerImage) { this.computerImage = computerImage; }

Write getters and setters for all class variables

Refactoring

Perform other encapsulation refactorings

1. Self encapsulating field2. Encapsulate field3. Encapsulate collection

These are not covered in detail

Refactoring

// GETS AND SETS ADDED TO CODE

public int getComputerStatus () { return computerStatus; } public Image getUserImage (){return userImage;} public boolean getFirst () { return first; } public Image getComputerImage () { return computerImage;} …… public void setComputerStatus (int computerStatus) { this.computerStatus = computerStatus; } public void setUserImage (Image userImage) { this.userImage = userImage;} public void setFirst (boolean first) { this.first = first; } public void setCommputerImage (Image computerImage) { this.computerImage = computerImage; }

Refactoring

//METHODS that need changes in their access to variables

int bestMove(int computerStatus, int userStatus) { has parameters making them local variables – scoping? ok

boolean legalUserMove(int canidateMove) {

boolean legalComputerMove() {

int gameStatus(int computerStatus, int userStatus) {

public void paint(Graphics g) { // paint the screenpublic void mouseReleased(MouseEvent e) {

Refactoring

//METHODS that need changes in their access to variables

computerStatus = userStatus = 0; BECOMESsetComputerStatus(0); setUserStatus(0);

if ((userStatus | computerStatus) == ENDINGSTATE) {BECOMESif ((getUserStatus() | getComputerStatus()) == ENDINGSTATE) {

computerImage = getImage(getCodeBase(), "oimage.gif");BECOMESsetComputerImage(getImage(getCodeBase(), "oimage.gif"));

Refactoring

// domain functionality and GUIswitch (gameStatus()) { // determine status case WIN: case LOSE: case STALEMATE: play(getCodeBase(), "audio/return.au"); computerStatus = userStatus = 0; if (first) { // reset first computerStatus |= 1 << (int)(Math.random() * 9); }// end if first = !first; repaint(); // GUI controlling when to display // RED LINED code NEEDS TO BE A METHOD TO TEST

Refactoring

Make it a METHOD

public void resetFirst() { if (getComputerFirst()) { // reset who is first setComputerStatus ( 1 << (int)(Math.random() * 9)); }// end if setComputerFirst (!getComputerFirst()); } // end resetStatus

Refactoring

Call the METHOD Now this is all GUI code

switch (gameStatus()) { // determine status case WIN: case LOSE: case STALEMATE:

play(getCodeBase(), "audio/return.au"); resetStatus(); repaint(); return;} // end switch

Refactoring

Constants and Variables

Refactoring

Process1. Review scope of all constants2. Review scope of all variables3. Adjust any gets/sets due to reviews4. Apply refactorings

Refactoring

1. Review the scope of all constants2. Review the scope of all variables3. Adjust code to limit scope

A. Review all the constants and variables to make sure they are not MORE global in scope than necessary. Downcast any that are too broad in scope. Adjust gets and sets to adapt to new or modified scope.

Refactoring

3. Adjust code to limit scope

Investigate variables, parameters, and methods to see if they can be local or need to be uplifted to later evaluate their partitioning into classes.

setW

inn

ing

Sta

te

bes

tMo

ve

leg

alU

serM

ove

leg

alC

om

pu

terM

ove

gam

eS

tatu

s

rese

tFir

st

mo

use

Re

lea

sed

init

pai

nt

const int ENDINGSTATE r r rconst int LOSE r rconst int CONTINUE r rconst int STALEMATE r rconst int WIN r r

var int computerStatus p p p ps p s gs g g

var Image userImage g

var boolean computerFirst p sg g

var arrayint mostStrategicMove[] r

var Image computerImage g

var int userStatus p p ps p p gs g g

var arrayint winningState[] u r r

1

2

3

Refactoring

3. Adjust code to limit scope

private static int mostStrategicMove[] = {4,0,2,6,8,1,3,5,7};

// square order of importance

BECOMES

int mostStrategicMove[] = {4,0,2,6,8,1,3,5,7};// square order of importance

// in the method bestMove

Refactoring

4. Apply Refactorings

Replace Magic Number with Symbolic ConstantIntroduce Explaining VariableSplit Temporary Variable Replace Temp with QuerySeparate Query from Modifier ****Replace Array with Object *****Not all of these are covered in detail.

Refactoring

Summary:

You have a literal number with a particular meaning

Create a constant, name it after the meaning, and replace the number with it.

Replace Magic Number with Symbolic Constant

Refactoring

Replace Magic Number with Symbolic Constant:

Motivation:

Magic numbers are numbers with special values that usually are not obvious.

Add a constant to store the values of these magic numbers.

Refactoring

Replace Magic Number with Symbolic Constant: Example:

double potentialEnergy (double mass, double height) {

return mass * 9.81 * height;

}// end potential Energy

double potentialEnergy (double mass, double height {

return mass * GRAVITATIONAL_CONSTANT * height;

}// end potentialEnergy

static Final double GRAVITATIONAL_CONSTANT * 9.81;

Should be

Refactoring


Mechanics:

declare a constant and set to magic number

find occurrences of magic number

if magic matches constant usage – use constant

when all magic numbers changed

compile and test.

Refactoring


static final int ENDINGSTATE = (1 << 9) - 1; BECOMESstatic final int NINE_SHIFTING_BITS = 9;static final int ENDINGSTATE = (1 << NINE_SHIFTING_BITS) - 1;

for (int i = 0 ; i < 9 ; i++) { // for square = 0 to < 9 BECOMES

static final int firstCell = 0; // first cell at row 1 col 1static final int lastCell = 8; // last cell at row 3, col 3

for (int i = firstCell ; i <= lastCell ; i++) { \

Refactoring


int bestmove = -1;if (bestmove == -1) { if (bestmove != -1) { return -1; // return no more moves

BECOMESstatic final int bestMoveNotFound = -1; //indicating best move not foundint bestmove = bestmoveNotFound;if (bestmove == bestMoveNotFound) { if (bestmove != bestMoveNotFound) { return bestMoveNotFound; // return no more moves

Refactoring


for (int row = 0 ; row < 3 ; row++) { for (int col = 0 ; col < 3 ; col++, i++) {int col = (x * 3) / d.width;// determine colint row = (y * 3) / d.height; // determine the row

BECOMESstatic final int NUMBER_OF_COLUMNS = 3;static final int NUMBER_OF_ROWS = 3;for (int row = 0 ; row < NUMBER_OF_ROWS ; row++) { for (int col = 0 ; col < NUMBER_OF_COLUMNS ; col++, i++) {int col = (x * NUMBER_OF_COLUMNS) / d.width;// determine colint row = (y * NUMBER_OF_ROWS) / d.height; // determine the row

Refactoring

Summary:

You have a complicated expression.

Put the result of the expression in a temp with a name that explains the purpose.

4. Introduce Explaining Variable

Refactoring

4. Introduce Explaining Variable:

Motivation:

Complex code that requires many lines of code but could more easily be explained with some variable name that helps to explain the code semantics.

Refactoring

4. Introduce Explaining Variable: Example: If ( (platform.toUpperCase().indexOf(“MAC”) > -1) &&

(browser.toUpperCase().indexOf(“IE”) > -1) &&

wasInitialized () && resize >0)

{ ….. other code …..}

goes to

final boolean isMacOs = platform.toUpperCase().indexOf(“MAC”) > -1;

final boolean isIEBrowser = platform.toUpperCase().indexOf(“IE”) > -1;

final boolean wasResized = resize >0;

If (isMacOs && isIEBrowser && wasInitialized () && was Resized)

{ ….. other code …..}

Refactoring

4. Introduce Explaining Variable:

Mechanics:

declare a final temp variable

set it to the result portion of the expression

compile and test

Refactoring

Summary:

You have one temp assigned to more than once and it is not a loop variable.

Make a new temp for each assignment

5. Split Temporary Variables

Refactoring

5. Split Temporary Variables:

Motivation:

Looping variables (control variables of a loop) or accumulator variables (summing variables) may have the need for assignments more than once. Other variables should not be assigned to more than once.

Investigation of greater than one assignments to the same variable may yield variables that are actually used for different semantics.

Refactoring

5. Split Temporary Variables: Example: double getDistanceTravelled (int time) {

double result;

double acc = _primaryForce / _mass; // initial value of acceleration of first force

int primaryTime = Math.min(time, _delay);

result = 0.5 * acc * primaryTime * primaryTime;

int secondaryTime = time - _delay;

if (secondaryTime > 0) {

double primaryVel = acc * _delay;

acc = (_primaryForce * _secondaryForce) / _mass; // final value of acceleration

result += primaryVel * secondaryTime + 0.5 *acc * secondaryTime * secondaryTime;

} // end if

return result;

} // end getDistanceTravelled

Refactoring

5. Split Temporary Variables: Example: double getDistanceTravelled (int time) {

double result;

double primaryacc = _primaryForce / _mass; // initial value of acceleration of first force

int primaryTime = Math.min(time, _delay);

result = 0.5 * primaryacc * primaryTime * primaryTime;

int secondaryTime = time - _delay;

if (secondaryTime > 0) {

double primaryVel = acc * _delay;

double acc = (_primaryForce * _secondaryForce) / _mass; // final value of acceleration

result += primaryVel * secondaryTime + 0.5 *acc * secondaryTime * secondaryTime;

} // end if

return result;

} // end getDistanceTravelled

CAN YOU THINK OF OTHER REFACTORING?

Refactoring

5. Split Temporary Variables:

Mechanics:

find the variables assigned to more than once

change the name for each different assignments

change references

compile and test

Refactoring

Summary:

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.

3. Replace Temp with Query

Refactoring

3. Replace Temp with Query:

Motivation:

Methods with many temps tend to be long.

Replacing the temp variable with a query method makes the code cleaner for further refactoring.

Refactoring

3. Replace Temp with Query: Example:

double getPrice(){

int basePrice = _quanity * itemPrice;

double discountFactor;

if (basePrice > 1000) discountFactor = 0.95

else discountFactor = 0.98;

return basePrice *discountFactor;

}// end getPrice

Refactoring


double getPrice(){

int basePrice = _quanity * itemPrice;

double discountFactor;

if (basePrice() > 1000) discountFactor = 0.95


return basePrice() *discountFactor;

}// end getPrice

private int basePrice () { return quanity * itemPrice; }

Refactoring


double getPrice(){

double discountFactor = discountFactor();

if (basePrice() > 1000) discountFactor = 0.95



}// end getPrice

private double discountFactor () {

if (basePrice() > 1000) return 0.95;

else return 0.98;

} // end discountFactor

Extract Method

Refactoring


double getPrice(){

double discountFactor = discountFactor();


}// end getPrice

private double discountFactor () {

if (basePrice() > 1000) return 0.95;

else return 0.98;

} // end discountFactor

private int basePrice () { return quanity * itemPrice; }

Can I do more?

Replace Temp with Query

Refactoring


Mechanics:

declare the temp as final to check references

compile to test

extract the right hand side as a method body

if a loop take entire loop

name and construct the method

replace the temp with the call

Refactoring


int best = bestMove(computerStatus, userStatus);setComputerStatus (computerStatus | 1 << best);

BECOMESsetComputerStatus

(computerStatus | 1 << bestMove(computerStatus, userStatus));

Refactoring

Methods

Refactoring

Learning objective – “good” object-oriented methods.

Primary refactoring used to produce “good” methods is Extract Method, but others are covered.

Refactoring

“Good” method is defined as a method with an acceptable level of cohesion [Kwang99,Smith04]

Refactoring

“Good” method is further defined as a method

acceptably cohesive less than 20 lines in length

accomplishes one functionality testable with one unit test method

Refactoring

Kang [KAN99, SMI04] defines levels of cohesion which are ranked from the best level to the worst.

Functional, sequential and communicational cohesion in methods is an acceptable level.

Refactoring

Functional cohesion deals with the ability of a method to produce only one output (i.e., to change the state of only one member variable) and is considered the best level of cohesion.

Refactoring

In sequential cohesion, more than one variable is modified; however, all modifications result in the change to only one member variable. The outputs of one or more modifications are the inputs to the modification of the member variable.

Refactoring

In communicational cohesion, multiple outputs are dependent on a common input but are not derived in a loop or selection statement. Not as good as functional or sequential, refactor if possible.

Refactoring

Method sizes may be very small as a natural consequence of good object-oriented design [Lieberherr89]. Studies have shown that larger methods result in a reduction in understandability, reuse, and maintainability. [Hudli94, Lorenz94, McCabe94, Tegaden92].

Refactoring

Refactorings usedExtract MethodInline Method

Refactoring

Summary: - most key refactoring method

You have a cluster of code which can be grouped into a method to simplify the code.

Turn the cluster in to a method with a name that explains the function of the method.

1. Extract Method

Refactoring

1. Extract Method:

Motivation:

Method is too long, needs comments to understand.

Need very small methods returning one value.

Method names are allowed to be longer than the code itself.

Refactoring

1. Extract Method: Example: no local variablesvoid printOwing() { Enumeration e = _orders.elements(); double outstanding = 0.0;

// print banner System.our.println (“*************”); System.our.println (“Customer Owes”) System.our.println (“*************”);

// caculate outstanding while (e.hasMoreElements()) { Order each = (Order) e.nextElement(); outstanding += each.getAmount(); // print details System.out.println (“name:” + _name); System.out.println (“amount” + outstanding);

}

Refactoring

1. Extract Method: Example: no local variablesvoid printOwing() { Enumeration e = _orders.elements(); double outstanding = 0.0;

printBanner();

// cacluate outstanding while (e.hasMoreElements()) { Order each = (Order) e.nextElement(); outstanding += each.getAmount(); // print details System.out.println (“name:” + _name); System.out.println (“amount” + outstanding);…

Void printBanner() { System.our.println (“*************”); System.our.println (“Customer Owes”) System.our.println (“*************”);} // end printBanner

}

Refactoring

1. Extract Method: Example: locals only referencedvoid printOwing() { Enumeration e = _orders.elements(); double outstanding = 0.0;

printBanner()

// cacluate outstanding while (e.hasMoreElements()) { Order each = (Order) e.nextElement(); outstanding += each.getAmount(); // print details System.out.println (“name:” + _name); System.out.println (“amount” + outstanding);

Void printBanner() { System.our.println (“*************”); System.our.println (“Customer Owes”) System.our.println (“*************”);} // end printBanner

}

Refactoring

1. Extract Method: Example: locals only referenced pass in needed data as parameters

void printOwing() { Enumeration e = _orders.elements(); double outstanding = 0.0;

printBanner()

// cacluate outstanding while (e.hasMoreElements()) { Order each = (Order) e.nextElement(); outstanding += each.getAmount(); printDetails (outstanding);

….

Void printDetails (double outstanding) { System.out.println (“name:” + _name); System.out.println (“amount” + outstanding);

………

}

Refactoring

1. Extract Method: Example: one local updated void printOwing() { Enumeration e = _orders.elements(); double outstanding = 0.0;

printBanner()

// cacluate outstanding while (e.hasMoreElements()) { Order each = (Order) e.nextElement(); outstanding += each.getAmount(); printDetails (outstanding);

Void printDetails (double outstanding) { System.out.println (“name:” + _name); System.out.println (“amount” + outstanding);

………

}

Refactoring

1. Extract Method: Example: one local updatedreturn updated local

void printOwing() { printBanner()

double outstanding = getOutstanding(); printDetails (outstanding);

Double getOutstanding() { Enumeration e = _orders.elements(); double outstanding = 0.0; // cacluate outstanding while (e.hasMoreElements()) { Order each = (Order) e.nextElement(); outstanding += each.getAmount(); } // end while return outstanding; } // end getOutstanding………

}

Refactoring

1. Extract Method: Mechanics

locate clusters of actual code

copy to a new method

if local variables needed define parameters

if local variable (one) updated make return type

build call

if local variable updated (more than one)

make multiple methods or split variables

build calls

Refactoring

Summary:

You have a method body that is just as clear as the name of the method

Turn the method into inline code rather than a method.

2. Inline Method

Refactoring

2. Inline Method:

Motivation:

The body is just as clear as a method name and there exist needless indirection.

Refactoring

2. Inline Method: Example:

int getRating ()}

return (moreThanFiveLateDeliveries () ? 2:1;

}// end getRating

boolean moreThanFiveLateDeliveries() {

return _numberofLateDeliveries > 5;

} // end moreThanFiveLateDeliveries

Refactoring

2. Inline Method: Example:

int getRating ()}

return _numberofLateDeliveries > 5 ? 2:1;

} // end getRating

Refactoring

2. Inline Method: Mechanics:

find calls to the method

replace with inline code

test

remove method definition

Refactoring

???? init(…)???? destroy(…)???? legalComputerMove(…)???? legalUserMove (…)???? gameStatus (…)???? resetFirst(…)???? paint (…)???? mouseReleased(…)???? bestMove(…) - more than 20 lines

*acceptably cohesive*less than 20 lines *accomplishes one functionality *testable with one unit test

* Indicates that it meets criteria

Refactoring

**** init(…)

public void init() { // initialize applet, load images, add listener setComputerImage(getImage(getCodeBase(), "oimage.gif")); setUserImage(getImage(getCodeBase(), "ximage.gif")); addMouseListener(this); } // end init

*Cohesive acceptable - modifies two class variables indirectly *Size: acceptable

*One functionality: TWO set up images and add listener – what if we wanted to implement a phone interface.

*One unit test: neet two tests one for images and one for listenert

Refactoring

**** destroy(…)

public void destroy() { // destroy listener needed for closing removeMouseListener(this); } // end destroy

*Cohesive – modifies no class variables - acceptable*Size – acceptable *One function – only one acceptable *One unit test – only one acceptable

Refactoring

**** init(…)**** destroy(…)???? legalComputerMove(…)???? legalUserMove (…)???? gameStatus (…)???? resetFirst(…)???? paint (…)???? mouseReleased(…)???? bestMove(…) - more than 20 lines


Refactoring

???? legalUserMove (…)

boolean legalUserMove(int computerStatus, int userStatus, int canidateMove) { if ((canidateMove < 0) || (canidateMove > 8)) { return false; } // end if if (((userStatus | computerStatus) & (1 << canidateMove)) != 0) { return false;} // end if setUserStatus (userStatus | 1 << canidateMove); return true; } // end legalUserMove

*Cohesive – modifies userStatus indirectly - acceptable*Size – acceptable *Only One function – NO Test legal AND sets user status *Testable two types of tests for two function

LOOK at where this method is callled to see if we can separate.

Refactoring

???? legalUserMove (…)

boolean legalUserMove(int computerStatus, int userStatus, int canidateMove) { if ((canidateMove < 0) || (canidateMove > 8)) { return false; } // end if if (((userStatus | computerStatus) & (1 << canidateMove)) != 0) {return false;} // end if setUserStatus (userStatus | 1 << canidateMove); return true; } // end legalUserMove

if (legalUserMove(getComputerStatus(), getUserStatus(), col + row * 3)) { repaint();

…….

Refactoring

**** legalUserMove (…)

boolean legalUserMove(int computerStatus, int userStatus, int canidateMove) { // user move return true if legal move if ((canidateMove < 0) || (canidateMove > 8)) { return false;} // end if if (((userStatus | computerStatus) & (1 << canidateMove)) != 0) { return false; } // end if return true; } // end legalUserMove

int canidateMove = col + row * 3; if (legalUserMove(getComputerStatus(), getUserStatus(), canidateMove)) {

setUserStatus (userStatus | 1 << canidateMove);

Refactoring

???? legalComputerMove(…)

boolean legalComputerMove(int computerStatus, int userStatus) { if ((userStatus | computerStatus) == ENDINGSTATE) {return false;} //end if setComputerStatus(computerStatus|1<<bestMove(computerStatus,userStatus));

return true; } // end legalComputerMove

Cohesive = modifies computerStatus indirectly -- acceptableSize – acceptableOne function – NO it test if legal AND sets computer status Testing - two types of tests one for the legality and one for setAgain LOOK at where this method is called to see if we can separate

Refactoring

**** legalComputerMove(…)

boolean legalComputerMove(int computerStatus, int userStatus) { if ((userStatus | computerStatus) == ENDINGSTATE) {return false;} // end if setComputerStatus (computerStatus | 1 << bestMove(computerStatus, userStatus)); return true; } // end legalComputerMove

if (legalComputerMove(getComputerStatus(), getUserStatus())) { repaint();

………

Refactoring

**** legalComputerMove(…)

boolean legalComputerMove(int computerStatus, int userStatus) { if ((userStatus|computerStatus)==ENDINGSTATE) {return false; } // end if return true; } // end legalComputerMove

if (legalComputerMove(getComputerStatus(), getUserStatus())) { setComputerStatus (computerStatus | 1 << bestMove(getComputerStatus(),

getUserStatus()));

Refactoring

**** init(…)**** destroy(…)**** legalComputerMove(…)**** legalUserMove (…)???? gameStatus (…)???? resetFirst(…)???? paint (…)???? mouseReleased(…)???? bestMove(…) - more than 20 lines


Refactoring

**** gameStatus (…)

int gameStatus(int computerStatus, int userStatus ) { if (winningState[computerStatus]) { return WIN;} // end if if (winningState[userStatus]) { return LOSE; } // end if if ((userStatus | computerStatus) == ENDINGSTATE) {

return STALEMATE; } // end if return CONTINUE; } // end gameStatus Cohesive = no modifications -- acceptable

Size – acceptableOne function – acceptableTesting - one test – acceptable (4 test needed.

Refactoring

???? resetFirst(…)

public boolean resetFirst (boolean computerFirst) { if (computerFirst) { // reset who is first setComputerStatus ( 1 << (int)(Math.random() * 9)); }// end if return !computerFirst; } // end resetStatus

Cohesive = modifies one indirectly -- acceptableSize – acceptableOne function – NO resets first AND sets computer statusTesting - two types of tests one for reset and one for the setLOOK at where this method is called to see if we can separate

Refactoring

**?? resetFirst(…)

public boolean resetFirst (boolean computerFirst) { if (computerFirst) { // reset who is first setComputerStatus ( 1 << (int)(Math.random() * 9)); }// end if return !computerFirst; } // end resetStatus\

setComputerFirst (resetFirst(getComputerFirst()));

Refactoring

**** resetFirst(…)

if (computerFirst) { setComputerStatus ( 1 << (int)(Math.random() * 9));

}// end if setComputerFirst (!computerFirst);

It makes more sense to take move the code back into the calling method and look (our case) or relook (if already evaluated) at the calling method for characteristics of good.

Refactoring

**** init(…)**** destroy(…)**** legalComputerMove(…)**** legalUserMove (…)**** gameStatus (…)**** resetFirst(…)???? paint (…)???? mouseReleased(…)???? bestMove(…) - more than 20 lines


Refactoring

???? paint (…)

public void paint(Graphics g) { // paint the screenDimension d = getSize();g.setColor(Color.black);int xoff = d.width / NUMBER_OF_ROWS; int yoff = d.height / NUMBER_OF_COLUMNS;

g.drawLine(xoff, 0, xoff, d.height); // draw first horiz lineg.drawLine(2*xoff, 0, 2*xoff, d.height); // draw second lineg.drawLine(0, yoff, d.width, yoff); // draw first verticle lineg.drawLine(0, 2*yoff, d.width, 2*yoff); // draw second line

Refactoring

???? paint (…) **continued int i = 0;for (int row = 0 ; row < NUMBER_OF_ROWS ; row++) {draw images for (int col = 0 ; col < NUMBER_OF_COLUMNS ; col++, i++) { if ((getComputerStatus() & (1 << i)) != 0) { // if square take g.drawImage(getComputerImage(), col*xoff + 1,row*yoff + 1, this); } else if ((getUserStatus() & (1 << i)) != 0) { // if user square taken g.drawImage(getUserImage(), col*xoff + 1, row*yoff + 1, this); } // end if }// end for} // end for} // end paint

Refactoring

*??* paint (…) **continued

Cohesive – sets no class variables – acceptableSize – somewhat long Functionality - two exist 1) draw the grid 2) draw the imagesTesting – would need two tests for each each function except both are gui so these are functional tests

SO we will separate the functions

Refactoring

THE NEW methods

public void drawGrid(Graphics g, Dimension d, int xoff, int yoff){

g.drawLine(xoff, 0, xoff, d.height); // draw first horizontal lineg.drawLine(2*xoff, 0, 2*xoff, d.height); // draw second horizontal lineg.drawLine(0, yoff, d.width, yoff); // draw first verticle lineg.drawLine(0, 2*yoff, d.width, 2*yoff); // draw second verticle line

} // end drawGrid

Refactoring

THE NEW methods public void drawImages(Graphics g, Dimension d, int xoff, int yoff){ int i = 0; for (int row = 0 ; row < NUMBER_OF_ROWS ; row++) { // draw computer and user images for (int col = 0 ; col < NUMBER_OF_COLUMNS ; col++, i++) { if ((getComputerStatus() & (1 << i)) != 0) { // if computer bitmask square taken

g.drawImage(getComputerImage(), col*xoff + 1, row*yoff + 1, this);} else if ((getUserStatus() & (1 << i)) != 0) { // if user bitmask square taken g.drawImage(getUserImage(), col*xoff + 1, row*yoff + 1, this);

} // end if }// end for } // end for

} // end draw Images

Refactoring

**** paint (…) THE NEW paint public void paint(Graphics g) { // paint the screen

Dimension d = getSize();g.setColor(Color.black);int xoff = d.width / NUMBER_OF_ROWS; int yoff = d.height / NUMBER_OF_COLUMNS;

drawGrid(g, d, xoff, yoff);drawImages (g, d, xoff, yoff);

}// end paint

Refactoring

**** init(…)**** destroy(…)**** legalComputerMove(…)**** legalUserMove (…)**** gameStatus (…)**** resetFirst(…)**** paint (…)???? mouseReleased(…)???? bestMove(…) - more than 20 lines


Refactoring

???? mouseReleased(…)

public void mouseReleased(MouseEvent e) { // user clicked appletint x = e.getX(); // get mouse x locationint y = e.getY(); // get mouse y location

Many functions are included in this method and we will extract this functionality in methods

Refactoring

// this code checks if the game is over on the previous move switch (gameStatus(getComputerStatus(), getUserStatus())) { case WIN: case LOSE: case STALEMATE: play(getCodeBase(), "audio/return.au"); setComputerStatus(0); setUserStatus(0); if (computerFirst) { setComputerStatus ( 1 << (int)(Math.random() * 9)); }// end if setComputerFirst (!computerFirst);

repaint(); return;

} // end switch

Refactoring

// find out where the click occurred Dimension d = getSize(); int col = (x * NUMBER_OF_COLUMNS) / d.width; int row = (y * NUMBER_OF_ROWS) / d.height;

Refactoring

// determine if user move causes a win or stalemateif (legalUserMove(getComputerStatus(),getUserStatus(),col+row*3)) { repaint(); switch (gameStatus(getComputerStatus(), getUserStatus())) { case WIN:

System.out.println ("I win");break;

case LOSE: System.out.println ("You win"); break; case STALEMATE: System.out.println ("No Winner"); break;

………

Refactoring

// find a legal computer move and see if it wins or stalesdefault:


switch (gameStatus(getComputerStatus(), getUserStatus())) { case WIN:

System.out.println ("I win this move"); break;

case LOSE: System.out.println ("You win this move");

break; case STALEMATE:

System.out.println ("No Winner this move");break;

default:} // end if} else {

play(getCodeBase(), "audio/beep.au");} // end else

} // end default} else { play(getCodeBase(), "audio/beep.au");}// end else

} // end mouseReleased

Refactoring

???? mouseReleased(…)

Cohesive – sets computerStatus, computerFirst, userStatus Size – somewhat long Functionality - many functions some may be reusableTesting – would need many tests

SO we will separate the functions

Refactoring

???? gameOver(…)public boolean gameOver(int status) { // check and reset if a WIN, LOSE, or STALEMATE after computer move switch (status) { case WIN: case LOSE: case STALEMATE: play(getCodeBase(), "audio/return.au"); setComputerStatus(0); setUserStatus(0); if (computerFirst) { setComputerStatus ( 1 << (int)(Math.random() * 9)); }// end if setComputerFirst (!computerFirst); repaint(); return true; default: return false; } // end switch } // end gameOver

*Cohesive – NO all sets here*Size - acceptables *One functionality - no many*Testable – no many

Refactoring

???? resetGame(…)

public void resetGame () { play(getCodeBase(), "audio/return.au"); setComputerStatus(0); setUserStatus(0); if (computerFirst) { setComputerStatus ( 1 << (int)(Math.random() * 9)); }// end if setComputerFirst (!computerFirst); } // end resetGame *Cohesive – NO all sets here

*Size - acceptable *One functionality - no many*Testable – yes only one not a set

Refactoring

**** playComputerIffirst(…)

public void playComputerIfFirst ( ) { if (getComputerFirst()) { setComputerStatus ( 1 << (int)(Math.random() * 9)); }// end if } // end playComputerIfFirst

*Cohesive – only indirectly sets one class variable*Size - acceptable *One functionality - yes *Testable – yes

Refactoring

**** resetGame(…)

public void resetGame () { play(getCodeBase(), "audio/return.au"); setComputerStatus(0); setUserStatus(0); playComputerifFirst ( ); setComputerFirst (!getComputerFirst()); } // end resetGame *Cohesive – acceptable ONLY sets

*Size - acceptable *One functionality - only sets*Testable – yes sets

Refactoring

???? mouseReleased(…) new version 2

public void mouseReleased(MouseEvent e) { // user clicked applet int x = e.getX(); // get mouse x location int y = e.getY(); // get mouse y location

Dimension d = getSize(); int col = (x * NUMBER_OF_COLUMNS) / d.width; // determine col int row = (y * NUMBER_OF_ROWS) / d.height; // determine row

if (gameOver (gameStatus(getComputerStatus(), getUserStatus()))) { resetGame(); repaint(); return; } // end if

Refactoring

mouseReleased () VERSION 2 CONTINUED// determine if user move causes a win, loose or stalemate and post itif (legalUserMove(getComputerStatus(),getUserStatus(),col+row*3)) { repaint(); switch (gameStatus(getComputerStatus(), getUserStatus())) { case WIN:

System.out.println ("I win");break;

case LOSE: System.out.println ("You win"); break; case STALEMATE: System.out.println ("No Winner"); break;

………

Refactoring

// find a legal computer move and see if it wins or stales // VERSION 2 default:


switch (gameStatus(getComputerStatus(), getUserStatus())) { case WIN:

System.out.println ("I win this move"); break;

case LOSE: System.out.println ("You win this move");

break; case STALEMATE:

System.out.println ("No Winner this move");break;

default:} // end if} else {

play(getCodeBase(), "audio/beep.au");} // end else

} // end default} else { play(getCodeBase(), "audio/beep.au");}// end else

} // end mouseReleased

Refactoring

???? postGameStatus(…) public void postGameStatus (int status) { switch (status) { case WIN:

System.out.println ("I win #1");break;

case LOSE:System.out.println ("You win #1");break;

case STALEMATE: System.out.println ("No Winner #1");

break; default: } // end switch } // end checkGameStatus

*Cohesive – no modifies acceptable*Size - acceptable *One functionality - yes acceptable*Testable – yes one type (3 tests)

Refactoring

???? VERSION 3 of mouseReleased(…)

public void mouseReleased(MouseEvent e) { // user clicked applet int x = e.getX(); // get mouse x location int y = e.getY(); // get mouse y location Dimension d = getSize(); int col = (x * NUMBER_OF_COLUMNS) / d.width; / determine col int row = (y * NUMBER_OF_ROWS) / d.height; // determine row if (gameOver (gameStatus(getComputerStatus(), getUserStatus()))) {

resetGame(); repaint(); return; } // end if

Refactoring

???? REMAINDER OF mouseReleased(…) VERSION 3 int canidateMove = col + row * 3;if (legalUserMove(getComputerStatus(), getUserStatus(), canidateMove)) { setUserStatus (userStatus | 1 << canidateMove); repaint(); int status = gameStatus(getComputerStatus(), getUserStatus()); if (!(status == CONTINUE)) { postGameStatus (status); } else { if (legalComputerMove(getComputerStatus(), getUserStatus())) { setComputerStatus(computerStatus|1<<bestMove(getComputerStatus(),getUserStatus())); repaint(); postGameStatus(gameStatus(getComputerStatus(), getUserStatus())); } else { play(getCodeBase(), "audio/beep.au"); } // end else } // end else } else { // not legal user move play(getCodeBase(), "audio/beep.au");}// end else

Refactoring

???? mouseReleased(…) VERSION 3

*Cohesive – sets two class variables indirectly ?*Size – still somewhat large *One functionality - no several functions

tries to moveUser and moveComputer*Testable – no several types per function

Refactoring

???? mouseReleased(…) VERSION 4 public void mouseReleased(MouseEvent e) { // user clicked applet

int x = e.getX(); // get mouse x location int y = e.getY(); // get mouse y location

Dimension d = getSize(); int col = (x * NUMBER_OF_COLUMNS) / d.width; get col int row = (y * NUMBER_OF_ROWS) / d.height; // determine row

if (gameOver (gameStatus(getComputerStatus(), getUserStatus()))) { resetGame(); repaint(); return;

} // end if

Refactoring

???? mouseReleased(…) VERSION 4 CONTINUEDint canidateMove = col + row * 3;if (legalUserMove(getComputerStatus(), getUserStatus(), canidateMove)) { repaint(); setUserStatus (userStatus | 1 << canidateMove); int status = gameStatus(getComputerStatus(), getUserStatus()); if (status == CONTINUE) { if (legalComputerMove(getComputerStatus(), getUserStatus())) { repaint(); moveComputer(); } else { play(getCodeBase(), "audio/beep.au"); } // end else } else { postGameStatus (status); } // end else } else { play(getCodeBase(), "audio/beep.au"); }// end else // not legal} // end mouseReleased Still not good but best we can do

Refactoring

**** init(…)**** destroy(…)**** legalComputerMove(…)**** legalUserMove (…)**** gameStatus (…)**** resetFirst(…)**** paint (…)**?? mouseReleased(…)???? bestMove(…)


Refactoring

???? bestMove (…)

int bestMove(int computerStatus, int userStatus) { int mostStrategicMove[] = {4,0,2,6,8,1,3,5,7}; int bestmove = bestMoveNotFound; loop: for (int i = firstCell ; i <= lastCell ; i++) { int potentialComputerMove = mostStrategicMove[i]; if (((computerStatus & (1 << potentialComputerMove)) == 0) && ((userStatus & (1 << potentialComputerMove)) == 0)) {

int potentialComputerStatus = computerStatus | (1 << potentialComputerMove);

if (winningState[potentialComputerStatus]) { // computer wins return potentialComputerMove;

} /// end if (not user taken ) && ( not computer taken )

Refactoring

???? bestMove (…) CONTINUED for (int j = firstCell ; j <= lastCell ; j++) { // for square = 0 to < 9 if (((potentialComputerStatus & (1 << j)) == 0) && ((userStatus & (1 << j)) == 0)) { int potentialUserStatus = userStatus | (1 << j); if (winningState[potentialUserStatus]) { // user wins, take another continue loop; } // end if won } // end if && } // end for if (bestmove == bestMoveNotFound) { // neither wins, this move will do bestmove = potentialComputerMove; } // end if } // end if &&} // end for

Refactoring

???? bestMove (…) CONTINUED

*Cohesive – sets no class variables*Size – somewhat large *One functionality - yes*Testable – one type (several tests)

Refactoring

???? userWins (…) public boolean userWins (int potentialComputerStatus) { for (int j = firstCell ; j <= lastCell ; j++) { // for square = 0 to < 9 if (((potentialComputerStatus & (1 << j)) == 0) && ((userStatus & (1 << j)) == 0)) { int potentialUserStatus = userStatus | (1 << j);

if (winningState[potentialUserStatus]) { // user wins, take anothe return true; } // end if won

} // end if && } // end for return false; } // end checkIfUserWon

*Cohesive – no changes to class var*somewhat long *one function – yes one *testable - yes one type of test

Refactoring

???? bestMove (…) VERSION 2

int bestMove(int computerStatus, int userStatus) { int mostStrategicMove[] = {4,0,2,6,8,1,3,5,7}; int bestmove = bestMoveNotFound; loop:

Refactoring???? bestMove (…) VERSION 2 CONTINUED for (int i = firstCell ; i <= lastCell ; i++) { int potentialComputerMove = mostStrategicMove[i];

if (((computerStatus & (1 << potentialComputerMove)) == 0) && ((userStatus & (1 << potentialComputerMove)) == 0)) {

int potentialComputerStatus = computerStatus | (1 << potentialComputerMove);

if (winningState[potentialComputerStatus]) { // computer wins return potentialComputerMove;

} /// end if (not user taken ) && ( not computer taken ) if (userWins (potentialComputerStatus)) { continue loop;} if (bestmove == bestMoveNotFound) { // neither can win bestmove = potentialComputerMove;

} // end if } // end if &&

} // end for

Refactoring

???? bestMove (…) VERSION 2 CONTINUED if (bestmove != bestMoveNotFound) { // return best one return bestmove; } // end if bestmove for (int i = firstCell ; i <= lastCell ; i++) { // try first one open int firstAvailableComputerMove = mostStrategicMove[i]; if (((computerStatus & (1 << firstAvailableComputerMove)) == 0) && ((userStatus & (1 << firstAvailableComputerMove)) == 0)) { return firstAvailableComputerMove; } // end if && } // end for return bestMoveNotFound; // return no more moves } // end best move

Refactoring

**** bestMove (…) CONTINUED

*Cohesive – sets no class variables*Size – still somewhat large but tolerable *One functionality - yes*Testable – yes one type several tests

Refactoring

REMEMBER

For each new method that is not ONLY functionally tested, you will have to write a unit test.

Refactoring

Conditionals

Refactoring

Conditionals should not be too complex. If they are complex, refactor them to make simple methods making readability and understandability better.

Refactoring

Learning objective – have simple conditionals which are self documenting with meaningful names

Refactoring

In our program with explaining variable names, there are still some conditionals which are difficult to understand. We find them in mainly in bestMove().

Refactoring

FOR EXAMPLEint bestMove(int computerStatus, int userStatus) {

…… if (((computerStatus & (1 <<

potentialComputerMove)) == 0) && ((userStatus & (1 << potentialComputerMove)) == 0))

if (((potentialComputerStatus & (1 << j)) == 0) && ((userStatus & (1 << j)) == 0))

if (((computerStatus & (1 <<firstAvailableComputerMove)) == 0) && ((userStatus & (1 <firstAvailableComputerMove)) == 0)) {

Refactoring

REFACTORINGS

Decomposing ConditionalConsolidating Conditional Statements Consolidate Duplicate Conditional FragmentsReplace Nested Conditional with Guard ClausesRemove Control Flag

Not all of these are covered in detail.Rather the resulting code for TTT is shown.

Refactoring

if (((computerStatus & (1 << firstAvailableComputerMove)) == 0) && ((userStatus & (1 << firstAvailableComputerMove)) == 0))

public boolean cellEmpty (int computerStatus,int potentialComputerMove,int userStatus) { return ((computerStatus & (1 << potentialComputerMove)) == 0) && ((userStatus & (1 << potentialComputerMove)) == 0); } // end cellEmpty

BECOMES

Refactoring

if ((getComputerStatus() & (1 << i)) != 0)

public boolean squareOccupied (int i, int playerStatus) { return ((playerStatus & (1 << i)) != 0); } // end squareOccupied

BECOMES

WITH

if (squareOccupied (i, getComputerStatus()))

REUSED 3 times

Refactoring

• Refactoring has gained much popularity during the last few years. There a website that defines all the refactorings at

• http://www.refactoring.com/catalog/index.html• You will find that some of these will become

commonplace in your code and some you will have to work hard to accomplish.

refactoring this lecture is divided into and introduction to refactoring and then several lessons....

Documents

refactoring public class

movie movie

refactoring class rental

bad code code

end movie code

int pricecode

movie private int

string slide