Software Dev

Code Smells in Java: Complete Detection & Fixing Guide for Production Systems

Code smells are not bugs — they are structural warning signs that indicate deeper problems. Left unaddressed, each smell slows development, invites regressions, and compounds into technical debt that eventually halts new feature delivery. This guide catalogues 15+ of the most impactful Java code smells with their root causes, detection methods, and precise refactoring fixes using production-ready Spring Boot code.

Md Sanwar Hossain April 10, 2026 17 min read Software Dev

1. What Are Code Smells (and Why They Compound)

Martin Fowler coined the term in Refactoring: Improving the Design of Existing Code (1999). A code smell is "a surface indication that usually corresponds to a deeper problem in the system." It is not a bug — the code may work correctly today. But a smell signals that future changes will be harder, riskier, or more expensive than they need to be.

The compound effect is the real danger. A single God Class may be manageable in month 1. By month 18, a single change requires understanding 1,200 lines of interleaved concerns, touching six test files, and competing with three teammates editing the same file. Google's engineering research found that files with cyclomatic complexity above 10 contributed to 23% of all production bugs despite representing only 8% of the codebase.

2. Bloaters: God Class, Long Method, Long Parameter List

God Class

A God Class accumulates responsibilities from multiple domains until it becomes the "go-to" class for everything. It is the single biggest impediment to parallel team development because every team member edits the same file. A Spring Boot UserService reaching 1,200 lines typically owns authentication logic, profile management, notification dispatch, order history queries, and audit logging — five different domains, five different reasons to change.

// BAD: God Class — UserService owns everything
@Service
public class UserService {
    // Auth (1 actor)
    public AuthToken login(String email, String password) { /* 40 lines */ }
    public void resetPassword(String email) { /* 30 lines */ }
    // Profile (2nd actor)
    public void updateProfile(Long userId, ProfileDto dto) { /* 25 lines */ }
    public String uploadAvatar(Long userId, MultipartFile file) { /* 35 lines */ }
    // Orders (3rd actor)
    public List<Order> getOrderHistory(Long userId) { /* 20 lines */ }
    // Notifications (4th actor)
    public void sendWelcomeEmail(User user) { /* 15 lines */ }
    public void sendPasswordResetEmail(String email) { /* 15 lines */ }
    // Audit (5th actor)
    public void logUserAction(Long userId, String action) { /* 10 lines */ }
    // ... 1000+ more lines
}

// GOOD: Extract Class — one service per business actor
@Service
public class AuthService {
    public AuthToken login(String email, String password) { /* ... */ }
    public void resetPassword(String email) { /* ... */ }
}

@Service
public class UserProfileService {
    public void updateProfile(Long userId, ProfileDto dto) { /* ... */ }
    public String uploadAvatar(Long userId, MultipartFile file) { /* ... */ }
}

@Service
public class OrderHistoryService {
    public List<Order> getOrderHistory(Long userId) { /* ... */ }
}

@Service
public class UserNotificationService {
    public void sendWelcomeEmail(User user) { /* ... */ }
    public void sendPasswordResetEmail(String email) { /* ... */ }
}

Long Method

A method is too long when you cannot understand its full intent in 30 seconds, when it mixes orchestration with business logic, or when it exceeds 20 lines. The refactoring is always Extract Method — identify cohesive sub-steps and move each to a well-named private method.

// BAD: 50-line processOrder() with mixed concerns
@Service
public class OrderService {
    public OrderResponse processOrder(OrderRequest request) {
        // Validation block (lines 1-15)
        if (request.getItems() == null || request.getItems().isEmpty()) {
            throw new ValidationException("Order must have items");
        }
        for (OrderItem item : request.getItems()) {
            if (item.getQuantity() <= 0) throw new ValidationException("Qty must be positive");
            if (!inventoryRepository.isAvailable(item.getProductId(), item.getQuantity())) {
                throw new InsufficientInventoryException(item.getProductId());
            }
        }
        // Pricing block (lines 16-30)
        BigDecimal subtotal = BigDecimal.ZERO;
        for (OrderItem item : request.getItems()) {
            Product p = productRepository.findById(item.getProductId()).orElseThrow();
            subtotal = subtotal.add(p.getPrice().multiply(BigDecimal.valueOf(item.getQuantity())));
        }
        BigDecimal tax = subtotal.multiply(new BigDecimal("0.08"));
        BigDecimal total = subtotal.add(tax);
        // Persistence block (lines 31-40)
        Order order = new Order(request.getCustomerId(), request.getItems(), total);
        order = orderRepository.save(order);
        // Notification block (lines 41-50)
        emailService.sendOrderConfirmation(order);
        return new OrderResponse(order.getId(), total);
    }
}

