Code Smells — Recognizing Bad Design
Introduction
You open a codebase for the first time. A single class is 2,000 lines long. It processes orders, sends emails, formats receipts, handles database queries, and manages user authentication — all in one file. The class works. Tests pass. But every developer who touches it dreads the experience. A one-line change takes an hour because understanding the context requires reading 500 lines of unrelated logic.
You know something is wrong — but how do you describe the problem precisely? And more importantly, how do you know which principle to apply to fix it?
A code smell is a surface-level indicator of a deeper design problem. The term was coined by Kent Beck and popularized by Martin Fowler in Refactoring: Improving the Design of Existing Code. Code smells are not bugs — the code compiles, runs, and produces correct output. But the code's structure makes it hard to understand, hard to change, and easy to break accidentally.
Think of it like a musty smell in a house. The smell itself is not the problem — it points you toward a leak behind the wall. Similarly, a code smell points you toward a violated design principle. Once you learn to name the smell, you know exactly which principle to apply as the cure.
In this tutorial, you will learn to recognize the seven most critical code smells, understand which principles from S6 (SOLID) and S7 (Beyond SOLID) each one violates, and build a diagnostic checklist you can apply to any codebase. The next five tutorials in this section each tackle a specific refactoring technique with complete before-and-after code.
Why Code Smells Matter — The Compounding Effect
Code smells matter because they compound. One small violation of the Single Responsibility Principle leads to a class with two responsibilities. Over time, that class attracts more responsibilities because developers think: "The logic is already here — I will just add one more method." Eventually, the class has seven responsibilities, 2,000 lines, and touches every part of the system.
The cost is measurable and well-documented in software engineering research:
- Modification takes longer — Understanding a 2,000-line class to change one behavior takes hours instead of minutes. Studies show that developers spend roughly 70% of their time reading code, not writing it. A smelly codebase amplifies that reading time dramatically.
- Bugs multiply — Changing one responsibility accidentally breaks another because they share mutable state within the same class. A change to email formatting breaks order calculation because both live side by side.
- Testing becomes impractical — You cannot test email sending without also setting up order processing, database connections, and payment gateways. The feedback loop slows to minutes per test run.
- Onboarding slows — New developers cannot understand the system because classes do not match domain concepts. A
UserManagerthat handles authentication, profiles, notifications, sessions, and analytics does not map to any single real-world concept.
Recognizing smells early — before they compound — is what separates experienced designers from beginners. An experienced designer sees a class with two responsibilities and splits it immediately. A beginner waits until the class has seven, and by then the refactoring is a multi-week project with significant regression risk.
The Code Smell Catalog
1. God Class (Blob / Monster Class)
What it looks like: A single class with too many responsibilities — hundreds or thousands of lines, dozens of methods, and references to many unrelated objects. It becomes the central hub that every other class depends on.
Principle violated: Single Responsibility Principle (S6, Tutorial 2)
Example smell: A UserManager class that handles registration, authentication, profile updates, email notifications, session management, analytics tracking, and password resets. It has 40 methods and 15 fields spanning four unrelated concerns.
Why it hurts: Changing notification templates risks breaking authentication logic. Testing any single feature requires setting up the entire class with all its dependencies. Every developer works in the same file, causing constant merge conflicts.
Fix: Extract each responsibility into its own class — AuthenticationService, ProfileService, NotificationService, SessionManager, AnalyticsTracker. Each class gets the fields and methods that belong to its concern. We cover this refactoring in detail in the next tutorial (S9, Tutorial 2).
2. Long Method
What it looks like: A method with 50+ lines, multiple levels of nesting, and comments separating "sections" within the method body. The comments themselves are a clue — if you need comments to separate sections, those sections should be separate methods.
Principle violated: SRP at the method level, and KISS (S7, Tutorial 2)
Example smell: A processOrder() method that validates input (lines 1-15), checks inventory (lines 16-30), calculates tax (lines 31-45), processes payment (lines 46-65), generates a receipt (lines 66-80), sends a confirmation email (lines 81-95), and updates analytics (lines 96-110) — all in one method body.
Why it hurts: You cannot reuse the tax calculation logic elsewhere without copy-pasting. You cannot test inventory checking independently. A bug in the receipt generation obscures the stack trace because the method does too many things.
Fix: Extract each section into a named method — validateOrder(), checkInventory(), calculateTax(), processPayment(), generateReceipt(), sendConfirmation(). Each method should do one thing and be testable independently.
3. Feature Envy
What it looks like: A method that uses more data from another class than from its own class. It reaches into another object's fields repeatedly, accessing nested properties through chains of dots.
Principle violated: Law of Demeter (S7, Tutorial 4) and Information Expert from GRASP (S7, Tutorial 5). The Information Expert principle says: assign a responsibility to the class that has the information needed to fulfill it.
Example smell:
def calculate_discount(self, customer):
if customer.membership.tier == "GOLD" and customer.membership.years > 5:
return customer.order.total * customer.membership.discount_rate
This method knows intimate details about Customer's internal structure — its membership object, the tier field, the years field, the nested order.total. The method belongs on Customer, not here.
Fix: Move the logic to the class that owns the data — customer.calculate_discount(). The Customer class has all the information it needs. External code just asks for the result.
4. Shotgun Surgery
What it looks like: A single logical change requires modifications in many different classes scattered across the codebase. You want to add one feature, but you must edit 5-10 files in different packages.
Principle violated: Open/Closed Principle (S6, Tutorial 3) and DRY (S7, Tutorial 1). The related logic is scattered instead of consolidated.
Example smell: Adding a new payment method (Apple Pay) requires changes in PaymentProcessor, ReceiptGenerator, Analytics, EmailTemplates, AdminDashboard, and ConfigLoader. Miss one, and Apple Pay partially works — it processes payments but does not appear in analytics.
Why it hurts: Each change is small, but finding all the locations is error-prone. Developers forget one file, and the system becomes inconsistent.
Fix: Consolidate the scattered logic. Use the Strategy Pattern (S12, Tutorial 1) or polymorphism so a new payment method is a single new class that encapsulates all its specific behavior.
5. Switch-Case Smell (Type Code)
What it looks like: Multiple if/elif/else or switch/case blocks that branch on the same type or status field, repeated across several methods in the same class or across multiple classes.
Principle violated: Open/Closed Principle (S6, Tutorial 3) — adding a new type means modifying every switch block scattered across the codebase.
Example smell:
def calculate_fee(self, method):
if method == "credit_card":
return amount * 0.029
elif method == "paypal":
return amount * 0.035
elif method == "crypto":
return amount * 0.01
This exact same switch structure is duplicated in process(), validate(), generateReceipt(), and getDisplayName() — four methods all switching on the same method variable.
Fix: Replace with the Strategy Pattern — each payment method becomes a class with its own calculate_fee(), validate(), generate_receipt(), and display_name() methods. We cover this refactoring in S9, Tutorial 3.
6. Deep Inheritance Hierarchy
What it looks like: An inheritance chain 4+ levels deep where subclasses override parent behavior in fragile ways. Adding a new class that needs capabilities from two different branches is impossible without multiple inheritance (and its diamond problem) or code duplication.
Principle violated: Liskov Substitution Principle (S6, Tutorial 4) and Composition Over Inheritance (S7, Tutorial 6)
Example smell: Vehicle > MotorVehicle > Car > ElectricCar > TeslaModelS — each level overrides methods in conflicting ways. When you need a HybridCar that has both electric and gas behavior, the hierarchy cannot express it.
Why it hurts: The Fragile Base Class problem — a change to MotorVehicle ripples through every subclass. Adding cross-cutting capabilities requires copy-paste from sibling branches. Subclasses violate LSP because they override parent methods to do fundamentally different things.
Fix: Replace inheritance with composition — define capabilities as components (ElectricDrivetrain, GasDrivetrain, SelfDrivingModule) and compose them. A hybrid car has both ElectricDrivetrain and GasDrivetrain without any inheritance. We cover this in S9, Tutorial 4.
7. Tight Coupling
What it looks like: A class directly creates and depends on concrete implementations of other classes, making it impossible to test, swap, or extend components independently.
Principle violated: Dependency Inversion Principle (S6, Tutorial 6)
Example smell:
class OrderService:
def __init__(self):
self.db = MySQLDatabase() # Hard-coded to MySQL
self.emailer = SmtpEmailSender() # Hard-coded to SMTP
self.payment = StripeGateway() # Hard-coded to Stripe
The OrderService creates its own dependencies. It is chained to MySQL, SMTP, and Stripe. To test it, you need a running MySQL instance, an SMTP server, and a Stripe API key.
Why it hurts: You cannot write unit tests — every test is an integration test. Switching from MySQL to PostgreSQL means rewriting OrderService. The dependencies are hidden inside the constructor body; reading the class signature reveals nothing about what it needs.
Fix: Depend on abstractions (interfaces). Inject concrete implementations from outside via constructor injection. OrderService receives a Database, EmailSender, and PaymentGateway — it does not care which concrete implementation is behind them. We cover this in S9, Tutorial 5.
Visualization
Code Smells — Recognizing Bad Design
Code Smell Detection Checklist
Use this checklist when reviewing any code — yours or someone else's:
| Question | If Yes, Smell | Principle Violated |
|---|---|---|
| Does this class have more than one reason to change? | God Class | SRP |
| Is this method longer than 20-30 lines? | Long Method | SRP + KISS |
| Does this method use more data from another class than its own? | Feature Envy | Law of Demeter + GRASP |
| Does adding one feature require changes in 3+ files? | Shotgun Surgery | OCP + DRY |
Is the same if/switch block repeated in multiple methods? | Switch-Case Smell | OCP |
| Is the inheritance chain deeper than 3 levels? | Deep Inheritance | LSP + Composition over Inheritance |
Does this class create its own dependencies with new / direct construction? | Tight Coupling | DIP |
| Are there comments separating "sections" within a single method? | Long Method | Extract methods into named functions |
| Is there duplicated logic in multiple places? | DRY violation | DRY |
| Are there unused parameters, methods, or classes? | Dead Code / YAGNI | YAGNI |
Additional Smells Worth Knowing
Beyond the seven critical smells, here are three more that appear frequently in LLD interviews and code reviews:
Primitive Obsession
Using primitive types (str, int, float) instead of small domain objects. A phone_number: str has no validation, no formatting, and no type safety — you can accidentally pass an email address where a phone number is expected. The fix is a Value Object: PhoneNumber("555-1234") that validates on construction and provides formatting methods. You learned about Value Objects in S8 (Domain Modeling, Tutorial 2).
Data Clumps
The same group of fields appears together in multiple places: (street, city, state, zip_code) in User, Order, and Warehouse. These fields belong together as a concept — they should be an Address class. Data clumps are another signal to create a Value Object.
Refused Bequest
A subclass inherits from a parent but only uses a fraction of the parent's methods, ignoring or overriding the rest to do nothing. For example, a ReadOnlyList extending ArrayList but throwing exceptions from add(), remove(), and set(). This is a sign that the inheritance relationship is wrong — the child is not truly a specialization of the parent. It violates the Liskov Substitution Principle (S6, Tutorial 4). The fix is usually to replace inheritance with composition or to restructure the hierarchy.
A Quick Code Example — Spotting Multiple Smells
Let us look at a small class and identify all the smells it contains. Read the code first and try to spot them yourself before reading the analysis below.
# How many smells can you spot?
class ReportGenerator:
def __init__(self):
self.db = PostgresDatabase() # SMELL: Tight Coupling
def generate(self, report_type: str, user_id: int, format: str) -> str:
# --- Fetch data --- (SMELL: Long Method with comment-separated sections)
if report_type == "sales": # SMELL: Switch-Case
data = self.db.query("SELECT * FROM sales WHERE user_id = ?", user_id)
elif report_type == "inventory":
data = self.db.query(
"SELECT * FROM inventory WHERE warehouse_id = ?", user_id
)
elif report_type == "financial":
data = self.db.query(
"SELECT * FROM transactions WHERE org_id = ?", user_id
)
# --- Format output ---
if format == "pdf": # SMELL: Another Switch-Case chain
result = self._to_pdf(data)
elif format == "csv":
result = self._to_csv(data)
elif format == "html":
result = self._to_html(data)
# --- Send email --- (SMELL: God Class — report generator sends emails?)
self._send_email(user_id, result)
# --- Log ---
print(f"[LOG] Generated {report_type} report in {format} for user {user_id}")
return result
def _to_pdf(self, data): pass
def _to_csv(self, data): pass
def _to_html(self, data): pass
def _send_email(self, user_id, content): pass// Same smells in Java — how many can you spot?
public class ReportGenerator {
private final PostgresDatabase db; // SMELL: Tight Coupling
public ReportGenerator() {
this.db = new PostgresDatabase(); // Hard-coded concrete class
}
public String generate(String reportType, int userId, String format) {
// --- Fetch data --- (SMELL: Long Method + Switch-Case)
List<Map<String, Object>> data;
switch (reportType) {
case "sales":
data = db.query(
"SELECT * FROM sales WHERE user_id = ?", userId
);
break;
case "inventory":
data = db.query(
"SELECT * FROM inventory WHERE warehouse_id = ?", userId
);
break;
case "financial":
data = db.query(
"SELECT * FROM transactions WHERE org_id = ?", userId
);
break;
default:
throw new IllegalArgumentException(
"Unknown report type: " + reportType
);
}
// --- Format output --- (SMELL: Another Switch-Case)
String result;
switch (format) {
case "pdf": result = toPdf(data); break;
case "csv": result = toCsv(data); break;
case "html": result = toHtml(data); break;
default:
throw new IllegalArgumentException("Unknown format: " + format);
}
// SMELL: God Class — report generator sends emails?
sendEmail(userId, result);
System.out.printf(
"[LOG] Generated %s report in %s for user %d%n",
reportType, format, userId
);
return result;
}
private String toPdf(List<Map<String, Object>> data) { return ""; }
private String toCsv(List<Map<String, Object>> data) { return ""; }
private String toHtml(List<Map<String, Object>> data) { return ""; }
private void sendEmail(int userId, String content) { }
}Smells Found
- God Class —
ReportGeneratorhandles data fetching, formatting, emailing, and logging (4 responsibilities that should be in separate classes) - Long Method —
generate()has comment-separated sections doing completely different things: fetching data, formatting, sending email, logging - Switch-Case Smell (x2) — One switch on
report_type, another onformat. Adding a new report type or format requires modifying this method in two places - Tight Coupling —
PostgresDatabase()is created directly in the constructor; cannot test without a real PostgreSQL database running
In the next five tutorials, you will learn exactly how to fix each of these smells with specific, repeatable refactoring techniques.
Common Mistakes
Mistake 1: Over-Refactoring — Splitting Too Aggressively
The mistake: Creating a separate class for every 5-line method. A StringValidator, NumberValidator, EmailValidator, PhoneValidator when one InputValidator with clear methods would be perfectly cohesive.
Why it is wrong: Over-splitting creates indirection without benefit. You end up with 20 tiny classes that are hard to navigate, and the relationships between them are more complex than the original class.
The fix: Apply the SRP test: "Does this class have more than one reason to change?" If a class has one coherent responsibility, leave it alone — even if it has 10 methods. A BankAccount with deposit(), withdraw(), getBalance(), getStatement(), freeze(), and close() has one responsibility: managing an account. That is not a God Class.
Mistake 2: Treating All Smells as Equally Urgent
The mistake: Refactoring a 3-level inheritance chain in stable, rarely-changed utility code instead of fixing a God Class that is modified every sprint and causes merge conflicts daily.
Why it is wrong: Smells in code that never changes are harmless — they are technical debt that accrues no interest. Smells in hot paths (code changed frequently) are urgent because every modification risks breaking something.
The fix: Prioritize smells using the "change frequency" heuristic. Run git log --stat on your codebase to find which files change most often. Refactor those first. A perfect design in code that nobody ever modifies is wasted effort.
Mistake 3: Refactoring Without Tests
The mistake: Splitting a God Class into five classes without having tests that verify the original behavior. The refactoring introduces a subtle bug — the order of operations changes — and it goes undetected for weeks.
Why it is wrong: Refactoring means changing structure without changing behavior. Without tests, you have no way to verify the behavior is preserved.
The fix: Write characterization tests first — tests that capture the current behavior, right or wrong. These tests document what the code actually does today. Then refactor. Run the tests after every step. If a test breaks, you know exactly which step introduced the regression.
Key Takeaways
- A code smell is a surface indicator of a deeper design problem — the code works but is hard to understand, change, or test. The term was coined by Kent Beck and popularized by Martin Fowler.
- The seven critical smells are: God Class, Long Method, Feature Envy, Shotgun Surgery, Switch-Case Smell, Deep Inheritance, and Tight Coupling.
- Every smell maps to a specific principle violation — recognizing the smell tells you exactly which principle to apply as the fix. God Class violates SRP. Switch-Case violates OCP. Tight Coupling violates DIP.
- Do not over-refactor — only fix smells in code that changes frequently. Use
git logto find hot files. Stable code with smells is lower priority. - Always write characterization tests before refactoring — they are your safety net against introducing bugs during structural changes.
- This tutorial is the diagnostic guide. The next five tutorials are the prescriptions: God Object to SRP (S9.2), Switch-Case to Strategy (S9.3), Deep Inheritance to Composition (S9.4), Tight Coupling to DI (S9.5), and a complete Before/After Case Study (S9.6).