The Day I Realized My "Working" Code Was Actually Broken
I was staring at a Patient class that calculated health metrics – BMI, ideal weight, heart rate – all crammed into one massive method. It worked, sure, but it was a nightmare to test, impossible to extend, and frankly, embarrassing to show to other developers.
In this issue, I'll take you through the journey from messy, monolithic methods to clean, testable, maintainable code. No theory dumps – just real code, real problems, and real solutions.
🚀 What you'll learn:
How to identify code smells that are killing your productivity
A systematic approach to refactoring without breaking everything
Why good code isn't just about making it work – it's about making it last
Practical metrics that actually matter for code quality
The Problem
Here's what we were dealing with – a Patient class that seemed innocent enough:
class Patient
def initialize(weight, height, age)
@weight = weight
@height = height
@age = age
end
def health_metrics
# BMI calculation
bmi = (@weight / (@height ** 2)).round(2)
# Ideal weight calculation
ideal_weight = (22 * (@height ** 2)).round(2)
# Heart rate calculation based on age
heart_rate = calculate_heart_rate(@age)
# Returning the health metrics as a hash
{ bmi: bmi, ideal_weight: ideal_weight, heart_rate: heart_rate }
end
private
def calculate_heart_rate(age)
70 - (age / 10)
end
end
💡 Warning Signs:
One method doing three completely different calculations
Impossible to test individual calculations separately
Adding new health metrics meant modifying existing code
Comments explaining what each section does (usually a red flag)
The Journey: From Problem to Solution
Step 1: Recognizing the Code Smell
The first breakthrough came when I ran Reek on this code. The complexity score was off the charts – well above the danger threshold of 10. The tool immediately flagged the health_metrics method for violating the Single Responsibility Principle.
Before:
def health_metrics
# Three different responsibilities mashed together
bmi = (@weight / (@height ** 2)).round(2)
ideal_weight = (22 * (@height ** 2)).round(2)
heart_rate = calculate_heart_rate(@age)
{ bmi: bmi, ideal_weight: ideal_weight, heart_rate: heart_rate }
end
After:
def health_metrics
{
bmi: calculate_bmi,
ideal_weight: calculate_ideal_weight,
heart_rate: calculate_heart_rate(@age)
}
end
private
def calculate_bmi
(@weight / (@height ** 2)).round(2)
end
def calculate_ideal_weight
(22 * (@height ** 2)).round(2)
end
🎯 Impact:
Each calculation became independently testable
Adding new metrics no longer required modifying existing code
The main method became a clear orchestrator rather than a do-everything monster
Step 2: Following the Open/Closed Principle
The real aha moment came when I asked: "Can I add a new health metric without modifying existing code?" The answer was still no. So I kept refactoring.
The Aha Moment
The breakthrough happened when I realized that good code isn't just about making it work – it's about making it continue to work as requirements change. The key insight: as production code becomes more generic, tests should become more specific and explicit.
Real Numbers From This Experience
Before: 1 test for the entire health_metrics method
After: 4 focused tests, each covering one responsibility
Test writing time: Reduced from 15 minutes to 3 minutes per new health metric
Bug detection: Increased from catching issues in production to catching them at compile time
The Final Result
class Patient
def initialize(weight, height, age)
@weight = weight
@height = height
@age = age
end
def health_metrics
{
bmi: calculate_bmi,
ideal_weight: calculate_ideal_weight,
heart_rate: calculate_heart_rate
}
end
def calculate_bmi
(@weight / (@height ** 2)).round(2)
end
def calculate_ideal_weight
(22 * (@height ** 2)).round(2)
end
def calculate_heart_rate
(70 - (@age / 10)).round(2)
end
private
attr_reader :weight, :height, :age
end
🎉 Key Improvements:
Each method has a single, clear responsibility
Easy to test individual calculations
Simple to add new health metrics
Code that's both simple and expressive
Monday Morning Action Items
Quick Wins (5-Minute Changes)
Run a code smell detection tool (like Reek for Ruby) on your current project
Identify one method that's doing more than one thing
Extract one responsibility into its own private method
Next Steps
Apply the "Can I implement this change?" test to your recent code
Write focused unit tests for each extracted responsibility
Establish a complexity threshold (like score > 10) for your team
Your Turn!
The Refactoring Challenge
Here's a method that's crying out for refactoring. Can you spot the code smells?
def process_order(items, customer_type, discount_code)
total = 0
items.each { |item| total += item.price * item.quantity }
# Apply customer discount
if customer_type == 'premium'
total *= 0.9
elsif customer_type == 'vip'
total *= 0.8
end
# Apply discount code
if discount_code == 'SAVE10'
total *= 0.9
elsif discount_code == 'SAVE20'
total *= 0.8
end
# Calculate tax
tax = total * 0.08
final_total = total + tax
# Send confirmation email
send_email(customer_email, "Order confirmed for $#{final_total}")
final_total
end
💬 Discussion Prompts:
How many responsibilities can you count in this method?
What would happen if you needed to add a new customer type?
How would you test the email sending separately from the calculation?
What's Next?
Next week: "The Testing Trap: Why 100% Coverage Might Be Hurting Your Code" - We'll explore the balance between thorough testing and over-testing, and discover metrics that actually predict code quality.
🔧 Useful Resources:
Found this useful? Share it with a fellow developer! And don't forget to reply with your refactoring challenge solution – I read every response.
Happy coding!
Tips and Notes:
Pro Tip: Start refactoring with the smallest, most obvious extraction – momentum builds confidence
Remember: Working code isn't the same as good code – good code continues to work as requirements evolve