// GOOD: Extract Method — each step is a named, testable unit
@Service
public class OrderService {
    public OrderResponse processOrder(OrderRequest request) {
        validateOrder(request);
        BigDecimal total = calculateTotal(request.getItems());
        Order order = persistOrder(request, total);
        notifyCustomer(order);
        return new OrderResponse(order.getId(), total);
    }

    private void validateOrder(OrderRequest request) {
        if (request.getItems() == null || request.getItems().isEmpty()) {
            throw new ValidationException("Order must have items");
        }
        request.getItems().forEach(item -> {
            if (item.getQuantity() <= 0) throw new ValidationException("Qty must be positive");
            if (!inventoryRepository.isAvailable(item.getProductId(), item.getQuantity())) {
                throw new InsufficientInventoryException(item.getProductId());
            }
        });
    }

    private BigDecimal calculateTotal(List<OrderItem> items) {
        BigDecimal subtotal = items.stream()
            .map(item -> productRepository.findById(item.getProductId()).orElseThrow()
                .getPrice().multiply(BigDecimal.valueOf(item.getQuantity())))
            .reduce(BigDecimal.ZERO, BigDecimal::add);
        return subtotal.add(subtotal.multiply(new BigDecimal("0.08")));
    }

    private Order persistOrder(OrderRequest request, BigDecimal total) {
        return orderRepository.save(new Order(request.getCustomerId(), request.getItems(), total));
    }

    private void notifyCustomer(Order order) {
        emailService.sendOrderConfirmation(order);
    }
}

Long Parameter List

// BAD: caller must know the order and meaning of 7 parameters
public void createUser(String name, String email, String phone,
                       String address, String role, boolean active, int tier) { }

// GOOD: Introduce Parameter Object — self-documenting, extensible
public record CreateUserCommand(
    String name,
    String email,
    String phone,
    String address,
    String role,
    boolean active,
    int tier
) {}

public void createUser(CreateUserCommand command) { }

3. OO Abusers: Switch Statements & Refused Bequest

Switch Statements

A switch (or if-else chain) on a type tag is a missed opportunity for polymorphism. Every time you add a new type, you must find and update every switch statement that mentions the type — a classic Shotgun Surgery scenario. The fix is to move the type-specific behaviour into subclasses or implementations.

// BAD: switch on shape type spread across multiple methods
public double area(Shape shape) {
    switch (shape.getType()) {
        case "CIRCLE": return Math.PI * shape.getRadius() * shape.getRadius();
        case "RECTANGLE": return shape.getWidth() * shape.getHeight();
        case "TRIANGLE": return 0.5 * shape.getBase() * shape.getHeight();
        default: throw new IllegalArgumentException("Unknown shape");
    }
}

// GOOD: Replace Conditional with Polymorphism
public interface Shape {
    double area();
}

public record Circle(double radius) implements Shape {
    @Override public double area() { return Math.PI * radius * radius; }
}

public record Rectangle(double width, double height) implements Shape {
    @Override public double area() { return width * height; }
}

public record Triangle(double base, double height) implements Shape {
    @Override public double area() { return 0.5 * base * height; }
}

// Caller — no switch, works with any future Shape implementation
public double totalArea(List<Shape> shapes) {
    return shapes.stream().mapToDouble(Shape::area).sum();
}

Refused Bequest

