Refactoring Techniques Catalog
Comprehensive reference of refactoring techniques for transforming code while preserving behavior. Based on Refactoring.Guru Techniques.
Cross-Reference: See code-smells skill for detecting WHAT to fix. This skill shows HOW to fix it.
When to Use
| Context | Trigger |
|---|---|
| Planning Refactor | Need specific technique to fix identified smell |
| Code Review | Suggesting improvements during PR review |
| Learning | User asks how to apply a specific refactoring |
| Quick Fix | User wants to apply a refactoring immediately |
Quick Reference: Smell to Refactoring
| Code Smell | Recommended Refactorings |
|---|---|
| Long Method | Extract Method, Replace Temp with Query, Decompose Conditional |
| Large Class | Extract Class, Extract Subclass, Extract Interface |
| Long Parameter List | Introduce Parameter Object, Preserve Whole Object |
| Duplicate Code | Extract Method, Pull Up Method, Form Template Method |
| Feature Envy | Move Method, Extract Method |
| Switch Statements | Replace Conditional with Polymorphism, Replace Type Code with Subclasses |
| Data Clumps | Extract Class, Introduce Parameter Object |
| Primitive Obsession | Replace Data Value with Object, Replace Type Code with Class |
| Shotgun Surgery | Move Method, Move Field, Inline Class |
| Divergent Change | Extract Class |
| Message Chains | Hide Delegate |
| Middle Man | Remove Middle Man |
| Refused Bequest | Replace Inheritance with Delegation |
Category 1: Composing Methods
Techniques for correctly structuring methods. Long methods are often the root of many problems.
Extract Method
| Aspect | Details |
|---|---|
| Impact | High - Most frequently used refactoring |
| Risk | Low |
| Fixes | Long Method, Duplicate Code, Comments |
Problem: Code fragment that can be logically grouped together.
Solution: Create a new method with a descriptive name and move the fragment there.
Steps:
- Create new method named after WHAT it does (not HOW)
- Copy the code fragment to the new method
- Look for local variables used only within the fragment - make them local to new method
- Pass external variables as parameters
- If extracted code modifies a variable needed later, return it
- Replace original fragment with method call
C++ Example:
// BEFORE
void print_invoice(const Invoice& inv) {
// Print header
std::cout << "================\n";
std::cout << "INVOICE #" << inv.number << "\n";
std::cout << "Date: " << inv.date << "\n";
std::cout << "================\n";
// Print items
for (const auto& item : inv.items) {
std::cout << item.name << ": $" << item.price << "\n";
}
// Calculate and print total
double total = 0;
for (const auto& item : inv.items) {
total += item.price;
}
std::cout << "Total: $" << total << "\n";
}
// AFTER
void print_header(const Invoice& inv) {
std::cout << "================\n";
std::cout << "INVOICE #" << inv.number << "\n";
std::cout << "Date: " << inv.date << "\n";
std::cout << "================\n";
}
void print_items(const std::vector<Item>& items) {
for (const auto& item : items) {
std::cout << item.name << ": $" << item.price << "\n";
}
}
double calculate_total(const std::vector<Item>& items) {
return std::accumulate(items.begin(), items.end(), 0.0,
[](double sum, const Item& item) { return sum + item.price; });
}
void print_invoice(const Invoice& inv) {
print_header(inv);
print_items(inv.items);
std::cout << "Total: $" << calculate_total(inv.items) << "\n";
}
Python Example:
# BEFORE
def print_invoice(inv):
# Print header
print("================")
print(f"INVOICE #{inv.number}")
print(f"Date: {inv.date}")
print("================")
# Print items and calculate total
total = 0
for item in inv.items:
print(f"{item.name}: ${item.price}")
total += item.price
print(f"Total: ${total}")
# AFTER
def print_header(inv):
print("================")
print(f"INVOICE #{inv.number}")
print(f"Date: {inv.date}")
print("================")
def print_items(items):
for item in items:
print(f"{item.name}: ${item.price}")
def calculate_total(items):
return sum(item.price for item in items)
def print_invoice(inv):
print_header(inv)
print_items(inv.items)
print(f"Total: ${calculate_total(inv.items)}")
Inline Method
| Aspect | Details |
|---|---|
| Impact | Low |
| Risk | Low |
| Fixes | Speculative Generality, excessive delegation |
Problem: Method body is more obvious than the method itself.
Solution: Replace method calls with the method content and delete the method.
Steps:
- Verify method is not overridden in subclasses
- Find all calls to the method
- Replace each call with the method body
- Delete the method
// BEFORE
int get_rating() {
return more_than_five_late_deliveries() ? 2 : 1;
}
bool more_than_five_late_deliveries() {
return late_deliveries_ > 5;
}
// AFTER
int get_rating() {
return late_deliveries_ > 5 ? 2 : 1;
}
Extract Variable
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Complex expressions, Comments |
Problem: Hard-to-understand expression.
Solution: Place the expression result in a self-explanatory variable.
Steps:
- Insert a new variable and assign the expression to it
- Replace the original expression with the variable
- Repeat for other occurrences of the same expression
// BEFORE
if (platform.find("MAC") != std::string::npos &&
browser.find("IE") != std::string::npos &&
was_initialized() && resize > 0) {
// ...
}
// AFTER
bool is_mac = platform.find("MAC") != std::string::npos;
bool is_ie = browser.find("IE") != std::string::npos;
bool was_resized = resize > 0;
if (is_mac && is_ie && was_initialized() && was_resized) {
// ...
}
Replace Temp with Query
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Long Method (preparation for Extract Method) |
Problem: Temporary variable storing an expression result for later use.
Solution: Move the expression to a new method and call the method instead of using the variable.
Steps:
- Ensure the variable is only assigned once
- Extract the expression into a new method
- Replace all uses of the variable with method calls
- Remove the variable declaration
// BEFORE
double calculate_total() {
double base_price = quantity_ * item_price_;
if (base_price > 1000) {
return base_price * 0.95;
}
return base_price * 0.98;
}
// AFTER
double base_price() const {
return quantity_ * item_price_;
}
double calculate_total() {
if (base_price() > 1000) {
return base_price() * 0.95;
}
return base_price() * 0.98;
}
Split Temporary Variable
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Variable reuse, unclear intent |
Problem: Local variable used for multiple unrelated purposes.
Solution: Create separate variables for each purpose.
// BEFORE
double temp = 2 * (height_ + width_);
std::cout << "Perimeter: " << temp << "\n";
temp = height_ * width_;
std::cout << "Area: " << temp << "\n";
// AFTER
double perimeter = 2 * (height_ + width_);
std::cout << "Perimeter: " << perimeter << "\n";
double area = height_ * width_;
std::cout << "Area: " << area << "\n";
Remove Assignments to Parameters
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Confusing parameter modification |
Problem: Value assigned to a parameter inside method body.
Solution: Use a local variable instead of the parameter.
// BEFORE
int discount(int input_val, int quantity) {
if (quantity > 50) {
input_val -= 2; // Modifying parameter!
}
return input_val;
}
// AFTER
int discount(int input_val, int quantity) {
int result = input_val;
if (quantity > 50) {
result -= 2;
}
return result;
}
Replace Method with Method Object
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | Medium |
| Fixes | Long Method with intertwined local variables |
Problem: Long method where local variables prevent extraction.
Solution: Transform the method into a separate class where locals become fields.
Steps:
- Create a new class named after the method
- Create a private field for the original object and each local variable/parameter
- Create a constructor that initializes all fields
- Copy the method body to a
compute()method in the new class - Replace original method with creation of method object and call to
compute()
// BEFORE
class Order {
public:
double calculate_price() {
double primary_base = // complex calculation
double secondary = // uses primary_base
double tertiary = // uses secondary
// 100 more lines using all these variables...
return result;
}
};
// AFTER
class PriceCalculator {
Order& order_;
double primary_base_;
double secondary_;
double tertiary_;
public:
PriceCalculator(Order& order) : order_(order) {}
double compute() {
primary_base_ = // complex calculation
secondary_ = // uses primary_base_
tertiary_ = // uses secondary_
// Can now Extract Method freely!
return calculate_final();
}
private:
double calculate_final() { /*...*/ }
};
class Order {
public:
double calculate_price() {
return PriceCalculator(*this).compute();
}
};
Substitute Algorithm
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | High |
| Fixes | Inefficient or unclear algorithm |
Problem: Want to replace an algorithm with a better one.
Solution: Replace the method body with the new algorithm.
Steps:
- Simplify the existing algorithm first
- Create tests covering all edge cases
- Write the new algorithm
- Run tests to verify behavior is identical
- If tests fail, use old algorithm for debugging
// BEFORE
std::string find_person(const std::vector<std::string>& people) {
for (const auto& person : people) {
if (person == "Don") return "Don";
}
for (const auto& person : people) {
if (person == "John") return "John";
}
for (const auto& person : people) {
if (person == "Kent") return "Kent";
}
return "";
}
// AFTER
std::string find_person(const std::vector<std::string>& people) {
static const std::vector<std::string> candidates = {"Don", "John", "Kent"};
for (const auto& candidate : candidates) {
if (std::find(people.begin(), people.end(), candidate) != people.end()) {
return candidate;
}
}
return "";
}
Category 2: Moving Features Between Objects
Techniques for distributing functionality optimally between classes.
Move Method
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | Medium |
| Fixes | Feature Envy, Shotgun Surgery |
Problem: Method is used more by another class than its own class.
Solution: Move the method to the class that uses it most.
Steps:
- Examine all features used by the method in its current class
- Check if method is declared in parent/child classes (may need polymorphism)
- Declare the method in the target class
- Copy the method body, adapting references to the new context
- Convert original method to delegate to the new location (or remove it)
- Update all callers
// BEFORE
class Account {
AccountType type_;
public:
double overdraft_charge() {
if (type_.is_premium()) {
double result = 10;
if (days_overdrawn_ > 7) {
result += (days_overdrawn_ - 7) * 0.85;
}
return result;
}
return days_overdrawn_ * 1.75;
}
};
// AFTER - Method moved to AccountType where it belongs
class AccountType {
public:
double overdraft_charge(int days_overdrawn) {
if (is_premium()) {
double result = 10;
if (days_overdrawn > 7) {
result += (days_overdrawn - 7) * 0.85;
}
return result;
}
return days_overdrawn * 1.75;
}
};
class Account {
public:
double overdraft_charge() {
return type_.overdraft_charge(days_overdrawn_);
}
};
Move Field
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | Medium |
| Fixes | Feature Envy, Data Clumps |
Problem: Field is used more by another class than its own class.
Solution: Create a field in the target class and redirect all users.
Steps:
- If field is public, encapsulate it first
- Create field and accessors in target class
- Determine how to reference the target from the source
- Update all references to use the new location
- Remove the field from the original class
// BEFORE
class Account {
AccountType type_;
double interest_rate_;
};
// AFTER - interest_rate moved to AccountType
class AccountType {
double interest_rate_;
public:
double interest_rate() const { return interest_rate_; }
void set_interest_rate(double rate) { interest_rate_ = rate; }
};
class Account {
AccountType type_;
public:
double interest_rate() const { return type_.interest_rate(); }
};
Extract Class
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | Medium |
| Fixes | Large Class, Divergent Change, Data Clumps |
Problem: One class does the work of two.
Solution: Create a new class and move relevant fields and methods.
Steps:
- Decide what to split out
- Create a new class for the extracted responsibility
- Create a link from the old class to the new one
- Use Move Field for each field to move
- Use Move Method for each method to move
- Review and reduce interfaces of both classes
- Decide on new class visibility (public or encapsulated)
// BEFORE
class Person {
std::string name_;
std::string office_area_code_;
std::string office_number_;
public:
std::string telephone_number() {
return "(" + office_area_code_ + ") " + office_number_;
}
};
// AFTER
class TelephoneNumber {
std::string area_code_;
std::string number_;
public:
std::string format() const {
return "(" + area_code_ + ") " + number_;
}
// getters/setters...
};
class Person {
std::string name_;
TelephoneNumber office_telephone_;
public:
std::string telephone_number() {
return office_telephone_.format();
}
};
Inline Class
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Lazy Class, Speculative Generality |
Problem: Class does almost nothing and has no planned responsibilities.
Solution: Move all features to another class and delete the empty class.
Steps:
- Make all public members of the source class public in the target
- Change all references to point to the target class
- Move methods and fields one by one to the target
- Delete the empty source class
// BEFORE - TelephoneNumber is too thin
class TelephoneNumber {
std::string number_;
public:
std::string number() const { return number_; }
};
class Person {
TelephoneNumber phone_;
public:
std::string phone_number() { return phone_.number(); }
};
// AFTER
class Person {
std::string phone_number_;
public:
std::string phone_number() const { return phone_number_; }
};
Hide Delegate
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Message Chains, tight coupling |
Problem: Client calls object B from object A, then calls methods on B.
Solution: Create a method in A that delegates to B, hiding B from the client.
// BEFORE - Client knows about Department
class Person {
Department* department_;
public:
Department* department() { return department_; }
};
// Client code:
manager = john.department()->manager();
// AFTER - Department hidden
class Person {
Department* department_;
public:
Person* manager() { return department_->manager(); }
};
// Client code:
manager = john.manager();
Remove Middle Man
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Middle Man |
Problem: Class has too many methods that simply delegate.
Solution: Delete delegation methods and have clients call the delegate directly.
// BEFORE - Person delegates everything
class Person {
Department* department_;
public:
Person* manager() { return department_->manager(); }
std::string budget() { return department_->budget(); }
std::string team_size() { return department_->team_size(); }
// ... 10 more delegation methods
};
// AFTER - Expose delegate for complex queries
class Person {
Department* department_;
public:
Department* department() { return department_; }
};
// Client handles department directly when needed
auto budget = john.department()->budget();
Introduce Foreign Method
| Aspect | Details |
|---|---|
| Impact | Low |
| Risk | Low |
| Fixes | Incomplete Library Class (1-2 methods) |
Problem: Utility class lacks a method you need, and you cannot modify it.
Solution: Create the method in your client class, taking the utility object as a parameter.
// Can't modify Date class, need next_day functionality
class Report {
// Foreign method - ideally should be in Date class
static Date next_day(const Date& date) {
return Date(date.year(), date.month(), date.day() + 1);
}
void generate() {
Date start = get_start_date();
Date end = next_day(start); // Using foreign method
// ...
}
};
Introduce Local Extension
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Medium |
| Fixes | Incomplete Library Class (many methods) |
Problem: Utility class lacks several methods you need.
Solution: Create a new class (subclass or wrapper) containing the methods.
// Subclass approach
class ExtendedDate : public Date {
public:
using Date::Date; // Inherit constructors
Date next_day() const {
return Date(year(), month(), day() + 1);
}
bool is_weekend() const {
int dow = day_of_week();
return dow == 0 || dow == 6;
}
};
// Wrapper approach (when inheritance not possible)
class DateWrapper {
Date date_;
public:
explicit DateWrapper(const Date& d) : date_(d) {}
// Delegate existing methods
int year() const { return date_.year(); }
int month() const { return date_.month(); }
int day() const { return date_.day(); }
// Add new methods
Date next_day() const {
return Date(year(), month(), day() + 1);
}
};
Category 3: Organizing Data
Techniques for improving data handling and class associations.
Self Encapsulate Field
| Aspect | Details |
|---|---|
| Impact | Low |
| Risk | Low |
| Fixes | Direct field access issues |
Problem: Direct access to private fields inside the class.
Solution: Create getter/setter and use them internally.
// BEFORE
class IntRange {
int low_, high_;
public:
bool includes(int arg) {
return arg >= low_ && arg <= high_; // Direct access
}
};
// AFTER
class IntRange {
int low_, high_;
public:
int low() const { return low_; }
int high() const { return high_; }
bool includes(int arg) {
return arg >= low() && arg <= high(); // Via getters
}
};
Replace Data Value with Object
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | Medium |
| Fixes | Primitive Obsession, Data Clumps |
Problem: Data field has behavior or associated data.
Solution: Turn the field into a class.
// BEFORE
class Order {
std::string customer_name_;
std::string customer_email_;
std::string customer_phone_;
};
// AFTER
class Customer {
std::string name_;
std::string email_;
std::string phone_;
public:
// Behavior can live here
bool is_valid_email() const;
std::string formatted_phone() const;
};
class Order {
Customer customer_;
};
Replace Magic Number with Symbolic Constant
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Magic Numbers |
Problem: Code uses a number with special meaning.
Solution: Replace with a named constant.
// BEFORE
double potential_energy(double mass, double height) {
return mass * 9.81 * height; // Magic number!
}
// AFTER
constexpr double GRAVITATIONAL_ACCELERATION = 9.81; // m/s^2
double potential_energy(double mass, double height) {
return mass * GRAVITATIONAL_ACCELERATION * height;
}
Encapsulate Field
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Public fields, Data Class |
Problem: You have a public field.
Solution: Make it private and provide accessors.
// BEFORE
class Person {
public:
std::string name; // Public field!
};
// AFTER
class Person {
std::string name_;
public:
const std::string& name() const { return name_; }
void set_name(const std::string& name) { name_ = name; }
};
Encapsulate Collection
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | Medium |
| Fixes | Collection exposure, broken encapsulation |
Problem: Method returns a collection that can be modified externally.
Solution: Return read-only view and provide add/remove methods.
// BEFORE
class Person {
std::vector<Course> courses_;
public:
std::vector<Course>& courses() { return courses_; } // Dangerous!
};
// Client can: person.courses().clear(); // Bypass Person!
// AFTER
class Person {
std::vector<Course> courses_;
public:
const std::vector<Course>& courses() const { return courses_; }
void add_course(const Course& c) {
courses_.push_back(c);
}
void remove_course(const Course& c) {
courses_.erase(
std::remove(courses_.begin(), courses_.end(), c),
courses_.end());
}
};
Replace Type Code with Class
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | Medium |
| Fixes | Primitive Obsession (type codes) |
Problem: Class has a field with type code that doesn't affect behavior.
Solution: Replace with a new class.
// BEFORE
class Person {
int blood_type_; // 0=O, 1=A, 2=B, 3=AB
public:
static constexpr int O = 0;
static constexpr int A = 1;
static constexpr int B = 2;
static constexpr int AB = 3;
};
// AFTER
class BloodType {
int code_;
BloodType(int code) : code_(code) {}
public:
static const BloodType O;
static const BloodType A;
static const BloodType B;
static const BloodType AB;
bool operator==(const BloodType& other) const {
return code_ == other.code_;
}
};
const BloodType BloodType::O{0};
const BloodType BloodType::A{1};
const BloodType BloodType::B{2};
const BloodType BloodType::AB{3};
class Person {
BloodType blood_type_;
};
Replace Type Code with Subclasses
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | High |
| Fixes | Switch Statements, Primitive Obsession |
Problem: Type code that directly affects behavior via conditionals.
Solution: Create subclasses for each type code value.
// BEFORE
class Employee {
int type_; // 0=Engineer, 1=Manager, 2=Salesman
public:
static constexpr int ENGINEER = 0;
static constexpr int MANAGER = 1;
static constexpr int SALESMAN = 2;
int bonus() {
switch (type_) {
case ENGINEER: return 100;
case MANAGER: return 500;
case SALESMAN: return 200;
}
return 0;
}
};
// AFTER
class Employee {
public:
virtual int bonus() = 0;
};
class Engineer : public Employee {
public:
int bonus() override { return 100; }
};
class Manager : public Employee {
public:
int bonus() override { return 500; }
};
class Salesman : public Employee {
public:
int bonus() override { return 200; }
};
Replace Type Code with State/Strategy
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | High |
| Fixes | Switch Statements when subclassing is impossible |
Problem: Type code affects behavior but you cannot use subclasses (type changes at runtime).
Solution: Replace type code with a state object.
// BEFORE
class Employee {
int type_;
void set_type(int type) { type_ = type; } // Type can change!
public:
int pay_amount() {
switch (type_) {
case ENGINEER: return monthly_salary_;
case SALESMAN: return monthly_salary_ + commission_;
case MANAGER: return monthly_salary_ + bonus_;
}
}
};
// AFTER
class EmployeeType {
public:
virtual int pay_amount(const Employee& emp) = 0;
};
class Engineer : public EmployeeType {
public:
int pay_amount(const Employee& emp) override {
return emp.monthly_salary();
}
};
class Employee {
std::unique_ptr<EmployeeType> type_;
public:
void set_type(std::unique_ptr<EmployeeType> t) {
type_ = std::move(t); // Can change type at runtime
}
int pay_amount() {
return type_->pay_amount(*this);
}
};
Category 4: Simplifying Conditional Expressions
Techniques for managing conditional complexity.
Decompose Conditional
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | Low |
| Fixes | Long Method, Complex conditionals |
Problem: Complex conditional (if-then-else or switch).
Solution: Extract condition, then-block, and else-block into separate methods.
// BEFORE
double calculate_charge(const Date& date, int quantity) {
if (date < SUMMER_START || date > SUMMER_END) {
return quantity * winter_rate_ + winter_service_charge_;
} else {
return quantity * summer_rate_;
}
}
// AFTER
bool is_summer(const Date& date) {
return date >= SUMMER_START && date <= SUMMER_END;
}
double summer_charge(int quantity) {
return quantity * summer_rate_;
}
double winter_charge(int quantity) {
return quantity * winter_rate_ + winter_service_charge_;
}
double calculate_charge(const Date& date, int quantity) {
if (is_summer(date)) {
return summer_charge(quantity);
}
return winter_charge(quantity);
}
Consolidate Conditional Expression
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Duplicate conditionals |
Problem: Multiple conditionals leading to the same result.
Solution: Combine into a single expression and extract to a method.
// BEFORE
double disability_amount() {
if (seniority_ < 2) return 0;
if (months_disabled_ > 12) return 0;
if (is_part_time_) return 0;
// Calculate disability
return calculate_disability();
}
// AFTER
bool is_not_eligible_for_disability() {
return seniority_ < 2
|| months_disabled_ > 12
|| is_part_time_;
}
double disability_amount() {
if (is_not_eligible_for_disability()) return 0;
return calculate_disability();
}
Consolidate Duplicate Conditional Fragments
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Duplicate Code in branches |
Problem: Same code in all branches of a conditional.
Solution: Move it outside the conditional.
// BEFORE
if (is_special_deal()) {
total = price * 0.95;
send_notification(); // Duplicated!
} else {
total = price * 0.98;
send_notification(); // Duplicated!
}
// AFTER
if (is_special_deal()) {
total = price * 0.95;
} else {
total = price * 0.98;
}
send_notification(); // Moved outside
Replace Nested Conditional with Guard Clauses
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | Low |
| Fixes | Deep nesting, unclear control flow |
Problem: Nested conditionals make normal flow hard to see.
Solution: Use guard clauses for special cases, returning early.
// BEFORE
double get_pay_amount() {
double result;
if (is_dead_) {
result = dead_amount();
} else {
if (is_separated_) {
result = separated_amount();
} else {
if (is_retired_) {
result = retired_amount();
} else {
result = normal_pay_amount();
}
}
}
return result;
}
// AFTER
double get_pay_amount() {
if (is_dead_) return dead_amount();
if (is_separated_) return separated_amount();
if (is_retired_) return retired_amount();
return normal_pay_amount();
}
Replace Conditional with Polymorphism
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | High |
| Fixes | Switch Statements, Type-based conditionals |
Problem: Conditional that varies behavior based on object type.
Solution: Create subclasses and override the method.
Steps:
- If conditional is part of larger method, use Extract Method first
- If conditional is in a class that has other responsibilities, Extract Class first
- Create subclasses for each conditional branch
- Create abstract method in superclass
- Override method in each subclass with branch logic
- Delete branches from original conditional
- Delete the conditional; make method abstract
// BEFORE
class Bird {
std::string type_;
public:
double get_speed() {
switch (type_) {
case "European": return get_base_speed();
case "African":
return get_base_speed() - load_factor_ * num_coconuts_;
case "NorwegianBlue":
return is_nailed_ ? 0 : get_base_speed(voltage_);
}
}
};
// AFTER
class Bird {
public:
virtual double get_speed() = 0;
};
class European : public Bird {
public:
double get_speed() override {
return get_base_speed();
}
};
class African : public Bird {
public:
double get_speed() override {
return get_base_speed() - load_factor_ * num_coconuts_;
}
};
class NorwegianBlue : public Bird {
public:
double get_speed() override {
return is_nailed_ ? 0 : get_base_speed(voltage_);
}
};
Introduce Null Object
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | Medium |
| Fixes | Repeated null checks |
Problem: Many null checks throughout the code.
Solution: Return a null object that exhibits default behavior instead of null.
// BEFORE
class Customer {
BillingPlan* plan_;
public:
BillingPlan* plan() { return plan_; } // May return nullptr
};
// Usage scattered with null checks:
if (customer != nullptr) {
plan = customer->plan();
} else {
plan = BillingPlan::basic();
}
// AFTER
class NullCustomer : public Customer {
public:
bool is_null() override { return true; }
BillingPlan* plan() override { return &BillingPlan::basic(); }
};
class Customer {
public:
virtual bool is_null() { return false; }
virtual BillingPlan* plan() { return plan_; }
};
// Usage - no null checks needed:
plan = customer->plan();
Introduce Assertion
| Aspect | Details |
|---|---|
| Impact | Low |
| Risk | Low |
| Fixes | Implicit assumptions |
Problem: Code assumes certain conditions are true.
Solution: Make assumptions explicit with assertions.
// BEFORE
double get_expense_limit() {
// Assumes either expense_limit or primary_project is set
return (expense_limit_ != NULL_EXPENSE)
? expense_limit_
: primary_project_->member_expense_limit();
}
// AFTER
double get_expense_limit() {
assert(expense_limit_ != NULL_EXPENSE || primary_project_ != nullptr);
return (expense_limit_ != NULL_EXPENSE)
? expense_limit_
: primary_project_->member_expense_limit();
}
Category 5: Simplifying Method Calls
Techniques for making method interfaces cleaner.
Rename Method
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | Low |
| Fixes | Unclear intent, Comments |
Problem: Method name doesn't explain what it does.
Solution: Rename it.
// BEFORE
std::string get_tlp() { return telephone_; }
// AFTER
std::string get_telephone_number() { return telephone_; }
Add Parameter / Remove Parameter
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Medium |
| Fixes | Missing/unused data |
Add Parameter: Method needs data it doesn't have. Remove Parameter: Parameter is unused.
// Add Parameter
// BEFORE: Date fetched from global state
Date get_date() { return Date::today(); }
// AFTER: Date passed explicitly
Date get_date(const Date& base_date) { return base_date; }
// Remove Parameter
// BEFORE: Unused parameter
void set_value(int value, int unused) { value_ = value; }
// AFTER
void set_value(int value) { value_ = value; }
Separate Query from Modifier
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | Medium |
| Fixes | Side effects, hard to test |
Problem: Method returns value AND changes object state.
Solution: Split into two methods: one returns value, one modifies state.
// BEFORE
int get_total_and_clear() {
int result = calculate_total();
clear_items(); // Side effect!
return result;
}
// AFTER
int get_total() const {
return calculate_total(); // Query - no side effects
}
void clear() {
clear_items(); // Modifier - no return value
}
Parameterize Method
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Similar methods, Duplicate Code |
Problem: Multiple methods do similar things with different values.
Solution: Create one method with a parameter.
// BEFORE
void five_percent_raise() { salary_ *= 1.05; }
void ten_percent_raise() { salary_ *= 1.10; }
void fifteen_percent_raise() { salary_ *= 1.15; }
// AFTER
void raise(double factor) { salary_ *= (1 + factor); }
// Usage: raise(0.05), raise(0.10), raise(0.15)
Introduce Parameter Object
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | Medium |
| Fixes | Long Parameter List, Data Clumps |
Problem: Methods have repeating parameter groups.
Solution: Replace with an object containing those parameters.
// BEFORE
void amount_invoiced(Date start, Date end);
void amount_received(Date start, Date end);
void amount_overdue(Date start, Date end);
// AFTER
class DateRange {
Date start_, end_;
public:
DateRange(Date start, Date end) : start_(start), end_(end) {}
Date start() const { return start_; }
Date end() const { return end_; }
bool includes(Date d) const { return d >= start_ && d <= end_; }
};
void amount_invoiced(DateRange range);
void amount_received(DateRange range);
void amount_overdue(DateRange range);
Preserve Whole Object
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Long Parameter List, Feature Envy |
Problem: Getting several values from an object then passing them as parameters.
Solution: Pass the whole object instead.
// BEFORE
int low = days_temp_range.low();
int high = days_temp_range.high();
bool within_plan = plan.within_range(low, high);
// AFTER
bool within_plan = plan.within_range(days_temp_range);
Replace Constructor with Factory Method
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | Medium |
| Fixes | Complex construction, Type-based creation |
Problem: Constructor does more than setting fields, or you need type-based creation.
Solution: Replace with factory method.
// BEFORE
class Employee {
int type_;
public:
Employee(int type) : type_(type) {}
};
// AFTER
class Employee {
protected:
Employee() = default;
public:
static std::unique_ptr<Employee> create(int type) {
switch (type) {
case ENGINEER: return std::make_unique<Engineer>();
case MANAGER: return std::make_unique<Manager>();
case SALESMAN: return std::make_unique<Salesman>();
}
throw std::invalid_argument("Invalid employee type");
}
};
Replace Error Code with Exception
| Aspect | Details |
|---|---|
| Impact | High |
| Risk | Medium |
| Fixes | Error handling clutter |
Problem: Method returns special value to indicate error.
Solution: Throw an exception instead.
// BEFORE
int withdraw(int amount) {
if (amount > balance_) {
return -1; // Error code
}
balance_ -= amount;
return 0; // Success
}
// Caller must check: if (account.withdraw(100) == -1) { ... }
// AFTER
void withdraw(int amount) {
if (amount > balance_) {
throw std::runtime_error("Insufficient funds");
}
balance_ -= amount;
}
// Caller: try { account.withdraw(100); } catch (...) { ... }
Replace Exception with Test
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Exception misuse |
Problem: Exception thrown where a condition check would suffice.
Solution: Replace exception with a condition test.
// BEFORE
double value_for_period(int period) {
try {
return values_.at(period);
} catch (std::out_of_range&) {
return 0;
}
}
// AFTER
double value_for_period(int period) {
if (period < 0 || period >= values_.size()) {
return 0;
}
return values_[period];
}
Category 6: Dealing with Generalization
Techniques for managing inheritance hierarchies.
Pull Up Field / Pull Up Method
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Duplicate Code in subclasses |
Problem: Subclasses have the same field or method.
Solution: Move to the superclass.
// BEFORE
class Salesman : public Employee {
std::string name_; // Duplicated!
};
class Engineer : public Employee {
std::string name_; // Duplicated!
};
// AFTER
class Employee {
protected:
std::string name_; // Moved to parent
};
class Salesman : public Employee {};
class Engineer : public Employee {};
Pull Up Constructor Body
| Aspect | Details |
|---|---|
| Impact | Medium |
| Risk | Low |
| Fixes | Duplicate constructor code |
Problem: Subclass constructors have mostly identical code.
Solution: Create superclass constructor and call from subclasses.
// BEFORE
class Manager : public Employe
…(truncated)