A Quick Overview of Code Smells and Refactoring Techniques by Martin Fowler
Refactoring is a systematic process of improving code by transforming the messy dirty code into clean code and simple design that is easy to read, understand, and maintain to fight technical debt.
Following the Best Practice Principles
- KISS: Keep It Stupid Simple
- DRY: Don’t Repeat Yourself
- SOLID: Single responsibility principle, Open–closed principle, Liskov substitution principle, Interface segregation principle, Dependency inversion principle
Build new components alongside existing ones. Plan for substitutions where possible. Test the new ones thoroughly, refactoring should be followed by regression testing. Avoid altering existing components until the end — when you may have to make alterations.
Refactorings — Composing Methods
Composing methods to package code properly, like extract method and extract class are two refactoring techniques can help reduce duplicated code
Extract Method
Takes a clump of code and turns it into its own method
//before
function printOwing(invoice) {
printBanner();
let outstanding = calculateOutstanding();
console.log(`name: ${invoice.customer}`);
console.log(`amount: ${outstanding}`);
}
//after
function printOwing(invoice) {
printBanner();
let outstanding = calculateOutstanding();
printDetails(outstanding);
function printDetails(outstanding) {
console.log(`name: ${invoice.customer}`);
console.log(`amount: ${outstanding}`);
}
}Inline Method
Take a method call and replace it with the body of the code
//before
function getRating(driver) {
return moreThanFiveLateDeliveries(driver) ? 2 : 1;
}
function moreThanFiveLateDeliveries(driver) {
return driver.numberOfLateDeliveries > 5;
}
//after
function getRating(driver) {
return (driver.numberOfLateDeliveries > 5) ? 2 : 1;
}Replace Temp with Query
To get rid of any temporary variables
//before
const basePrice = this._quantity * this._itemPrice;
if (basePrice > 1000)
return basePrice * 0.95;
else
return basePrice * 0.98;
//after
get basePrice() {this._quantity * this._itemPrice;}
if (this.basePrice > 1000)
return this.basePrice * 0.95;
else
return this.basePrice * 0.98;Split Temporary Variable
If the temp is used for many things
//before
let temp = 2 * (height + width);
console.log(temp);
temp = height * width;
console.log(temp);
//after
const perimeter = 2 * (height + width);
console.log(perimeter);
const area = height * width;
console.log(area);Remove Assignments to Parameters
If assignments are made to parameters
//before
int discount(int inputVal, int quantity) {
if (quantity > 50) {
inputVal -= 2;
}
// ...
}
//after
int discount(int inputVal, int quantity) {
int result = inputVal;
if (quantity > 50) {
result -= 2;
}
// ...
}Replace Method with Method Object
Break up even the most tangled method, at the cost of introducing a new class for the job
//before
class Order {
// ...
price(): number {
let primaryBasePrice;
let secondaryBasePrice;
let tertiaryBasePrice;
// Perform long computation.
}
}
//after
class Order {
// ...
price(): number {
return new PriceCalculator(this).compute();
}
}
class PriceCalculator {
private _primaryBasePrice: number;
private _secondaryBasePrice: number;
private _tertiaryBasePrice: number;
constructor(order: Order) {
// Copy relevant information from the
// order object.
}
compute(): number {
// Perform long computation.
}
}Substitute Algorithm
Once the method is broken down, introduce a clearer algorithm
//before
function foundPerson(people) {
for(let i = 0; i < people.length; i++) {
if (people[i] === "Don") {
return "Don";
}
if (people[i] === "John") {
return "John";
}
if (people[i] === "Kent") {
return "Kent";
}
}
return "";
}
//after
function foundPerson(people) {
const candidates = ["Don", "John", "Kent"];
return people.find(p => candidates.includes(p)) || '';
}Refactorings — Moving Features Between Objects
Where to put responsibilities
Move Method
//before
class Account {
get overdraftCharge() {...}
//after
class AccountType {
get overdraftCharge() {...}Move Field
//before
class Customer {
get plan() {return this._plan;}
get discountRate() {return this._discountRate;}
//after
class Customer {
get plan() {return this._plan;}
get discountRate() {return this.plan.discountRate;}Extract Class
If classes become bloated with too many responsibilities
//before
class Person {
get officeAreaCode() {return this._officeAreaCode;}
get officeNumber() {return this._officeNumber;}
}
//after
class Person {
get officeAreaCode() {return this._telephoneNumber.areaCode;}
get officeNumber() {return this._telephoneNumber.number;}
}
class TelephoneNumber {
get areaCode() {return this._areaCode;}
get number() {return this._number;}
}Inline Class
If a class becomes too irresponsible, merge it with another class
//before
class Person {
get officeAreaCode() {return this._telephoneNumber.areaCode;}
get officeNumber() {return this._telephoneNumber.number;}
}
class TelephoneNumber {
get areaCode() {return this._areaCode;}
get number() {return this._number;}
}
//after
class Person {
get officeAreaCode() {return this._officeAreaCode;}
get officeNumber() {return this._officeNumber;}
}Hide Delegate
If another class is being used, hide the fact
//before
manager = aPerson.department.manager;
//after
manager = aPerson.manager;
class Person {
get manager() {return this.department.manager;}Remove Middle Man
If hiding the delegate class results in constantly changing the owner’s interface
//before
manager = aPerson.manager;
class Person {
get manager() {return this.department.manager;}
//after
manager = aPerson.department.manager;Introduce Foreign Method | Introduce Local Extension
When not able to access the source code of a class, but want to move responsibilities to this unchangeable class. For one or two methods, use Introduce Foreign Method; otherwise, use Introduce Local Extension.
//before
class Report {
// ...
sendReport(): void {
let nextDay: Date = new Date(previousEnd.getYear(),
previousEnd.getMonth(), previousEnd.getDate() + 1);
// ...
}
}
//after
class Report {
// ...
sendReport() {
let newStart: Date = nextDay(previousEnd);
// ...
}
private static nextDay(arg: Date): Date {
return new Date(arg.getFullYear(), arg.getMonth(), arg.getDate() + 1);
}
}Refactorings — Organizing Data
To make working with data easier
Self Encapsulate Field
Whether an object should access its own data directly or through accessors
//before
class Range {
private int low, high;
boolean includes(int arg) {
return arg >= low && arg <= high;
}
}
//after
class Range {
private int low, high;
boolean includes(int arg) {
return arg >= getLow() && arg <= getHigh();
}
int getLow() {
return low;
}
int getHigh() {
return high;
}
}Replace Data Value with Object
Turn dumb data into articulate objects
//before
orders.filter(o => "high" === o.priority
|| "rush" === o.priority);
//after
orders.filter(o => o.priority.higherThan(new Priority("normal")))Change Value to Reference
If the above objects are needed in many parts of the program, make them into reference objects
//before
let customer = new Customer(customerData);
//after
let customer = customerRepository.get(customerData.id);Change Reference to Value
A reference object that is small, immutable, and awkward to manage can be turned into a value object
//before
class Product {
applyDiscount(arg) {this._price.amount -= arg;}
}
//after
class Product {
applyDiscount(arg) {
this._price = new Money(this._price.amount - arg, this._price.currency);
}
}Replace Array With Object
For an array acting like a data structure
//before
let row = new Array(2);
row[0] = "Liverpool";
row[1] = "15";
//after
let row = new Performance();
row.setName("Liverpool");
row.setWins("15");Replace Magic Number with Symbolic Constant
When you have a literal number with a particular meaning
//before
function potentialEnergy(mass, height) {
return mass * 9.81 * height;
}
//after
const STANDARD_GRAVITY = 9.81;
function potentialEnergy(mass, height) {
return mass * STANDARD_GRAVITY * height;
}Duplicate Observed Data
When you have domain data available only is a GUI control, and domain methods need access, copy the data to a domain object and set up an observer to synchronize the two pieces of data
Change Unidirectional Association to Bidirectional
To support a new function. (Only if necessary.) Install backpointer.
Change Bidirectional Association to Unidirectional
If the two-way link is no longer needed. Find a way to drop; consider the third party.
Encapsulate Field
If there is a public field, make it private and provide accessors
//before
let defaultOwner = {firstName: "Martin", lastName: "Fowler"};
//after
let defaultOwnerData = {firstName: "Martin", lastName: "Fowler"};
export function defaultOwner() {return defaultOwnerData;}
export function setDefaultOwner(arg) {defaultOwnerData = arg;}Encapsulate Collection
When a method returns a collection, make it return a read-only view and provide add/remove methods
//before
class Person {
get courses() {return this._courses;}
set courses(aList) {this._courses = aList;}
//after
class Person {
get courses() {return this._courses.slice();}
addCourse(aCourse) { ... }
removeCourse(aCourse) { ... }Replace Record with Data Class
When you need to interface with a record structure in a traditional programming environment, make a dumb object for the record
//before
organization = {name: "Acme Gooseberries", country: "GB"};
//after
class Organization {
constructor(data) {
this._name = data.name;
this._country = data.country;
}
get name() {return this._name;}
set name(arg) {this._name = arg;}
get country() {return this._country;}
set country(arg) {this._country = arg;}
}Replace Type Code with Class
A class has a numeric type code that does not affect its behavior, replace the number with a new class
//before
class Person {
get officeAreaCode() {return this._officeAreaCode;}
get officeNumber() {return this._officeNumber;}
//after
class Person {
get officeAreaCode() {return this._telephoneNumber.areaCode;}
get officeNumber() {return this._telephoneNumber.number;}
}
class TelephoneNumber {
get areaCode() {return this._areaCode;}
get number() {return this._number;}
}Replace Type Code with Subclasses
When you have an immutable type code that affects the behavior of a class, replace the type code with subclasses
//before
class Department {
get totalAnnualCost() {...}
get name() {...}
get headCount() {...}
}
class Employee {
get annualCost() {...}
get name() {...}
get id() {...}
}
//after
class Party {
get name() {...}
get annualCost() {...}
}
class Department extends Party {
get annualCost() {...}
get headCount() {...}
}
class Employee extends Party {
get annualCost() {...}
get id() {...}
}Replace Type Code with State/Strategy
When you have a type code that affects the behavior of a class, but you cannot use subclassing, then replace the type code with a state object
//before
function createEmployee(name, type) {
return new Employee(name, type);
}
//after
function createEmployee(name, type) {
switch (type) {
case "engineer": return new Engineer(name);
case "salesman": return new Salesman(name);
case "manager": return new Manager (name);
}Replace Subclass with Fields
When subclasses vary only in methods that return constant data, change the methods to superclass fields and eliminate the subclasses
Change Function Declaration
//before
function circum(radius) {...}
//after
function circumference(radius) {...}Collapse Hierarchy
//before
class Employee {...}
class Salesman extends Employee {...}
//after
class Employee {...}Combine Functions into Class
//before
function base(aReading) {...}
function taxableCharge(aReading) {...}
function calculateBaseCharge(aReading) {...}
//after
class Reading {
base() {...}
taxableCharge() {...}
calculateBaseCharge() {...}
}Combine Functions into Transform
//before
function base(aReading) {...}
function taxableCharge(aReading) {...}
//after
function enrichReading(argReading) {
const aReading = _.cloneDeep(argReading);
aReading.baseCharge = base(aReading);
aReading.taxableCharge = taxableCharge(aReading);
return aReading;
}Consolidate Conditional Expression
//before
if (anEmployee.seniority < 2) return 0;
if (anEmployee.monthsDisabled > 12) return 0;
if (anEmployee.isPartTime) return 0;
//after
if (isNotEligibleForDisability()) return 0;
function isNotEligibleForDisability() {
return ((anEmployee.seniority < 2)
|| (anEmployee.monthsDisabled > 12)
|| (anEmployee.isPartTime));
}Decompose Conditional
//before
if (!aDate.isBefore(plan.summerStart) && !aDate.isAfter(plan.summerEnd))
charge = quantity * plan.summerRate;
else
charge = quantity * plan.regularRate + plan.regularServiceCharge;
//after
if (summer())
charge = summerCharge();
else
charge = regularCharge();