Refused Bequest occurs when a subclass inherits methods from a parent but throws exceptions or provides empty stubs for them. This violates LSP and is a sign that inheritance was used for code reuse rather than for modelling an "is-a" relationship.

// BAD: Square refuses Rectangle's width/height independence
public class Rectangle {
    protected int width, height;
    public void setWidth(int w) { this.width = w; }
    public void setHeight(int h) { this.height = h; }
    public int area() { return width * height; }
}

public class Square extends Rectangle {
    @Override public void setWidth(int w) {
        // Refused bequest: forces both to change
        this.width = w;
        this.height = w; // breaks caller assumptions!
    }
}

// GOOD: separate interface hierarchy using composition
public interface Shape { int area(); }

public record Rectangle(int width, int height) implements Shape {
    @Override public int area() { return width * height; }
}

public record Square(int side) implements Shape {
    @Override public int area() { return side * side; }
}

4. Change Preventers: Divergent Change, Shotgun Surgery, Parallel Inheritance

Divergent Change

Divergent Change means one class is modified for many different reasons over time. If your ReportService changed last sprint for a new report format, the sprint before for a data source change, and the sprint before that for a new export target, it has three distinct responsibilities that should be separated. The fix is Extract Class for each axis of change.

Shotgun Surgery

Shotgun Surgery is the opposite of Divergent Change: one business change requires editing many classes simultaneously. Adding a new payment method that forces edits to PaymentController, PaymentService, PaymentRepository, PaymentValidator, NotificationService, AuditLogger, and ReportGenerator is pure Shotgun Surgery. Consolidate the scattered behaviour into one class.

// BAD: Adding Stripe payment touches 5 different files
// File 1: PaymentValidator.java — add STRIPE case
// File 2: PaymentService.java — add stripe processing
// File 3: NotificationService.java — add stripe receipt template
// File 4: AuditLogger.java — add stripe audit category
// File 5: ReportGenerator.java — add stripe to report

// GOOD: Encapsulate all payment-provider logic in one strategy
@Component
public class StripePaymentStrategy implements PaymentStrategy {
    @Override public boolean supports(String provider) { return "STRIPE".equals(provider); }
    @Override public void validate(PaymentRequest req) { /* stripe-specific validation */ }
    @Override public PaymentResult process(PaymentRequest req) { /* stripe API call */ }
    @Override public String receiptTemplate() { return "stripe-receipt"; }
    @Override public AuditCategory auditCategory() { return AuditCategory.CARD_PAYMENT; }
}

Parallel Inheritance Hierarchies

Every time you add a subclass in one hierarchy, you are forced to add a corresponding subclass in another hierarchy. Adding Dog extends Animal also forces you to add DogHandler extends AnimalHandler. Fix by merging the handler logic into the Animal hierarchy or using a Visitor pattern.

5. Dispensables: Dead Code, Speculative Generality, Duplicate Code

Dead Code

Dead code — commented-out blocks, unused private methods, unreachable branches — creates cognitive noise. Every reader must parse it to determine whether it is intentional. IntelliJ IDEA's "Find Usages" (Alt+F7) and SonarQube's "unused code" rules detect it automatically. The fix is simple: delete it. Version control preserves history if it is ever needed again.

// BAD: commented-out dead code obscures intent
@Service
public class PricingService {
    public BigDecimal calculate(Product product, int qty) {
        // Old logic — kept "just in case"
        // BigDecimal base = product.getCostPrice().multiply(new BigDecimal("1.3"));
        // if (qty > 10) base = base.multiply(new BigDecimal("0.95"));
        // return base;
        return product.getListPrice().multiply(BigDecimal.valueOf(qty));
    }

    // Never called — remove it
    private BigDecimal applyLegacyDiscount(BigDecimal price) {
        return price.multiply(new BigDecimal("0.9"));
    }
}

// GOOD: clean, intentional code only
@Service
public class PricingService {
    public BigDecimal calculate(Product product, int qty) {
        return product.getListPrice().multiply(BigDecimal.valueOf(qty));
    }
}

Speculative Generality

