KwokYC Refactor Master
Systematic refactoring. Make code easier to read, change, and test. Preserve behavior.
Golden Rule
Every refactoring must pass all existing tests without modification. If you need to change tests, it's not refactoring — it's a feature change.
Before Refactoring Checklist
- [ ] Tests exist and pass (if not, write tests FIRST)
- [ ] You understand what the code does
- [ ] You know WHY it needs refactoring (not just "it looks messy")
- [ ] You have a specific goal (readability, testability, performance)
Refactoring Catalog
1. Extract Function
When a code block does one identifiable thing but is buried inline.
# ❌ Before
def process_order(order):
# validate
if not order.items:
raise ValueError("Empty order")
if not order.customer.email:
raise ValueError("No email")
if order.total < 0:
raise ValueError("Negative total")
# calculate
subtotal = sum(item.price * item.qty for item in order.items)
tax = subtotal * 0.08
total = subtotal + tax
# ... more code
# ✅ After
def process_order(order):
validate_order(order)
total = calculate_order_total(order)
# ... more code
def validate_order(order):
if not order.items:
raise ValueError("Empty order")
if not order.customer.email:
raise ValueError("No email")
if order.total < 0:
raise ValueError("Negative total")
def calculate_order_total(order):
subtotal = sum(item.price * item.qty for item in order.items)
tax = subtotal * 0.08
return subtotal + tax
2. Extract Variable
When a complex expression is hard to understand.
// ❌ Before
if (order.items.length > 0 && order.customer.isVerified && order.total > 0 && !order.isCancelled) {
processPayment(order);
}
// ✅ After
const hasItems = order.items.length > 0;
const customerVerified = order.customer.isVerified;
const validTotal = order.total > 0;
const notCancelled = !order.isCancelled;
const isReadyForPayment = hasItems && customerVerified && validTotal && notCancelled;
if (isReadyForPayment) {
processPayment(order);
}
3. Inline Function
When a function's body is clearer than its name.
# ❌ Before (function adds indirection without value)
def get_rating(driver):
return more_than_five_rides(driver) and has_good_reviews(driver)
def more_than_five_rides(driver):
return driver.total_rides > 5
def has_good_reviews(driver):
return driver.avg_rating >= 4.5
# ✅ After (inline trivial helpers)
def get_rating(driver):
return driver.total_rides > 5 and driver.avg_rating >= 4.5
4. Replace Magic Values
# ❌ Before
if user.role == 3:
allow_access()
if timeout > 86400:
refresh_session()
# ✅ After
ROLE_ADMIN = 3
SESSION_TIMEOUT_SECONDS = 86400
if user.role == ROLE_ADMIN:
allow_access()
if timeout > SESSION_TIMEOUT_SECONDS:
refresh_session()
5. Flatten Nested Conditionals
# ❌ Before
def process_payment(order):
if order:
if order.items:
if order.customer:
if order.customer.verified:
return charge(order)
else:
raise Error("Unverified")
else:
raise Error("No customer")
else:
raise Error("No items")
else:
raise Error("No order")
# ✅ After (guard clauses)
def process_payment(order):
if not order:
raise Error("No order")
if not order.items:
raise Error("No items")
if not order.customer:
raise Error("No customer")
if not order.customer.verified:
raise Error("Unverified")
return charge(order)
6. Remove Duplication (DRY)
# ❌ Before
def send_welcome_email(user):
server = smtplib.SMTP("smtp.example.com", 587)
server.starttls()
server.login("bot@example.com", os.environ["EMAIL_PASS"])
msg = create_welcome_message(user)
server.send_message(msg)
server.quit()
def send_password_reset_email(user):
server = smtplib.SMTP("smtp.example.com", 587)
server.starttls()
server.login("bot@example.com", os.environ["EMAIL_PASS"])
msg = create_reset_message(user)
server.send_message(msg)
server.quit()
# ✅ After
def send_email(user, message_factory):
with smtplib.SMTP("smtp.example.com", 587) as server:
server.starttls()
server.login("bot@example.com", os.environ["EMAIL_PASS"])
msg = message_factory(user)
server.send_message(msg)
def send_welcome_email(user):
send_email(user, create_welcome_message)
def send_password_reset_email(user):
send_email(user, create_reset_message)
7. Split Large File
When a file exceeds 300 lines or has multiple responsibilities.
❌ user_handler.py (800 lines)
- Authentication
- Profile management
- Avatar upload
- Notification preferences
- Account deletion
✅ Split into:
auth_handler.py
profile_handler.py
avatar_handler.py
notifications_handler.py
account_handler.py
8. Replace Temp with Query
# ❌ Before
def get_price(item):
base_price = item.quantity * item.price
discount_factor = 0.95 if base_price > 1000 else 0.98
return base_price * discount_factor
# ✅ After (when base_price and discount_factor are used in multiple places)
def get_price(item):
return base_price(item) * discount_factor(item)
def base_price(item):
return item.quantity * item.price
def discount_factor(item):
return 0.95 if base_price(item) > 1000 else 0.98
Refactoring Decision Tree
Is the code hard to read?
→ Extract functions, rename variables, flatten nesting
Is there duplication?
→ Extract shared logic into functions/classes
Is a function too long (>30 lines)?
→ Split into smaller functions by responsibility
Is a file too long (>300 lines)?
→ Split by feature/responsibility
Are there magic numbers/strings?
→ Replace with named constants
Is it hard to test?
→ Extract dependencies, use injection
Is there deep nesting (>3 levels)?
→ Use guard clauses, early returns, extract inner blocks
When NOT to Refactor
❌ Don't refactor when:
- No tests exist (write tests first)
- You're in the middle of a feature (finish first)
- The code will be deleted soon
- The "improvement" is just style preference
- Deadline is today (refactor in next sprint)
- You don't understand what the code does
Refactoring Workflow
1. Ensure tests pass (run test suite)
2. Make one small change
3. Run tests again
4. Commit if tests pass
5. Repeat from step 2
Never make multiple changes at once.