SDSU CS 683 Emerging Technologies: Embracing Change
Spring Semester, 2001
Code Smells
Previous    Lecture Notes Index    Next    
© 2001, All Rights Reserved, SDSU & Roger Whitney
San Diego State University -- This page last updated 27-Mar-01

Contents of Doc 15, Code Smells


Refactoring: Improving the Design of Existing Code, Fowler, Addison-Wesley, 1999

More metrics (Was: Woohoo! 1000 tests!), Jim Little, XP Mailing list, 22 Mar 2001

Doc 15, Code Smells Slide # 2

Code Smells

If it stinks, change it

-- Grandma Beck on child-rearing

Some Smells

Duplicate Code
Long Method
Large Class
Long Parameter List
Divergent Change
Shotgun Surgery
Feature Envy
Data Clumps
Primitive Obsession
Switch Statements
Parallel Inheritance Hierarchies
Lazy Class
Speculative Generality
Temporary Field
Message Chains
Middle Man
Inappropriate Intimacy
Alternative Classes with Different Interfaces
Incomplete Library Class
Data Class
Refused Bequest

Doc 15, Code Smells Slide # 3

Duplicate Code[1]

Same code in two methods in same class

Try Extract method

Same code in two methods in two sibling subclass

Try Extract Method and Pull Up Field
If code is similar try:
Extract Method(110) to separate similar bits
Form Template Method(345) may help reform method
If two methods use different algorithms to do the same thing try Substitute Algorithm(139)
Same code in two unrelated classes

Try Extract Class(149)
Consider moving the code to one of the existing classes

Doc 15, Code Smells Slide # 4

Form Template Method[2]

You have two methods in subclasses that perform similar steps in the same order, yet the steps are different

Get the steps into methods with the same signature, so that the original methods become the same. Then you can pull them up


Note "_" is used to indicate an instance variable

Site subclass: #ResidentialSite

Site subclass: #LifelineSite

   | base tax |
   base := _units * _rate.
   tax := base * TaxRate.
   ^base + tax

   | base tax |
   base := _units * _rate * 0.7.
   tax := base * TaxRate * 0.2.
   ^base + tax

Doc 15, Code Smells Slide # 5
With Template Method [3]

   ^self baseAmount + self taxAmount

   ^self subclassResponsibility

   ^self subclassResponsibility

   ^self baseAmount * TaxRate

   ^_units * _rate

   ^self baseAmount * TaxRate * 0.2

   ^_units * _rate * 0.7

Doc 15, Code Smells Slide # 6

Extract Class[4]

You have one class doing the work that should be done by two

Create a new class and move the relevant fields and methods from the old class into the new class

Substitute Algorithm[5]

You want to replace an algorithm with one that is clearer

Replace the body of the method with the new algorithm

Doc 15, Code Smells Slide # 7

Long Method[6]

Metrics from XP Java Project [7]

Production code
Test code
Methods / Class
Statements / Method

Try Extract Method(110) to shorten a method

Comments often indicate code to extract from a method

Conditionals and loops provide code to extract
Decompose Conditional(238)

Too many temporary variables and/or parameters may prohibit using Extract Method

If have too many temporary variables try:
Replace Temp with Query(120)

If have long parameter list try:
Introduce Parameter Object(295)
Preserve Whole Object(288)

Doc 15, Code Smells Slide # 8

Replace Temp with Query[8]

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


   | basePrice |
   basePrice := _quantity * _itemPrice.
   basePrice > 1000
      ifTrue:[^basePrice * 0.95]
      ifFalse:[^basePrice * 0.98]

Replace with

   self basePrice > 1000
      ifTrue:[^self basePrice * 0.95]
      ifFalse:[^self basePrice * 0.98]

   ^_quantity * _itemPrice.

Doc 15, Code Smells Slide # 9

Introduce Parameter Object[9]

You have a group of parameters that naturally go together

Replace them with an object


Customer>>amountInvoicedfrom: aDate to: anotherDate

Replace with

Customer>>amountInvoicedIn: aDateRange

Doc 15, Code Smells Slide # 10

Preserve Whole Object[10]

You are getting several values from an object and passing these values as parameters in a method call

Send the whole object instead


| low high |
low := range high.
high := range low.
plan validRangeFrom: low to: high.

Replace with

plan validRange: range.

Doc 15, Code Smells Slide # 11


Comments are a good smell, but:

"Comments are often used as a deodorant" [11]

When you feel the need for a comment:

Try refactoring to make the comment unneeded

Have comment explaining a block of code:

Try Extract Method(110)

If you need a comment to explain what a method does:

Try Rename Method(273)

If you to comment on required state of the system/parameters

Try Introduce Assertion(267)

Doc 15, Code Smells Slide # 12

Rename Method[12]

The name of a method does not reveal its purpose

Change the name of the method.



Replace with


Doc 15, Code Smells Slide # 13

Long Parameter List [13]

Long parameter lists are:

Reduce parameter list by trying:

Beware of coupling when reducing parameter lists

Doc 15, Code Smells Slide # 14

Replace Parameter with Method [14]

An object invokes a method, then passes the result as a parameter for a method. The receiver can also invoke this method.

Remove the parameter and let the receiver invoke this method


   | basePrice discount |
   basePrice := _quantity * _itemPrice.
   discountLevel := self discountLevel.
   ^self discountedPriceOn: basePrice discount: discountLevel 

Replace with:

   | basePrice discount |
   basePrice := _quantity * _itemPrice.
   ^self discountedPriceOn: basePrice

discountedPriceOn: can call self discountLevel

Doc 15, Code Smells Slide # 15
[1] Fowler, pp. 76
[2] Fowler, pp. 345-351
[3] Can you find a better Template Method?
[4] Fowler, pp. 149-153
[5] Fowler, pp. 139-140
[6] Fowler, .pp. 76-77
[7] Jim Little
[8] Fowler, pp. 120-123
[9] Fowler, pp. 295-299
[10] Fowler, pp. 288-291
[11] Fowler pp. 87
[12] Fowler pp. 273-274
[13] Fowler pp. 78-79
[14] Fowler pp. 292-294

Copyright ©, All rights reserved.
2001 SDSU & Roger Whitney, 5500 Campanile Drive, San Diego, CA 92182-7700 USA.
OpenContent license defines the copyright on this document.

Previous    visitors since 27-Mar-01    Next