Refactoring and code smells

Why do good developers write bad software?

• Requirements change over time, making it hard to update your code
(leading to less optimal designs)
• Time and money cause you to take shortcuts
• You learn a better way to do something (the second time you paint a
room, it’s always better than the first because you learned during the
first time!)

Two questions:
1.How do we fix our software?
2.How do we know our software is “bad”… when it works fine!
Coming up: Refactoring
Refactoring
• Definition: Refactoring modifies software to improve
its readability, maintainability, and extensibility
without changing what it actually does.
• External behavior does NOT change
• Internal structure is improved

Coming up: Refactoring

Refactoring

• The goal of refactoring is NOT to add new

functionality
• The goal is refactoring is to make code easier to
maintain in the future

Coming up: Refactoring Simple Example

The refactoring environment
• Traditional software engineering is modeled after traditional engineering
practices (= design first, then code)
• Assumptions:
• The desired end product can be determined in advance
• Workers of a given type (plumbers, electricians, etc.) are interchangeable
• Agile software engineering is based on different assumptions:
• Requirements (and therefore design) change as users become acquainted with the
software
• Programmers are professionals with varying skills and knowledge
• Programmers are in the best position for making design decisions
• Refactoring is fundamental to agile programming
• Refactoring is sometimes necessary in a traditional process, when the design is
found to be flawed
Refactoring Process
• Make a small change
• a single refactoring
• Run all the tests to ensure everything still works
• If everything works, move on to the next refactoring
• If not, fix the problem, or undo the change, so you still have a working
system
Refactoring Simple Example
Move a method:
Motivation: A method is, or will be, using or used by more features of another class than the class
on which it is defined.
Technique: Create a new method with a similar body in the class it uses most. Either turn the old
method into a simple delegation, or remove it altogether.

Coming up: Danger!

Danger!
• Refactoring CAN introduce problems, because anytime you modify
software you may introduce bugs!
• Management thus says:
• Refactoring adds risk!
• It’s expensive – we’re spending time in development, but not “seeing” any
external differences? And we still have to retest?

• Why are we doing this?

Coming up: Motivation

Motivation
• We refactor because we understand getting the design right the first
time is hard and you get many benefits from refactoring:

• Code size is often reduced

• Confusing code is restructured into simpler code
• Both of these greatly improve mainatainability! Which is required because
requirements always change!

Coming up: Refactoring: Complex Example

Refactoring: Complex Example
Introduce Null Object
Motivation: You have many checks for null
Technique: Replace the null value with a null object.

Customer c = findCustomer(...);
...
if (customer == null) {
name = “occupant”
} else {
name = customer.getName()
}