Speculative Generality happens when developers add abstractions "for future flexibility" that never materialises. Abstract base classes with a single concrete subclass, hook methods that are never overridden, and complex configuration frameworks for a feature that only ever has one variant — these all add complexity with zero present value. Apply YAGNI: build generality when you have a second concrete use case, not before.

Duplicate Code

// BAD: same email validation duplicated in three controllers
@RestController
public class UserController {
    @PostMapping("/users")
    public ResponseEntity<?> createUser(@RequestBody UserRequest req) {
        if (req.getEmail() == null || !req.getEmail().matches("^[\\w.-]+@[\\w.-]+\\.[a-z]{2,}$")) {
            return ResponseEntity.badRequest().body("Invalid email");
        }
        // ...
    }
}

@RestController
public class AdminController {
    @PostMapping("/admins")
    public ResponseEntity<?> createAdmin(@RequestBody AdminRequest req) {
        // Same validation copy-pasted!
        if (req.getEmail() == null || !req.getEmail().matches("^[\\w.-]+@[\\w.-]+\\.[a-z]{2,}$")) {
            return ResponseEntity.badRequest().body("Invalid email");
        }
        // ...
    }
}

// GOOD: Extract Method into shared validator
@Component
public class EmailValidator {
    private static final String EMAIL_REGEX = "^[\\w.-]+@[\\w.-]+\\.[a-z]{2,}$";

    public void validate(String email) {
        if (email == null || !email.matches(EMAIL_REGEX)) {
            throw new ValidationException("Invalid email format: " + email);
        }
    }
}

@RestController
public class UserController {
    private final EmailValidator emailValidator;
    // Inject and reuse — single source of truth
}

6. Couplers: Feature Envy & Message Chains

Feature Envy

Feature Envy is when a method in class A uses more fields and methods from class B than from its own class. This is a strong signal that the method belongs in class B. The refactoring is Move Method.

// BAD: ShippingService is envious of Order's data
@Service
public class ShippingService {
    public BigDecimal calculateCost(Order order) {
        // Uses 5 fields from Order — this logic belongs in Order!
        double weight = order.getTotalWeight();
        String zone = order.getDeliveryAddress().getZone();
        boolean isPremium = order.getCustomer().isPremiumMember();
        int itemCount = order.getItems().size();
        boolean isFragile = order.getItems().stream().anyMatch(OrderItem::isFragile);
        // ... cost calculation using all of Order's data
        return BigDecimal.valueOf(weight * 2.5).add(isFragile ? BigDecimal.TEN : BigDecimal.ZERO);
    }
}

// GOOD: Move Method to Order — the data and behaviour co-locate
public class Order {
    public BigDecimal calculateShippingCost() {
        double weight = this.getTotalWeight();
        boolean isFragile = this.items.stream().anyMatch(OrderItem::isFragile);
        return BigDecimal.valueOf(weight * 2.5).add(isFragile ? BigDecimal.TEN : BigDecimal.ZERO);
    }
}

@Service
public class ShippingService {
    public BigDecimal calculateCost(Order order) {
        return order.calculateShippingCost(); // delegate — no envy
    }
}

Message Chains & the Law of Demeter

// BAD: message chain — fragile, exposes internal structure
String city = order.getCustomer().getAddress().getCity().toUpperCase();
// Any null in the chain throws NullPointerException.
// Change in Address structure breaks all callers.

// GOOD: Hide delegate — Law of Demeter
public class Order {
    public String getCustomerCity() {
        return Optional.ofNullable(customer)
            .map(Customer::getAddress)
            .map(Address::getCity)
            .map(String::toUpperCase)
            .orElse("UNKNOWN");
    }
}
// Caller now: order.getCustomerCity() — clean, null-safe, encapsulated

7. Code Smell Detection Tools

Manual code review catches some smells, but automated tools catch far more consistently. The following tools integrate into your standard Java build pipeline and provide immediate feedback to developers before code reaches review.

8. Smell-to-Refactoring Mapping Table

9. How Code Smells Compound Over Time

