cs2340: Refactoring and Smalltalk

1
cs2340 Refactoring and Smalltalk

• Spring 2007

2
Motivation

• Most of us will not be involved in a totally new
  forward-engineered product.
• There is already a code base or design.
• What if it is crummy?

3
Refactoring

• Improving the design of code already written.
• Used in many Agile development processes
• A disciplined technique not hacking
• Proper implementation requires good unit tests.

4
Bad Smells

• Duplicate Code (Extract method)
• Long Method (Extract method)
• OO methods (especially in Smalltalk) are short
  (5-10 lines)
• Large Class (Extract Class)
• Usually indicates violation of Single
  Responsibility (Low Cohesion)

5
Bad Smells

• Long Parameter List (Introduce Parameter Object)
• from 1 to 4 with except ascending
  true security myAcl target myClass
• Divergent Change (Extract Class)
• Shotgun Surgery (Move Method)

6
Bad Smells

• Feature Envy (Extract Method, Move Method)
• Data Clumps (Extract Class, Preserve Whole
  Object, Introduce Parameter Object)
• Primitive Obsession (Replace Data Value with
  Object)

7
Bad Smells

• Switch statements (Replace Type Code with
  State/Strategy, Replace conditional with
  Polymorphism)
• Lazy Class (Collapse Hierarchy, Inline Class)
• Speculative Generality (Collapse Hierarchy,
  Inline class, Remove Parameter)

8
Bad Smells

• Conditional Field (Extract Class, Introduce Null
  Object)
• Message Chains (Hide Delegate, Extract Method,
  Move Method)
• Middle Man (Remove Middle Man, Inline method)
• Inappropriate Intimacy (Move Method, Move Field,
  Change bi to uni assoc)

9
Bad Smell

• Alternative Classes with Different Interfaces
  (Rename Method, Move Method)
• Incomplete Library Class (Introduce Foreign
  Method, Introduce Local Extension)
• Data Class (Encapsulate Collection, Hide Method,
  Remove Setting Method)

10
Bad Smell

• Refused Bequest (Push Down Method, Replace
  Inheritance with Delegation)
• Comments (Extract Method, Rename Method,
  Introduce Assertion)

11
Video Store Example
Movie
Rental
Customer
name string
children int 2 regularint 0 newRelease
int 1 title string priceCode int
daysRentedint

statement() string
12
What about this design?
13
Refactoring

• Step 1 Make Unit Tests
• Step 2 Start to look for Bad Smells
• Step 3 Apply 1 refactoring to correct Bad Smell
• Step 4 Run Tests to ensure nothing is broken
• Step 5 Modify Unit Tests
• Return to Step 2.

14
Code
statement totalAmount freqRenterPoints result
thisAmount newline tab totalAmount
0. freqRenterPoints 0. newline String with
Character cr. tab String with Character
tab. result 'Rental Record for ' , name ,
newline. rentals do each thisAmount
0. each movie priceCode Movie regular
ifTrue thisAmount thisAmount 2.
each daysRented gt
2 ifTrue thisAmount thisAmount ((each
daysRented - 2) 1.5). .
.
each movie priceCode Movie newRelease
ifTrue thisAmount each daysRented 3. .
each movie priceCode Movie children ifTrue
thisAmount thisAmount 1.5.
each daysRented gt 3
ifTruethisAmount thisAmount ((each
daysRented - 3) 1.5)..
.
freqRenterPoints freqRenterPoints 1.
((each movie priceCode Movie newRelease) and
each daysRented gt 1 ) ifTrue
freqRenterPoints freqRenterPoints
1.. result result , tab , each movie
title, tab, thisAmount asString , newline.
totalAmount totalAmount thisAmount. .
result result , 'Amount owed is ' ,
totalAmount asString, newline. result result
, 'You earned ' , freqRenterPoints asString , '
frequent renter points'. result.
15
What is the worse bad smell?

• Long Method
• Long Class
• Data Class (Movie)
• Data Class (Rental)
• Lazy Class (Movie)
• Lazy Class (Rental)
• Divergent Change (Customer statement)
• Inappropriate Intimacy
• Feature Envy

16
Lets Refactor statement

• Apply extract method to the inside of the rental
  do method and remove the calculation of
  individual amounts
• Apply move method to move that method to the
  Rental class where it belongs.

17
Code
statement totalAmount freqRenterPoints result
thisAmount newline tab totalAmount
0. freqRenterPoints 0. newline String with
Character cr. tab String with Character
tab. result 'Rental Record for ' , name ,
newline. rentals do each thisAmount
0. each movie priceCode Movie regular
ifTrue thisAmount thisAmount 2.
each daysRented gt
2 ifTrue thisAmount thisAmount ((each
daysRented - 2) 1.5). .
.
each movie priceCode Movie newRelease
ifTrue thisAmount each daysRented 3. .
each movie priceCode Movie children ifTrue
thisAmount thisAmount 1.5.
each daysRented gt 3
ifTruethisAmount thisAmount ((each
daysRented - 3) 1.5)..
.
freqRenterPoints freqRenterPoints 1.
((each movie priceCode Movie newRelease) and
each daysRented gt 1 ) ifTrue
freqRenterPoints freqRenterPoints
1.. result result , tab , each movie
title, tab, thisAmount asString , newline.
totalAmount totalAmount thisAmount. .
result result , 'Amount owed is ' ,
totalAmount asString, newline. result result
, 'You earned ' , freqRenterPoints asString , '
frequent renter points'. result.
18
Code
statement totalAmount freqRenterPoints result
thisAmount newline tab totalAmount
0. freqRenterPoints 0. newline String with
Character cr. tab String with Character
tab. result 'Rental Record for ' , name ,
newline. rentals do each thisAmount
amountFor each.
.
freqRenterPoints freqRenterPoints 1.
((each movie priceCode Movie newRelease) and
each daysRented gt 1 ) ifTrue
freqRenterPoints freqRenterPoints
1.. result result , tab , each movie
title, tab, thisAmount asString , newline.
totalAmount totalAmount thisAmount. .
result result , 'Amount owed is ' ,
totalAmount asString, newline. result result
, 'You earned ' , freqRenterPoints asString , '
frequent renter points'. result.
amountFor each thisAmount each movie
priceCode Movie regular ifTrue thisAmount
thisAmount 2.
each daysRented gt 2 ifTrue thisAmount
thisAmount ((each daysRented - 2) 1.5). .

