IN THE MIDDLE OF PROGRESS
- This summary provides a concise overview of the key concepts and principles presented in the book, focusing on the art of refactoring code to improve its design and maintainability.
- With the inclusion of qualified code examples, you'll gain a deeper understanding of how to improve the readability and maintainability of your codebase.
- The structure of this summary draws inspiration from Hugo Matilla's work.
- I. Bad Smells in Code
- 1. Mysterious Name
- 2. Duplicated code
- 3. Long Function
- 4. Long Parameter List
- 5. Global Data
- 6. Mutable Data
- 7. Divergent Change
- 8. Shotgun Surgery
- 9. Feature Envy
- 10. Data Clumps
- 11. Primitive Obsession
- 12. Repeated Switches
- 13. Loops
- 14. Lazy Element
- 15. Speculative Generality
- 16. Temporary Field
- 17. Message Chain
- 18. Middle Man
- 19. Insider Trading
- 20. Large Classes
- 21. Alternative Classes with Different Interfaces
- 22. Data Class
- 23. Refused Bequest
- 24. Comments
- II. A First Set of Refactorings
- 1. II Overview
- 2. Function Extraction or Inlining
- 3. Variable Extraction or Inlining
- 4. Changing Function Declaration
- 5. Encapsulation of Variables
- 6. Renaming Variables
- 7. Creation of Parameter Objects
- 8. Grouping Multiple Functions into a Class
- 9. Grouping Multiple Functions into a Transformation Function
- III. Encapsulation
- IV. Moving Features
- V. Organizing Data
- VI. Simplifying Conditional Logic
- VII. Refactoring APIs
- VIII. Dealing with Inheritance
Code names lack both simplicity and clarity
The same code snippet exists in multiple places.
fun calculateSalesTax(amount: Double): Double {
return amount * 0.08
}
fun calculateServiceTax(amount: Double): Double {
return amount * 0.08
}
There is overly long logic in a single function.
fun calculateTotal(cart: List<Product>): Double{
var total = 0.0
// caculate sub total
for (product in cart) {
total += product.price
}
// apply discount
if (total > 100) {
total -= total * 0.1
}
// add sales tax
val tax = total * 0.08
total += tax
return total
}
There are too many parameters in the function signature.
There is global data accessible throughout the codebase.
There is mutable data with potential side effects due to changes.
One object is frequently modified due to multiple concerns.
data class Address(var street: String, var city: String, var state: String, var zip: String)
data class Order(var id: Int, var product: String, var quantity: Int)
class UserProfile {
var username: String = ""
var email: String = ""
var address: Address = Address("", "", "", "")
var orders: MutableList<Order> = mutableListOf()
// user profile management
fun changeUsername(newUsername: String) {
username = newUsername
}
// address management
fun changeAddress(newAddress: Address) {
address = newAddress
}
// order management
fun placeOrder(newOrder: Order) {
orders.add(newOrder)
}
}
The same code snippet is scattered across multiple concerns.
class OrderConfirmation {
fun sendConfirmationEmail(order: Order) {
// Send order confirmation email
}
}
class OrderHistory {
fun sendConfirmationEmail(order: Order) {
// Send order confirmation email
}
}
class OrderStatus {
fun sendConfirmationEmail(order: Order) {
// Send order confirmation email
}
}
Some attributes of an object are frequently used by other objects.
class Customer(
val name: String,
val age: Int,
val address: Address
)
class Order(
val customer: Customer,
val items: List<Item>
) {
fun calculateTotal(): Double { /*...*/ }
fun printCustomerDetails() {
println("Customer Name: ${customer.name}")
println("Customer Age: ${customer.age}")
println("Customer Address: ${customer.address}")
}
}
There are combinations of data that are used together in most cases.
class Order(
private val orderNumber: String,
private val customerName: String,
private val customerEmail: String,
private val customerAddress: Address,
private val items: List<Item>
)
class Address(
val street: String,
val city: String,
val state: String,
val zipCode: String
)
class Item(
val name: String,
val price: Double
)
Primitive types are used instead of defining objects that meet the requirements.
data class User(
val name: String,
val email: String,
val phoneNumber: String
)
When adding a new case, associated cases need to be added everywhere switch statements exist.
enum class AnimalType {
DOG, CAT, BIRD
}
class Animal(val type: AnimalType) {
fun makeSound() {
when (type) {
AnimalType.DOG -> println("Woof!")
AnimalType.CAT -> println("Meow!")
AnimalType.BIRD -> println("Tweet!")
}
}
fun eat() {
when (type) {
AnimalType.DOG -> println("Dog is eating...")
AnimalType.CAT -> println("Cat is eating...")
AnimalType.BIRD -> println("Bird is eating...")
}
}
}
There are loops that are difficult to understand the intention of the code.
fun sumOfPositiveNumbers(numbers: List<Int>): Int {
var total = 0
for (number in numbers) {
if (number > 0) {
total += number
}
}
return total
}
There are objects defined with weak functionality.
class UserName(val name: String)
fun printUserName(userName: UserName) {
println(userName.name)
}
val user = UserName("John Doe")
printUserName(user) // Outputs: John Doe
There are cases where implementations are done in advance based on assumptions.
interface PaymentMethod {
fun processPayment(amount: Double)
}
class CreditCardPayment : PaymentMethod {
override fun processPayment(amount: Double) {
// process credit card payment
}
}
class BitcoinPayment : PaymentMethod {
override fun processPayment(amount: Double) {
// process bit coin payment
}
}
There are variables that are only used in specific situations.
class Shopping {
var eventDiscount: Double? = null
fun getTotalPrice(items: List<Item>): Double {
var total = items.sumOf { it.price }
if (eventDiscount != null) {
total -= total * eventDiscount!!
}
return total
}
}
There is a structure where a client requests one object, which then requests another object.
class Order(val customer: Customer)
class Customer(val address: Address)
class Address(val city: City)
class City(val name: String)
// Client code
val order = Order(Customer(Address(City("New York"))))
val cityName = order.customer.address.city.name
An object is unable to fulfill its own responsibilities, and delegates responsibility to another object.
class Order(val totalCost: Double)
class PaymentProcessor {
fun processPayment(order: Order, paymentMethod: PaymentMethod) {
paymentMethod.pay(order.totalCost)
}
}
interface PaymentMethod {
fun pay(amount: Double)
}
class CreditCard : PaymentMethod {
override fun pay(amount: Double) {
// payment logic
}
}
// Client code
val order = Order(100.0)
val paymentMethod = CreditCard()
PaymentProcessor.processPayment(order, paymentMethod)
An object is excessively coupled with the internals of another object.
class Trader(private val stockMarket: StockMarket) {
fun buy(stock: Stock, quantity: Int): Double {
// needs to know about the inner workings of StockMarket to make a trade.
if (!stockMarket.stocks.contains(stock)) {
throw RuntimeException("Stock not available in the market.")
}
val cost = stock.price * quantity
stock.price += quantity // directly manipulates the price of Stock
return cost
}
}
class Stock(var price: Double)
class StockMarket {
var stocks: MutableList<Stock> = mutableListOf()
fun addStock(stock: Stock) {
stocks.add(stock)
}
}
An Class has too many responsibilities.
class Trader(var name: String, var address: String, var phoneNumber: String, var accountBalance: Double) {
//...
fun buy(stock: Stock, quantity: Int) {
// buy logic
}
fun sell(stock: Stock, quantity: Int) {
// sell logic
}
fun updateAddress(newAddress: String) {
this.address = newAddress
}
fun updatePhoneNumber(newPhoneNumber: String) {
this.phoneNumber = newPhoneNumber
}
fun deposit(amount: Double) {
this.accountBalance += amount
}
fun withdraw(amount: Double) {
if (this.accountBalance < amount) {
throw IllegalArgumentException("Insufficient balance!")
}
this.accountBalance -= amount
}
//...
}
Classes perform similar functions on a large scale, but have different interfaces.
class StockTrader {
fun purchaseStock(stock: Stock, quantity: Int) {
// buy logic
}
fun sellStock(stock: Stock, quantity: Int) {
// sell logic
}
}
class CryptoTrader {
fun acquireCrypto(crypto: Crypto, quantity: Int) {
// buy logic
}
fun disposeCrypto(crypto: Crypto, quantity: Int) {
// sell logic
}
}
Attributes are manipulated in data class.
// Data Class
data class Image(var width: Int, var height: Int, val format: String) {
fun resize() {
// resize width and height
}
}
A child object does not use some of the methods of its parent object.
open class Vehicle {
open fun startEngine() {
// start engine
}
open fun move() {
// move
}
open fun stop() {
// stop
}
}
class Car: Vehicle() {
override fun startEngine() {
// start car engine
}
override fun move() {
// car moves
}
override fun stop() {
// car stops
}
}
class Bicycle: Vehicle() {
override fun startEngine() {
throw UnsupportedOperationException("Bicycles don't have engines")
}
override fun move() {
// bicycle moves
}
override fun stop() {
// bicycle stops
}
}
Comments are added because the code itself cannot sufficiently explain the business logic.
class OrderProcessing {
fun processOrder(order: Order) {
// validate order
if (order.items.isEmpty()) {
throw IllegalArgumentException("Order cannot be empty")
}
if (order.customer == null) {
throw IllegalArgumentException("Customer information is required")
}
// calculate total
var total = 0.0
for (item in order.items) {
total += item.price
}
// apply discount
if (order.customer.isPremium) {
total *= 0.9 // 10% discount for premium customers
}
// create invoice
val invoice = Invoice(order.customer, total)
// send invoice
sendInvoice(invoice)
}
private fun sendInvoice(invoice: Invoice) {
// send the invoice
}
}
graph TD
stage1[Stage 1: Compose Functions and Naming]
Function(Function Extraction or Inlining)
Variable(Variable Extraction or Inlining)
ChangeFunctionDeclaration(Change Function Declaration)
ChangeVariableName(Change Variable Name)
VariableEncapsulation(Variable Encapsulation)
CreateParameterObject(Create Parameter Object)
stage2[Stage 2: Grouping Functions]
GroupFunctionsInClass(Group Functions in Class)
GroupFunctionsInTransformFunction(Group Functions in Transform Function)
stage3[Stage 3: Splitting Steps]
stage1 --> Function --> stage2
stage1 --> Variable --> stage2
stage1 --> ChangeFunctionDeclaration --> stage2
stage1 --> ChangeVariableName --> stage2
stage1 --> VariableEncapsulation --> stage2
stage1 --> CreateParameterObject --> stage2
stage2 --> GroupFunctionsInClass --> stage3
stage2 --> GroupFunctionsInTransformFunction --> stage3
Extract based on the separation of 'purpose' and 'implementation'
If the code contains the implementation, it is extracted into a function. Conversely, if the extracted function contains the purpose, it is inlined.
It is not important if the function name is longer than the function body or if the function body is too short.
-
Function Extraction Example
class OrderService() { fun placeOrder(order: Order) val totalPrice = order.items.sumOf { item -> item.price } var finalPrice = if (order.items.size >= 10) totalPrice * 0.9 else totalPrice order.status = Status.COMPLETED order.totalPrice = finalPrice } }
to
class OrderService() { fun placeOrder(order: Order) { val totalPrice = order.items.sumOf { item -> item.price } val finalPrice = applyDiscounts(totalPrice) finalizeOrder(finalPrice) } private fun applyDiscounts(totalPrice: Double): Double { return if (order.items.size >= 10) totalPrice * 0.9 else totalPrice } private fun finalizeOrder(finalPrice: Double) { order.status = Status.COMPLETED order.totalPrice = finalPrice } }
-
Function Inlining Example
class OrderService() { fun placeOrder(order: Order) { val totalPrice = calculateTotal() val finalPrice = applyDiscounts(totalPrice) finalizeOrder(finalPrice) } // ...omitted }
to
class OrderService() { fun placeOrder(order: Order) { val totalPrice = order.items.sumOf { item -> item.price } val finalPrice = applyDiscounts(totalPrice) finalizeOrder(finalPrice) } // ...omitted }
Again extract based on the separation of 'purpose' and 'implementation'
If the code to be extracted has meaning outside of the function, it should be extracted into a function rather than a variable.
-
Variable Extraction Example
private fun applyDiscounts(totalPrice: Double): Double { return if (order.items.size >= 10) totalPrice * 0.9 else totalPrice }
to
private fun applyDiscounts(totalPrice: Double): Double { val isDiscountApplicable = order.items.size >= 10 val discountRate = 0.9 return if (isDiscountApplicable) totalPrice * discountRate else totalPrice }
-
Variable Inlining Example
private fun applyDiscounts(totalPrice: Double): Double { val itemSize = order.items.size val discountRate = 0.9 return if (itemSize >= 10) totalPrice * discountRate else totalPrice }
to
private fun applyDiscounts(totalPrice: Double): Double { val discountRate = 0.9 return if (order.items.size >= 10) totalPrice * discountRate else totalPrice }
Declare the name and parameters of the function in line with its 'purpose'
When it comes to the name, it should be defined according to the purpose it serves where it's called, not based on its internal implementation.
As for parameters, you should decide whether to deliver only essential values to increase utilization, or to pass associated objects to raise the level of encapsulation.
-
Example of Changing Function Name
class Bank() { fun calculateInterestByAccountType(account: Account): Double { val rate = if (account.type == AccountType.CHECKING) 0.01 else 0.02 return account.balance * rate } }
to
class Bank() { fun calculateInterest(account: Account): Double { val rate = if (account.type == AccountType.CHECKING) 0.01 else 0.02 return account.balance * rate } }
-
Example of Changing Parameters
fun applyDiscount(totalPrice: Double, numberOfItems: Int): Double { val discountRate = 0.9 return if (numberOfItems >= 10) totalPrice * discountRate else totalPrice }
to
fun applyDiscount(order: Order): Double { val discountRate = 0.9 return if (order.items.size >= 10) order.totalPrice * discountRate else order.totalPrice }
Encapsulate mutable data with broad scope for access and updates
In flexible programming environments, there's a high likelihood of mutable data with a wide range of validity across the code. In OOP environments, mutable data that goes beyond the class is not tolerated.
Name the variables so that their 'purpose' can be clearly understood within the context
If understandable within context, it's okay to name variables with a single character. For persistent data stored in the DB, more attention should be paid to the variable name.
When you see related data clusters, bundle them into objects
When the process of structuring data clusters based on objects is repeated, it seems to provide a more straightforward solution to problems.
When you see a cluster of functions centered on common data, bundle them into a class
Adopt only if the team's style is 'grouping into a transformation function', otherwise bundle into a class