Md Sanwar Hossain

Software Engineer · Java · Spring Boot · Microservices

Code Smells & Refactoring in Java: Detecting and Fixing the 12 Most Dangerous Anti-Patterns

In a real incident, a critical bug fix that should have taken 2 hours took 3 days. The root cause wasn't the complexity of the logic — it was the code. A 2,800-line OrderService class with 47 methods, no clear ownership, and business logic interwoven with infrastructure code. The engineer had to understand the entire class just to safely change 10 lines. This is the hidden cost of accumulated code smells. This post identifies the 12 most dangerous smells in Java production codebases and shows the specific refactoring moves to eliminate them.

1. Why Code Smells Cause Production Incidents

Code smells are not just aesthetic problems. They have measurable operational consequences. Research consistently shows that files with high complexity (cyclomatic complexity >15) are 3–4x more likely to contain bugs. Long methods with many conditional branches are harder to test, leading to lower test coverage for edge cases — the exact cases that cause production failures.

Beyond bugs, smells slow down the entire engineering organization. A team with a God Class in the critical path has a de facto bottleneck: only engineers who understand the whole class can safely make changes. Onboarding time increases. Fear of change leads to symptom-fixing instead of root-cause fixing. Technical debt compounds.

2. God Class: The 2,000-Line OrderService

A God Class knows too much and does too much. It typically starts as a reasonable service class and grows unbounded over years as "just one more thing" gets added. The hallmarks: 500+ lines, 20+ methods, 10+ injected dependencies, multiple unrelated responsibilities tangled together.

How to identify it: Count constructor parameters. If a Spring service has 8+ injected dependencies, it's almost certainly handling too many responsibilities.

// God Class symptom — 8 injected dependencies
@Service
public class OrderService {
    private final OrderRepository orderRepository;
    private final CustomerRepository customerRepository;
    private final InventoryService inventoryService;
    private final PaymentGateway paymentGateway;
    private final NotificationService notificationService;
    private final ShippingService shippingService;
    private final InvoiceService invoiceService;
    private final AuditService auditService;
    // + 40 methods handling all of the above...
}

// Refactored — Single Responsibility: each service owns one domain
@Service public class OrderPlacementService { /* only handles order placement */ }
@Service public class OrderPaymentService   { /* only handles payment coordination */ }
@Service public class OrderFulfillmentService { /* only handles fulfillment */ }
@Service public class OrderNotificationService { /* only handles notifications */ }

Refactoring move: Extract Class. Group related methods and their corresponding dependencies together. Move them into a new class. Update callers to use the new class directly, or wire the new classes through the original service as delegates.

3. Long Method: When Methods Do Everything

A method longer than 20–30 lines is almost always doing too many things. Long methods are hard to understand, hard to test, and hard to modify safely. They typically mix different levels of abstraction — high-level business logic interspersed with low-level detail like string formatting or collection manipulation.

// Before — 80-line method doing validation, transformation, and persistence
public OrderConfirmation placeOrder(OrderRequest request) {
    // 15 lines of validation
    if (request.getItems() == null || request.getItems().isEmpty()) { ... }
    if (request.getCustomerId() == null) { ... }
    // ... 10 more validation lines

    // 20 lines of inventory checks
    for (OrderItem item : request.getItems()) {
        InventoryRecord inv = inventoryService.getRecord(item.getSkuId());
        if (inv.getAvailable() < item.getQuantity()) { ... }
        // ...
    }

    // 30 lines of order construction + saving + notification
    ...
}

// After — Extract Method at each seam
public OrderConfirmation placeOrder(OrderRequest request) {
    validateOrderRequest(request);           // private — 15 lines
    reserveInventory(request.getItems());    // private — 20 lines
    Order order = buildOrder(request);       // private — 10 lines
    orderRepository.save(order);
    notifyCustomer(order);                   // private — delegate to service
    return OrderConfirmation.from(order);
}

4. Feature Envy: Classes That Want to Be Another Class

A method has Feature Envy when it accesses the data of another class more than its own. It's a sign that the method belongs in the other class. The symptom is a method that calls someOtherObject.getX(), someOtherObject.getY(), and someOtherObject.getZ() repeatedly to make decisions.

// Feature Envy — DiscountCalculator is envious of Customer's data
public class DiscountCalculator {
    public BigDecimal calculate(Order order, Customer customer) {
        if (customer.getLoyaltyTier() == LoyaltyTier.GOLD
                && customer.getMemberSinceYears() > 3
                && customer.getTotalSpend().compareTo(new BigDecimal("10000")) > 0) {
            return order.getSubtotal().multiply(new BigDecimal("0.15"));
        }
        // more customer field accesses...
    }
}

// Fix — Move method to Customer where it belongs
public class Customer {
    public DiscountRate calculateEligibleDiscount() {
        // Customer knows its own data — no external access needed
        if (loyaltyTier == LoyaltyTier.GOLD && memberSinceYears > 3
                && totalSpend.compareTo(new BigDecimal("10000")) > 0) {
            return DiscountRate.of(15);
        }
        return DiscountRate.NONE;
    }
}

5. Primitive Obsession: Using Strings for Domain Concepts

