Introduction
Code smells are surface indications that usually correspond to a deeper problem in the system. They are not bugs but rather weaknesses in the code that may be slowing down development or increasing the risk of bugs.
Duplicated Code
Same or similar code appearing in multiple places creates maintenance problems.
# Smell: Duplicated validation logic
def create_user(name, email):
if not name:
raise ValueError("Name is required")
if not email or "@" not in email:
raise ValueError("Valid email is required")
print(f"Creating user: {name}")
def update_user(user_id, name, email):
if not name:
raise ValueError("Name is required")
if not email or "@" not in email:
raise ValueError("Valid email is required")
print(f"Updating user: {user_id}")
# Fix: Extract to shared validation function
def validate_user_input(name, email):
if not name:
raise ValueError("Name is required")
if not email or "@" not in email:
raise ValueError("Valid email is required")
def create_user(name, email):
validate_user_input(name, email)
print(f"Creating user: {name}")
def update_user(user_id, name, email):
validate_user_input(name, email)
print(f"Updating user: {user_id}")
Long Methods and Functions
Functions that do too much become hard to understand and test.
# Smell: A 100-line function doing everything
def process_order(order):
# Validate order
if not order.items:
return "No items"
for item in order.items:
if item.quantity <= 0:
return "Invalid quantity"
# Calculate total
total = sum(item.price * item.quantity for item in order.items)
# Apply discount
if order.customer.is_vip:
total *= 0.9
# Check inventory
for item in order.items:
if item.stock < item.quantity:
return f"Out of stock: {item.name}"
# Process payment
if order.payment_method == "credit_card":
print(f"Processing credit card")
elif order.payment_method == "paypal":
print(f"Processing PayPal")
# Update inventory
for item in order.items:
item.stock -= item.quantity
# Send confirmation
print(f"Sending email to {order.customer.email}")
# Log order
print(f"Logging order")
return f"Order processed: ${total}"
# Fix: Split into smaller functions
def validate_order(order):
if not order.items:
return "No items"
for item in order.items:
if item.quantity <= 0:
return "Invalid quantity"
return None
def calculate_total(order):
total = sum(item.price * item.quantity for item in order.items)
return total * 0.9 if order.customer.is_vip else total
def check_inventory(order):
for item in order.items:
if item.stock < item.quantity:
return f"Out of stock: {item.name}"
return None
def process_payment(order, total):
print(f"Processing {order.payment_method}")
def update_inventory(order):
for item in order.items:
item.stock -= item.quantity
def send_confirmation(order):
print(f"Sending email to {order.customer.email}")
def process_order(order):
if error := validate_order(order):
return error
if error := check_inventory(order):
return error
total = calculate_total(order)
process_payment(order, total)
update_inventory(order)
send_confirmation(order)
return f"Order processed: ${total}"
God Classes
Classes that have too many responsibilities become hard to maintain.
# Smell: One class doing everything
class CustomerManager:
def create_customer(self, data):
# Database logic
print("Inserting into database")
# Email logic
print("Sending welcome email")
# Validation logic
if not data.get("email"):
raise ValueError("Email required")
# Logging logic
print("Logging customer creation")
# Notification logic
print("Notifying sales team")
# Fix: Separate into focused classes
class CustomerValidator:
def validate(self, data):
if not data.get("email"):
raise ValueError("Email required")
return True
class CustomerRepository:
def save(self, customer):
print("Inserting into database")
class CustomerNotifier:
def send_welcome_email(self, email):
print(f"Sending welcome email to {email}")
def notify_sales(self, customer):
print("Notifying sales team")
class CustomerService:
def __init__(self):
self.validator = CustomerValidator()
self.repository = CustomerRepository()
self.notifier = CustomerNotifier()
def create_customer(self, data):
self.validator.validate(data)
customer = self.repository.save(data)
self.notifier.send_welcome_email(data["email"])
self.notifier.notify_sales(customer)
return customer
Magic Numbers
Hardcoded numbers without explanation create confusion.
# Smell: Magic numbers everywhere
def calculate_shipping(weight):
if weight < 5:
return 10
elif weight < 20:
return 15
elif weight < 100:
return 25
return 50
# Fix: Use named constants
class ShippingConstants:
LIGHT_WEIGHT_THRESHOLD = 5
MEDIUM_WEIGHT_THRESHOLD = 20
HEAVY_WEIGHT_THRESHOLD = 100
LIGHT_WEIGHT_RATE = 10
MEDIUM_WEIGHT_RATE = 15
HEAVY_WEIGHT_RATE = 25
EXTRA_HEAVY_RATE = 50
def calculate_shipping(weight):
if weight < ShippingConstants.LIGHT_WEIGHT_THRESHOLD:
return ShippingConstants.LIGHT_WEIGHT_RATE
elif weight < ShippingConstants.MEDIUM_WEIGHT_THRESHOLD:
return ShippingConstants.MEDIUM_WEIGHT_RATE
elif weight < ShippingConstants.HEAVY_WEIGHT_THRESHOLD:
return ShippingConstants.HEAVY_WEIGHT_RATE
return ShippingConstants.EXTRA_HEAVY_RATE
Practice Problems
- Identify and fix duplicated code in a given snippet.
- Refactor a long function into smaller methods.
- Split a God class into focused, single-responsibility classes.
- Replace magic numbers with named constants.
- Fix a class with too many instance variables (feature envy smell).