. each movie priceCode
Movie newRelease ifTrue thisAmount each
daysRented 3. . each movie priceCode
Movie children ifTrue thisAmount thisAmount
1.5.
each daysRented gt 3 ifTruethisAmount
thisAmount ((each daysRented - 3) 1.5)..
thisAmount
19
Code
RENTAL Class amountFor thisAmount self
movie priceCode Movie regular ifTrue
thisAmount thisAmount 2.
self daysRented gt 2 ifTrue
thisAmount thisAmount ((self daysRented -
2) 1.5). .
. self
movie priceCode Movie newRelease ifTrue
thisAmount self daysRented 3. . self
movie priceCode Movie children ifTrue
thisAmount thisAmount 1.5.
each daysRented gt 3
ifTruethisAmount thisAmount ((self
daysRented - 3) 1.5).. thisAmount

Customer gtgt statement thisAmount amountFor
each. becomes thisAmount each amountFor.
20
Code
RENTAL Class amountFor movie getCharge
daysRented MOVIE Class getCharge days
result result 0 priceCode Movie
regular ifTrue result result 2.
days gt 2 ifTrue
result result ((days - 2) 1.5). .

. priceCode Movie newRelease
ifTrue result result 3. . priceCode
Movie children ifTrue result result 1.5.
days gt 3
ifTrueresult result ((days - 3)
1.5)..

21
Code
statement totalAmount freqRenterPoints result
thisAmount newline tab totalAmount
0. freqRenterPoints 0. newline String with
Character cr. tab String with Character
tab. result 'Rental Record for ' , name ,
newline. rentals do each thisAmount
each amountFor.
.
freqRenterPoints freqRenterPoints 1.
((each movie priceCode Movie newRelease) and
each daysRented gt 1 ) ifTrue
freqRenterPoints freqRenterPoints
1.. result result , tab , each movie
title, tab, thisAmount asString , newline.
totalAmount totalAmount thisAmount. .
result result , 'Amount owed is ' ,
totalAmount asString, newline. result result
, 'You earned ' , freqRenterPoints asString , '
frequent renter points'. result.
22
Code
statement totalAmount freqRenterPoints result
thisAmount newline tab totalAmount
0. freqRenterPoints 0. newline String with
Character cr. tab String with Character
tab. result 'Rental Record for ' , name ,
newline. rentals do each thisAmount
each amountFor. freqRenterPoints
freqRenterPoints each freqRenterPoints.

. result result , tab , each movie
title, tab, thisAmount asString , newline.
totalAmount totalAmount thisAmount. .
result result , 'Amount owed is ' ,
totalAmount asString, newline. result result
, 'You earned ' , freqRenterPoints asString , '
frequent renter points'. result. Rental
Class freqRenterPoints points points
1. ((movie priceCode Movie newRelease)
and daysRented gt 1 ) ifTrue
points points 1.. points.
23
Code
Rental Class freqRenterPoints movie
freqRenterPoints daysRented Movie
Class freqRenterPoints days points
points 1. ((priceCode Movie
newRelease) and days gt 1 ) ifTrue
points points 1..
points.
24
Code
statement totalAmount freqRenterPoints result
newline tab totalAmount 0. freqRenterPoints
0. newline String with Character cr. tab
String with Character tab. result 'Rental
Record for ' , name , newline. rentals do
each freqRenterPoints
freqRenterPoints each freqRenterPoints.

result result , tab , each movie
title, tab, each amountFor asString , newline.
totalAmount totalAmount each amountFor.
. result result , 'Amount owed is ' ,
totalAmount asString, newline. result result
, 'You earned ' , freqRenterPoints asString , '
frequent renter points'. result.
25
Still a problem with Movie
Movie Class freqRenterPoints days points
points 1. ((priceCode Movie
newRelease) and days gt 1 ) ifTrue
points points 1..
points. getCharge days result result
0 priceCode Movie regular ifTrue
result result 2.
days gt 2 ifTrue result result
((days - 2) 1.5). . .

priceCode Movie newRelease ifTrue result
result 3. . priceCode Movie children
ifTrue result result 1.5.
days gt 3 ifTrueresult
result ((days - 3) 1.5)..
26
Remove Switch
Movie class price RegularPrice
new. amountFor days price getCharge
days RegularPrice class getCharge days
amount amount 2. days gt 2 ifTrue
amount amount (days -2) 1.5. amount