Primitive Obsession is using primitive types (String, int, BigDecimal) to represent domain concepts that deserve their own type. The canonical Java example: using String for email addresses, phone numbers, or account IDs — then manually validating format in every method that receives them.

// Primitive Obsession — String for domain concepts
public void sendInvoice(String customerId, String email, BigDecimal amount, String currency) {
    if (!email.matches("^[\\w.-]+@[\\w.-]+\\.[a-z]{2,}$")) throw new InvalidEmailException();
    // Validation repeated at every call site
}

// Fix — Replace Primitive with Object (Value Types)
public record CustomerId(String value) {
    public CustomerId { if (value == null || value.isBlank()) throw new IllegalArgumentException(); }
}
public record Email(String value) {
    public Email { if (!value.matches("^[\\w.-]+@[\\w.-]+\\.[a-z]{2,}$"))
        throw new IllegalArgumentException("Invalid email: " + value); }
}
public void sendInvoice(CustomerId customerId, Email email, Money amount) {
    // No validation needed — types guarantee validity by construction
}

6. Shotgun Surgery: One Change, Twenty Files

Shotgun Surgery is the inverse of the God Class — instead of one class doing too much, responsibility for one concept is scattered across too many classes. When adding a new payment method requires touching 15 files, that's Shotgun Surgery. The fix is to consolidate scattered responsibilities into a single class or use a plugin pattern.

The detection signal: you can draw a "change blast radius" — count how many classes a typical new feature requires modifying. If the average blast radius is >5 classes, there's scattered responsibility that needs consolidation.

7. Data Clumps: Parameters That Always Travel Together

When you see the same 3–4 parameters appearing together in multiple method signatures, that's a Data Clump. Those parameters belong in their own class:

// Data Clump — these 4 always travel together
void createShipment(String street, String city, String country, String postalCode);
void validateAddress(String street, String city, String country, String postalCode);
void lookupTaxRate(String street, String city, String country, String postalCode);

// Fix — Extract Class to remove the clump
public record Address(String street, String city, String country, String postalCode) {
    public Address {
        if (country == null || country.length() != 2)
            throw new IllegalArgumentException("ISO 2-letter country code required");
    }
}

void createShipment(Address destination);
void validateAddress(Address address);
void lookupTaxRate(Address address);

8. Dead Code and Speculative Generality

Dead Code is code that is never executed — unused methods, unreachable branches, commented-out blocks preserved "just in case." Dead code has a cognitive cost: every reader must evaluate whether it matters before concluding it doesn't. Use IntelliJ's "Unused declaration" inspection + code coverage to identify it systematically.

Speculative Generality is the YAGNI violation: interfaces created for "future implementations" that never come, abstract base classes with a single subclass, generic parameter infrastructure for a use case that never materialized. Delete it. Version control is your safety net. The cost of keeping speculative code far exceeds the cost of adding it back if the need arises.

9. Refactoring Patterns: Extract, Move, Replace

The three most important refactoring moves in the Java engineer's toolkit:

• Extract Method: Take a coherent section of a long method and give it a name. This is the most-applied refactoring — it raises the abstraction level of the parent method and makes each extracted piece independently testable.
• Move Method: If a method accesses another class's data more than its own, move it to that class. Eliminates Feature Envy and brings behavior closer to the data it operates on.
• Replace Primitive with Object: Create a dedicated value class for any concept currently represented by a primitive. Centralizes validation and makes method signatures self-documenting.
• Extract Class: When a class has too many responsibilities, create a new class and move the relevant methods and fields into it.
• Replace Temp with Query: Replace local variables that hold computed results with method calls. The variable was hiding a concept that deserves a name.

For a deeper perspective on how these refactoring patterns map to SOLID principles, see SOLID Principles in Java Spring Boot on this blog.

10. IDE Tools: IntelliJ Shortcuts for Java Engineers

IntelliJ IDEA's refactoring menu (Refactor → ...) provides automated support for every major refactoring move. These shortcuts eliminate the manual error-prone work of renaming across files:

• Ctrl+Alt+M / ⌘⌥M — Extract Method: select lines, extract to named method automatically
• F6 — Move Class: relocate a class to a different package, updating all imports
• Ctrl+Alt+C / ⌘⌥C — Extract Constant: replace magic literals with named constants
• Ctrl+Alt+P / ⌘⌥P — Extract Parameter: promote a local variable to a method parameter
• Shift+F6 — Rename: safe rename across all usages including XML configs and tests
• Analyze → Inspect Code → run "Unused declarations" inspection to surface dead code

11. Key Takeaways

• Code smells are measurable risks — high cyclomatic complexity correlates directly with bug density
• God Class: too many dependencies in the constructor is the easiest detection heuristic
• Long Method: if you need a comment to describe what a section does, that section should be its own method
• Feature Envy: methods that use another object's data more than their own belong in that other object
• Primitive Obsession: model domain concepts as dedicated types to centralize validation and improve API clarity
• Data Clumps: parameters that always appear together belong in a class — use Java Records for conciseness
• Add code quality gates in CI (SonarQube complexity thresholds) to prevent smell accumulation proactively

Last updated: March 2026 — Written by Md Sanwar Hossain