if (customer == null) {
…

Coming up: Refactoring: Complex Example

 Refactoring: Complex Example
public class NullCustomer extends Customer {

public String getName() {

return “occupant”
}

------------------------------------------------------------

Customer c = findCustomer()
name = c.getName()

Completely eliminated the if statement by replacing

checks for null with a null object that does the right
thing for “null” values.
Coming up: Refactoring Example: Replace Conditional with
Polymorphism
 Refactoring Example: Replace Conditional with
Polymorphism

Motivation: You have a conditional that chooses different behavior depending on the type of
an object.
Technique: Move each leg of the conditional to an overriding method in a subclass. Make
the original method abstract.

double getSpeed() {
switch (_type) {
case EUROPEAN:
return getBaseSpeed();
case AFRICAN:
return getBaseSpeed() - getLoadFactor() * _numberOfCoconuts;
case NORWEGIAN_BLUE:
return (_isNailed) ? 0 : getBaseSpeed(_voltage);
} throw new RuntimeException ("Should be unreachable");
}

Coming up: Refactoring Example: Replace Conditional with

Polymorphism
Refactoring Example: Replace Conditional with
Polymorphism

Coming up: When do I refactor?

When do I refactor?
• When you add functionality
• Before you add new features, make sure your design and current code is
“good” this will help the new code be easier to write
• When you need to fix a bug
• When you do a peer review

Coming up: How do I identify code to refactor?

 How do I identify code to refactor?

• Martin Fowler uses “code smells” to identify when to refactor.

• Code smells are bad things done in code, somewhat like bad patterns
in code

• Many people have tied code smells to the specific refactorings to fix
the smell

Coming up: Code Smells

Code Smells
• Duplicated Code
• bad because if you modify one instance of duplicated code but not the
others, you (may) have introduced a bug!
• Long Method
• long methods are more difficult to understand
• performance concerns with respect to lots of short methods are largely
obsolete

Coming up: Code Smells

Code Smells
• Large Class
• classes try to do too much, which reduces cohesion
• Long Parameter List
• hard to understand, can become inconsistent
• Divergent Change
• Related to cohesion: symptom: one type of change requires changing one
subset of methods; another type of change requires changing another
subset

Coming up: Code Smells

Code Smells
• Lazy Class
• A class that no longer “pays its way”
• e.g. may be a class that was downsized by a previous
refactoring, or represented planned functionality that did not
pan out
• Speculative Generality
• “Oh I think we need the ability to do this kind of thing
someday”
• Temporary Field
• An attribute of an object is only set in certain
circumstances; but an object should need all of its
attributes
Coming up: Code Smells
Code Smells
• Data Class
• These are classes that have fields, getting and setting
methods for the fields, and nothing else; they are data
holders, but objects should be about data AND behavior
• Refused Bequest
• A subclass ignores most of the functionality provided by
its superclass
• Subclass may not pass the “IS-A” test
• Comments (!)
• Comments are sometimes used to hide bad code
• “…comments often are used as a deodorant” (!)

Coming up: Many more code smells

Many more code smells
• Many more smells at:
• http://c2.com/cgi/wiki?CodeSmell

• Given a smell, what refactorings are likly?

• http://wiki.java.net/bin/view/People/SmellsToRefactorings

Coming up: Why use them?

SMELLS EXAMPLE – See which smells you find in the sample code
Duplicated Code
Long Method
Large Class
Long Parameter List
Divergent Change- Related to cohesion: symptom: one type of change requires changing one
subset of methods; another type of change requires changing another subset
Data Class - These are classes that have fields, getting and setting methods for the fields, and
nothing else; they are data holders, but objects should be about data AND behavior
Refused Bequest - A subclass ignores most of the functionality provided by its superclass
Comments (!) - Comments are sometimes used to hide bad code
Lazy Class - A class that no longer “pays its way”
Speculative Generality - “Oh I think we need the ability to do this kind of thing someday”
Temporary Field - An attribute of an object is only set in certain circumstances; but an object
should need all of its attributes
Why use them?
• Code smells and refactoring are techniques to help you discover
problems in design and implementation and apply known solutions to
these problems

• Should they be used all the time? You should always think about
them, but only apply them when they make sense… sometimes you
need a long method… but think about it to make sure!

Coming up: Adding safety

Adding safety
• Remember that making these changes incurs some risk of introducing
bugs!
• To reduce that risk
• You must test constantly – using automated tests wherever possible
• Use refactoring patterns – I’ve shown you two… there are more.. many more!
• http://www.refactoring.com/catalog/index.html
• Use tools! Netbeans and Eclipse both support basic refactoring
(http://wiki.netbeans.org/Refactoring)

Coming up: Question from your boss

Question from your boss
"Refactoring is an overhead activity - I'm paid to write new, revenue generating features."
• Tools/technologies are now available to allow refactoring to be done quickly and relatively
painlessly.
• Experiences reported by some object-oriented programmers suggest that the overhead of
refactoring is more than compensated by reduced efforts and intervals in other phases of
program development.
• While refactoring may seem a bit awkward and an overhead at first, as it becomes part of a
software development regimen, it stops feeling like overhead and starts feeling like an essential.

Coming up: Summary

Quoted from: http://st-www.cs.illinois.edu/users/opdyke/wfo.990201.refac.html
Summary
• Refactoring improves the design of software
• without refactoring, a design will “decay” as people make
changes to a software system
• Refactoring makes software easier to understand
• because structure is improved, duplicated code is
eliminated, etc.
• Refactoring helps you find bugs
• Refactoring promotes a deep understanding of the code at
hand, and this understanding aids the programmer in
finding bugs and anticipating potential bugs
• Refactoring helps you program faster
• because a good design enables progress

Coming up: References

References
• Comic from:
http://www.irregularwebcomic.net/cgi-bin/comic.pl?comic=687
• http://www.refactoring.com/catalog/index.html
• http://www.cs.colorado.edu/~kena/classes/6448/s05/lectures/lectur
e18.pdf
• http://www.codeproject.com/KB/architecture/practicalexp.aspx

Coming up: In-Class Exercise

Example 1: switch statements
• switch statements are very rare in properly designed object-oriented
code
• Therefore, a switch statement is a simple and easily detected “bad smell”
• Of course, not all uses of switch are bad
• A switch statement should not be used to distinguish between various kinds
of object
• There are several well-defined refactorings for this case
• The simplest is the creation of subclasses
Example 1, continued
• class Animal {
final int MAMMAL = 0, BIRD = 1, REPTILE = 2;
int myKind; // set in constructor
...
String getSkin() {
switch (myKind) {
case MAMMAL: return "hair";
case BIRD: return "feathers";
case REPTILE: return "scales";
default: return “skin";
}
}
}
Example 1, improved
class Animal {
String getSkin() { return “skin"; }
}
class Mammal extends Animal {
String getSkin() { return "hair"; }
}
class Bird extends Animal {
String getSkin() { return "feathers"; }
}
class Reptile extends Animal {
String getSkin() { return "scales"; }
}
How is this an improvement?
• Adding a new animal type, such as Amphibian, does not require
revising and recompiling existing code
• Mammals, birds, and reptiles are likely to differ in other ways, and
we’ve already separated them out (so we won’t need more switch
statements)
• We’ve gotten rid of the flags we needed to tell one kind of animal
from another
• We’re now using Objects the way they were meant to be used
Example 2: Encapsulate Field
• Un-encapsulated data is a no-no in OO application design. Use property get and
set procedures to provide public access to private (encapsulated) member
variables.
public class Course
{
private List students;
public class Course
public List getStudents()
{
{
public List students;
return students;
}
}
public void setStudents(List s)
{
students = s;
int classSize = course.students.size(); }
}

int classSize = course.getStudents().size();

Encapsulating Fields
• I have a class with 10 fields. This is a pain to set up for each one.
• Refactoring Tools
• See NetBeans/Visual Studio refactoring examples

• Also:
• Rename Method
• Change Method Parameters
3. Extract Class
• Break one class into two, e.g. Having the phone details as part of the Customer class is not a
realistic OO model, and also breaks the Single Responsibility design principle. We can refactor this
into two separate classes, each with the appropriate responsibility.

public class Customer

{
public class Customer
private String name;
{
private Phone workPhone;
private String name;
}
private String workPhoneAreaCode;
private String workPhoneNumber;
public class Phone
}
{
private String areaCode;
private String number;
}
4. Extract Interface
• Extract an interface from a class. Some clients may need to know a Customer’s
name, while others may only need to know that certain objects can be serialized to
XML. Having toXml() as part of the Customer interface breaks the Interface
Segregation design principle which tells us that it’s better to have more specialized
interfaces than to have one multi-purpose interface.

public class Customer implements SerXML

{
private String name;
public class Customer
{ public String getName(){ return name; }
private String name;
public void setName(String string)
public String getName(){ return name; } { name = string; }

public void setName(String string) public String toXML()

{ name = string; } { return "<Customer><Name>" +
name + "</Name></Customer>";
public String toXML() }
{ return "<Customer><Name>" + }
name + "</Name></Customer>";
} public interface SerXml {
} public abstract String toXML();
}
5. Extract Method
• Sometimes we have methods that do too much. The more code in a single
method, the harder it is to understand and get right. It also means that logic
embedded in that method cannot be reused elsewhere. The Extract Method
refactoring is one of the most useful for reducing the amount of duplication in
code.

public class Customer

{
public class Customer void int foo()
{ {
void int foo() …
{ score = ComputeScore(a,b,c,xfactor);
… }
// Compute score
score = a*b+c; int ComputeScore(int a, int b, int c, int x)
score *= xfactor; {
} return (a*b+c)*x;
} }
}
6. Extract Subclass
• When a class has features (attributes and methods) that would only be useful in specialized
instances, we can create a specialization of that class and give it those features. This makes the
original class less specialized (i.e., more abstract), and good design is about binding to
abstractions wherever possible.

public class Person

{
protected String name;
public class Person }
{
private String name; public class Employee extends Person
private String jobTitle; {
} private String jobTitle;
}
7. Extract Super Class
• When you find two or more classes that share common features, consider abstracting those
shared features into a super-class. Again, this makes it easier to bind clients to an abstraction, and
removes duplicate code from the original classes.

public abstract class Person

{
public class Employee protected String name;
{ }
private String name;
private String jobTitle; public class Employee extends Person
} {
private String jobTitle;
public class Student }
{
private String name; public class Student extends Person
private Course course; {
} private Course course;
}
8. Form Template Method - Before
• When you find two methods in subclasses that perform the same steps, but do different things in
each step, create methods for those steps with the same signature and move the original method
into the base class

public abstract class Party { } public class Company extends Party

{
private String name;
private String companyType;
public class Person extends Party private Date incorporated;
{ public void PrintNameAndDetails()
private String firstName; {
private String lastName; System.out.println("Name: " + name + " " + companyType);
private Date dob; System.out.println("Incorporated: " + incorporated.toString());
private String nationality; }
public void printNameAndDetails() }
{
System.out.println("Name: " + firstName + " " + lastName);
System.out.println("DOB: " + dob.toString() + ", Nationality: " + nationality);
}
}
 Form Template Method - Refactored
public abstract class Party
{
public class Company extends Party
public void PrintNameAndDetails()
{
{
private String name;
printName();
private String companyType;
printDetails();
private Date incorporated;
}
public void printDetails()
public abstract void printName();
{
public abstract void printDetails();
System.out.println("Incorporated: " + incorporated.toString());
}
}
public void printName()
public class Person extends Party
{
{
private String firstName; System.out.println("Name: " + name + " " + companyType);
private String lastName; }
private Date dob; }
private String nationality;
public void printDetails()
{
System.out.println("DOB: " + dob.toString() + ", Nationality: " + nationality);
}
public void printName()
{
System.out.println("Name: " + firstName + " " + lastName);
}
}
9. Move Method - Before
• If a method on one class uses (or is used by) another class more than the class on which its
defined, move it to the other class

public class Student

{
public boolean isTaking(Course course)
{
return (course.getStudents().contains(this));
}
}

public class Course

{
private List students;
public List getStudents()
{
return students;
}
}
Move Method - Refactored
• The student class now no longer needs to know about the Course interface, and the isTaking()
method is closer to the data on which it relies - making the design of Course more cohesive and
the overall design more loosely coupled

public class Student

{
}

public class Course

{
private List students;
public boolean isTaking(Student student)
{
return students.contains(student);
}
}
10. Introduce Null Object
• If relying on null for default behavior, use inheritance instead

public class User

public class User {
{ Plan getPlan()
Plan getPlan() {
{ return plan;
return plan; }
} }
}
public class NullUser extends User
{
if (user == null) Plan getPlan()
plan = Plan.basic(); {
else return Plan.basic();
plan = user.getPlan(); }
}
11. Replace Error Code with Exception
• A method returns a special code to indicate an error is better accomplished with
an Exception.
int withdraw(int amount) void withdraw(int amount)
{ throws BalanceException
if (amount > balance) {
return -1; if (amount > balance)
else { {
balance -= amount; throw new BalanceException();
return 0; }
} balance -= amount;
} }
12. Replace Exception with Test
• Conversely, if you are catching an exception that could be handled by an if-
statement, use that instead.
double getValueForPeriod (int periodNumber)
{
try
{
return values[periodNumber];
}
catch (ArrayIndexOutOfBoundsException e)
{
return 0;
}
} double getValueForPeriod (int periodNumber)
{
if (periodNumber >= values.length) return 0;
return values[periodNumber];
}
13. Nested Conditional with Guard
• A method has conditional behavior that does not make clear what the
normal path of execution is. Use Guard Clauses for all the special cases.

double getPayAmount() {
double result;
if (isDead) result = deadAmount();
else {
if (isSeparated) result = separatedAmount();
else {
if (isRetired) result = retiredAmount();
else result = normalPayAmount();
}
}
return result;
}

double getPayAmount() {
if (isDead) return deadAmount();
if (isSeparated) return separatedAmount();
if (isRetired) return retiredAmount();
return normalPayAmount();
};
14. Replace Parameter with Explicit Method
• You have a method that runs different code depending on the values of an enumerated
parameter. Create a separate method for each value of the parameter.

void setValue (String name, int value) {

if (name.equals("height")) {
void setHeight(int arg)
height = value;
{
return;
height = arg;
}
}
if (name.equals("width")) {
width = value;
void setWidth (int arg)
return;
{
}
width = arg;
Assert.shouldNeverReachHere();
}
}
15. Replace Temp with Query
• You are using a temporary variable to hold the result of an expression. Extract the expression into
a method. Replace all references to the temp with the expression. The new method can then be
used in other methods and allows for other refactorings.

double basePrice = quantity * itemPrice;

if (basePrice > 1000)
return basePrice * 0.95;
else
return basePrice * 0.98;

if (basePrice() > 1000)

return basePrice() * 0.95;
else
return basePrice() * 0.98;
...
double basePrice() {
return quantity * itemPrice;
}
16. Rename Variable or Method
• Perhaps one of the simplest, but one of the most useful that bears repeating: If
the name of a method or variable does not reveal its purpose then change the
name of the method or variable.

public class Customer public class Customer

{ {
public double getinvcdtlmt(); public double getInvoiceCreditLimit();
} }
More on Refactorings
• Refactoring Catalog
• http://www.refactoring.com/catalog
• Java Refactoring Tools
• NetBeans 4+ – Built In
• JFactor – works with VisualAge and JBuilder
• RefactorIt – plug-in tool for NetBeans, Forte, JBuilder and JDeveloper. Also works standalone.
• JRefactory – for jEdit, NetBeans, JBuilder or standalone
• Visual Studio 2005+
• Refactoring Built In
• Encapsulate Field, Extract Method, Extract Interface, Reorder Parameters, Remove Parameter,
Promote Local Var to Parameter, more.
Refactoring Exercise
• Refactor the Trivia Game code
import java.util.ArrayList;

public class TriviaData

{
private ArrayList<TriviaQuestion> data;

public TriviaData()
{
data = new ArrayList<TriviaQuestion>();
}

public void addQuestion(String q, String a, int v, int t)

{
TriviaQuestion question = new TriviaQuestion(q,a,v,t);
data.add(question);
}

public void showQuestion(int index)

{
TriviaQuestion q = data.get(index);
System.out.println("Question " + (index +1) + ". " + q.value + " points.");
if (q.type == TriviaQuestion.TRUEFALSE)
{
System.out.println(q.question);
System.out.println("Enter 'T' for true or 'F' for false.");
}
else if (q.type == TriviaQuestion.FREEFORM)
{
System.out.println(q.question);
}
}
TriviaData.java TriviaQuestion.java
public int numQuestions()
{
return data.size();
}

public TriviaQuestion getQuestion(int index)

{
return data.get(index);
}
}

public class TriviaQuestion

{
public static final int TRUEFALSE = 0;
public static final int FREEFORM = 1;

public String question; // Actual question

public String answer; // Answer to question
public int value; // Point value of question
public int type; // Question type, TRUEFALSE or FREEFORM

public TriviaQuestion()
{
question = "";
answer = "";
value = 0;
type = FREEFORM;
}

public TriviaQuestion(String q, String a, int v, int t)

{
question = q;
answer = a;
value = v;
type = t;
}
}
TriviaGame.java
import java.io.*;
import java.util.Scanner;

public class TriviaGame

{
public TriviaData questions; // Questions

public TriviaGame()
{
// Load questions
questions = new TriviaData();
questions.addQuestion("The possession of more than two sets of chromosomes is termed?",
"polyploidy", 3, TriviaQuestion.FREEFORM);
questions.addQuestion("Erling Kagge skiied into the north pole alone on January 7, 1993.",
"F", 1, TriviaQuestion.TRUEFALSE);
questions.addQuestion("1997 British band that produced 'Tub Thumper'",
"Chumbawumba", 2, TriviaQuestion.FREEFORM);
questions.addQuestion("I am the geometric figure most like a lost parrot",
"polygon", 2, TriviaQuestion.FREEFORM);
questions.addQuestion("Generics were introducted to Java starting at version 5.0.",
"T", 1, TriviaQuestion.TRUEFALSE);
}
TriviaGame.java
// Main game loop
public static void main(String[] args)
{
int score = 0; // Overall score
int questionNum = 0; // Which question we're asking
TriviaGame game = new TriviaGame();
Scanner keyboard = new Scanner(System.in);
// Ask a question as long as we haven't asked them all
while (questionNum < game.questions.numQuestions())
{
// Show question
game.questions.showQuestion(questionNum);
// Get answer
String answer = keyboard.nextLine();
// Validate answer
TriviaQuestion q = game.questions.getQuestion(questionNum);
if (q.type == TriviaQuestion.TRUEFALSE)
{
if (answer.charAt(0) == q.answer.charAt(0))
{
System.out.println("That is correct! You get " + q.value + " points.");
score += q.value;
}
else
{
System.out.println("Wrong, the correct answer is " + q.answer);
}
}
TriviaGame.java
else if (q.type == TriviaQuestion.FREEFORM)
{
if (answer.toLowerCase().equals(q.answer.toLowerCase()))
{
System.out.println("That is correct! You get " + q.value + " points.");
score += q.value;
}
else
{
System.out.println("Wrong, the correct answer is " + q.answer);
}
}
System.out.println("Your score is " + score);
questionNum++;
}
System.out.println("Game over! Thanks for playing!");
}
}