The growth of a God Class follows a predictable curve that teams only recognize in retrospect. Consider a real scenario we see repeatedly in Spring Boot microservices:

Month 1:  UserService — 150 lines — clean, SRP respected
Month 3:  +Auth logic added — 300 lines — "just for now"
Month 6:  +Notifications added — 500 lines — "no time to refactor"
Month 9:  +Order history query added — 750 lines — "it's related to users"
Month 12: +Audit logging added — 950 lines — "almost everything is here anyway"
Month 18: +Report generation — 1,200 lines — development velocity drops 60%

Feature cycle time in Month 1:  2 days per feature
Feature cycle time in Month 18: 8 days per feature (same complexity features)
Reason: understanding the blast radius of any change takes 2+ days alone

The velocity curve does not drop sharply — it flattens gradually until the team begins blaming "complexity" and "legacy" for slowdowns that are purely structural. At month 18, extracting the five responsibilities is a 2-sprint effort. At month 6, it would have been 2 days. The compound cost of delay is the core argument for incremental, continuous refactoring over big-bang cleanup sprints.

10. Common Mistakes When Fixing Code Smells

• Changing behaviour while refactoring: the golden rule of refactoring is never change behaviour and structure in the same commit. Make the structural change first, verify all tests pass, then make the behaviour change in a separate commit. Violating this rule makes regressions untraceable.
• Refactoring without tests: refactoring without a test suite is rewriting with extra steps. Without tests, you cannot know whether you broke something. Write characterization tests that capture current behaviour before touching legacy code.
• Big-bang refactoring in one PR: a 50-file PR refactoring everything at once is impossible to review, high-risk to merge, and demoralising if it needs to be reverted. Break it into a sequence of small PRs, each independently deployable.
• Fixing smells in unrelated code: scope creep in refactoring is a real risk. If you are fixing a bug in OrderService, refactor only OrderService. Touching ProductService "while you are at it" creates a review burden and a risk surface that has nothing to do with the original change.
• Renaming without IDE refactoring tools: manual find-and-replace renames miss dynamic references, string literals, and documentation. Always use the IDE's Rename refactoring (Shift+F6 in IntelliJ) to get safe, complete renames across the entire codebase.

11. FAQ & Key Takeaways

What is the most dangerous code smell in Java?

God Class and Shotgun Surgery are the most damaging in production codebases. God Class prevents parallel team work and makes every change a high-risk event. Shotgun Surgery means a single business change requires coordinating edits across many files and teams, dramatically increasing merge conflict risk.

How do I detect code smells automatically in Java?

Integrate SonarQube or SonarCloud into your CI pipeline as a quality gate. Configure PMD and Checkstyle as Maven or Gradle plugins to run on every build. For architectural smells, add ArchUnit tests to your JUnit 5 test suite to enforce layer boundaries and forbidden dependencies at the package level.

Is duplicate code always a code smell?

Not always. The Rule of Three applies: the first time you write something, write it. The second time you write something similar, duplicate it. The third time you encounter the same pattern, extract a shared abstraction. Premature extraction based on superficial similarity can create worse coupling than the duplication it replaced.

When should I fix code smells vs ship features?

Use the Boy Scout Rule: fix smells in code you are already touching as part of a feature change. If refactoring a class will take longer than the feature itself, time-box the refactoring to 20% of the total sprint capacity. Avoid dedicated "refactoring sprints" — teams lose product confidence when they see nothing shipped for a week.

Key Takeaways:

• Every code smell has a specific refactoring technique — learn the smell-to-fix mapping.
• The Golden Rule: never change behaviour and structure in the same commit.
• God Class is the most expensive smell because it scales with team size — every developer is blocked by the same file.
• Automate smell detection with SonarQube quality gates, PMD, and Checkstyle in your build pipeline.
• Apply the Rule of Three before extracting shared abstractions from duplicated code.
• Write characterization tests before refactoring any legacy code that lacks test coverage.

Md Sanwar Hossain

Software Engineer · Java · Spring Boot · Microservices

All Posts