Proven refactoring patterns (Extract Method, Replace Temp, Introduce Parameter Object) to improve code structure safely. Use when improving existing code while keeping behavior unchanged.
From code-qualitynpx claudepluginhub sethdford/claude-skills --plugin engineer-code-qualityThis skill is limited to using the following tools:
examples/example-output.mdtemplate.mdDesigns and optimizes AI agent action spaces, tool definitions, observation formats, error recovery, and context for higher task completion rates.
Enables AI agents to execute x402 payments with per-task budgets, spending controls, and non-custodial wallets via MCP tools. Use when agents pay for APIs, services, or other agents.
Compares coding agents like Claude Code and Aider on custom YAML-defined codebase tasks using git worktrees, measuring pass rate, cost, time, and consistency.
Systematic, low-risk transformations to improve code structure, readability, and maintainability without changing observable behavior.
You are guiding code improvement through safe refactoring. Your role is to:
Refactoring is the art of cleaning house without breaking anything. Each refactoring is tiny, testable, and immediately verifiable.
Based on Martin Fowler's Refactoring and Michael Feathers' Working Effectively with Legacy Code:
Before refactoring, ensure:
Read the code and ask:
Example identifying refactoring opportunities:
# Poor: Vague names, duplicated logic, multiple responsibilities
def process_order(order):
subtotal = 0
for item in order.items:
subtotal += item.price * item.qty
subtotal *= 0.95 # 5% discount (magic number!)
if order.is_vip:
subtotal *= 0.9 # another 10% for VIP
tax = subtotal * 0.08
total = subtotal + tax
print(f"Total: {total}")
if total > 100:
order.flag_for_priority_shipping()
return total
Refactoring opportunities:
calculate_subtotal() (duplicated calculation logic across similar methods)apply_discount() (separate discount logic)apply_tax() (separate tax logic)is_priority_order() check (readability)Before refactoring, ensure tests cover the behavior you'll change:
def test_process_order_calculates_correct_total():
order = Order(items=[
Item("Widget", 50, 2), # 2x Widget @ $50 = $100
Item("Gadget", 25, 1) # 1x Gadget @ $25 = $25
], is_vip=False)
# Subtotal: $125 → -5% discount = $118.75 → +8% tax = $128.25
assert process_order(order) == 128.25
def test_process_order_vip_discount():
order = Order(items=[
Item("Widget", 100, 1)
], is_vip=True)
# Subtotal: $100 → -5% = $95 → -10% = $85.50 → +8% tax = $92.34
assert process_order(order) == 92.34
def test_process_order_flags_priority_for_large_orders():
order = Order(items=[
Item("Widget", 60, 2) # $120
], is_vip=False)
process_order(order)
assert order.priority_shipping_flagged is True
Run these tests. They pass with the current (messy) code. Now refactor with confidence.
Goal: Move duplicated or complex logic into a helper method.
Before:
def process_order(order):
subtotal = 0
for item in order.items:
subtotal += item.price * item.qty
subtotal *= 0.95
if order.is_vip:
subtotal *= 0.9
tax = subtotal * 0.08
total = subtotal + tax
# ... more logic
After:
def process_order(order):
subtotal = self._calculate_subtotal(order)
subtotal = self._apply_discount(subtotal, order.is_vip)
total = self._apply_tax(subtotal)
# ... more logic
def _calculate_subtotal(self, order):
"""Sum of all item prices (unit price * quantity)."""
subtotal = 0
for item in order.items:
subtotal += item.price * item.qty
return subtotal
def _apply_discount(self, subtotal, is_vip):
"""Apply standard 5% discount, plus 10% for VIP."""
subtotal *= 0.95
if is_vip:
subtotal *= 0.9
return subtotal
def _apply_tax(self, subtotal):
"""Apply 8% sales tax."""
return subtotal * 1.08
Run tests: All tests should pass. Behavior is identical.
Commit: refactor: extract discount and tax logic into helper methods
Goal: Replace hardcoded numbers with named constants.
Before:
def _apply_discount(self, subtotal, is_vip):
subtotal *= 0.95 # What is 0.95?
if is_vip:
subtotal *= 0.9 # What is 0.9?
return subtotal
def _apply_tax(self, subtotal):
return subtotal * 1.08 # What is 1.08?
After:
STANDARD_DISCOUNT_RATE = 0.05 # 5% discount
VIP_DISCOUNT_RATE = 0.10 # Additional 10% for VIP
SALES_TAX_RATE = 0.08 # 8% sales tax
def _apply_discount(self, subtotal, is_vip):
subtotal *= (1 - self.STANDARD_DISCOUNT_RATE)
if is_vip:
subtotal *= (1 - self.VIP_DISCOUNT_RATE)
return subtotal
def _apply_tax(self, subtotal):
return subtotal * (1 + self.SALES_TAX_RATE)
Run tests: All tests should pass.
Commit: refactor: replace magic numbers with named constants
Goal: Replace boolean flag parameters with separate, well-named methods.
Before:
def process_order(order, include_tax=True): # Flag parameter; confusing
subtotal = self._calculate_subtotal(order)
subtotal = self._apply_discount(subtotal, order.is_vip)
if include_tax:
total = self._apply_tax(subtotal)
else:
total = subtotal
return total
After:
def process_order(order):
"""Calculate order total including all discounts and taxes."""
subtotal = self._calculate_subtotal(order)
subtotal = self._apply_discount(subtotal, order.is_vip)
total = self._apply_tax(subtotal)
return total
def calculate_order_subtotal(order):
"""Calculate order total without taxes (for internal use)."""
subtotal = self._calculate_subtotal(order)
subtotal = self._apply_discount(subtotal, order.is_vip)
return subtotal
Now callers choose the appropriate method name, making intent clear.
Run tests: All tests should pass.
Commit: refactor: replace include_tax flag parameter with distinct methods
Goal: Move a method to the class that uses it most, improving cohesion.
Before:
class Order:
def process(self):
# ...
total = order_processor._apply_tax(subtotal)
class OrderProcessor:
def _apply_tax(self, subtotal):
return subtotal * (1 + TAX_RATE)
After (Tax logic belongs to Order, not OrderProcessor):
class Order:
def apply_tax(self, subtotal):
"""Apply sales tax to subtotal."""
return subtotal * (1 + TAX_RATE)
def process(self):
# ...
total = self.apply_tax(subtotal)
class OrderProcessor:
# _apply_tax removed; moved to Order
Run tests: All tests should pass.
Commit: refactor: move tax calculation to Order class
Scan for copy-paste code:
Before:
def format_price_usd(price):
return f"${price:.2f}"
def format_price_eur(price):
return f"€{price:.2f}"
def format_price_gbp(price):
return f"£{price:.2f}"
After:
def format_price(price, currency_symbol="$"):
return f"{currency_symbol}{price:.2f}"
Or use a Currency object for better abstraction.
Run tests: All tests should pass.
Commit: refactor: consolidate currency formatting
After a series of refactorings:
When using this skill, deliver:
Example output:
## Refactoring: Extract Method
**Identified Problem**:
Discount calculation is duplicated across `process_order()` and `calculate_order_subtotal()`.
**Before**:
[code showing duplication]
**After**:
[code with helper method]
**Tests**: All pass. Behavior unchanged.
**Commit Message**:
refactor: extract discount calculation into helper method
Starting code (smelly):
class PaymentProcessor:
def process_payment(self, amount, customer_type, apply_fee=True):
# Validate amount
if amount <= 0:
raise ValueError("Invalid amount")
# Calculate fee based on customer type
fee = 0
if customer_type == "premium":
fee = amount * 0.01
elif customer_type == "standard":
fee = amount * 0.025
else:
fee = amount * 0.05
# Apply fee
if apply_fee:
total = amount + fee
else:
total = amount
# Process
print(f"Processing ${total} for {customer_type} customer")
return total
Refactoring 1: Extract Method for validation
def _validate_amount(self, amount):
if amount <= 0:
raise ValueError("Invalid amount")
def process_payment(self, amount, customer_type, apply_fee=True):
self._validate_amount(amount)
fee = self._calculate_fee(amount, customer_type)
if apply_fee:
total = amount + fee
else:
total = amount
print(f"Processing ${total} for {customer_type} customer")
return total
Refactoring 2: Extract fee calculation
def _calculate_fee(self, amount, customer_type):
"""Calculate processing fee based on customer type."""
fee_rates = {
"premium": 0.01,
"standard": 0.025,
"standard": 0.05, # default
}
rate = fee_rates.get(customer_type, 0.05)
return amount * rate
Refactoring 3: Replace flag parameter with methods
def process_payment(self, amount, customer_type):
"""Process payment including fees."""
self._validate_amount(amount)
return self._calculate_total_with_fee(amount, customer_type)
def process_payment_without_fee(self, amount, customer_type):
"""Process payment without fees."""
self._validate_amount(amount)
return amount
def _calculate_total_with_fee(self, amount, customer_type):
fee = self._calculate_fee(amount, customer_type)
total = amount + fee
print(f"Processing ${total} for {customer_type} customer")
return total
Final refactored state:
class PaymentProcessor:
FEE_RATES = {
"premium": 0.01,
"standard": 0.025,
"basic": 0.05,
}
def process_payment(self, amount, customer_type):
"""Process payment with fees."""
self._validate_amount(amount)
fee = self._calculate_fee(amount, customer_type)
total = amount + fee
self._log_transaction(total, customer_type)
return total
def process_payment_without_fee(self, amount, customer_type):
"""Process payment without fees (e.g., internal transfers)."""
self._validate_amount(amount)
return amount
def _validate_amount(self, amount):
if amount <= 0:
raise ValueError("Invalid amount")
def _calculate_fee(self, amount, customer_type):
rate = self.FEE_RATES.get(customer_type, 0.05)
return amount * rate
def _log_transaction(self, amount, customer_type):
print(f"Processing ${amount} for {customer_type} customer")
Each refactoring:
When refactoring, use these decisions:
Mistake: Change code structure without test coverage; hope nothing breaks.
Why LLMs make this: Confidence in code changes without safety net.
Guard: Before refactoring any code, ensure tests pass and cover the area being refactored (70%+ coverage).
Example:
calculate_discount() without tests; verify manuallycalculate_discount() before refactoringMistake: In one commit/PR, both refactor code AND add a new feature.
Why LLMs make this: Feels efficient to clean up while implementing.
Guard: Separate refactoring commits from feature commits. Reviewers can't distinguish intent if mixed.
Example:
Mistake: Create helper methods for code that isn't duplicated.
Why LLMs make this: Speculative abstractions feel like good engineering.
Guard: Wait until you see duplication 2-3 times before extracting. One method alone is just splitting for no reason.
Example:
_format_debug_message() when used onceMistake: Refactor many things in one commit (rename variables, extract methods, move classes).
Why LLMs make this: Completing the whole improvement feels productive.
Guard: One refactoring per commit. If it takes >15 minutes, break into smaller commits.
Example:
Mistake: Make multiple refactorings, then run tests once.
Why LLMs make this: Batching feels efficient.
Guard: Run tests after each refactoring (every 5-15 minutes). Rapid feedback reveals problems immediately.
Example:
Mistake: Refactor for clarity; accidentally introduce performance regression.
Why LLMs make this: Focus on readability; miss runtime implications.
Guard: Profile before and after refactoring. "Replace Temp with Query" introduces function calls; measure impact.
Example:
calculate_total() that's called in a loop 10,000xBefore considering a refactoring complete, verify: