Recognizing Common Design Smells

21 min read

Object-Oriented Design and SOLID Review — spot structural warning signs in Java code that indicate poorly organized responsibilities, dependencies, or abstractions.

1. Introduction

A design smell is a structural warning sign in code that suggests responsibilities, dependencies, data ownership, or abstractions may be poorly organized.

A design smell does not always mean the code is incorrect.

The application may:

  • Compile successfully
  • Pass all tests
  • Work correctly in production
  • Produce the expected response

Yet the design may still make future changes unnecessarily difficult.

In real Java projects, design smells often become visible when developers repeatedly experience problems such as:

  • One small feature requires changes in many classes
  • A service class keeps growing with every requirement
  • The same business rule appears in multiple places
  • Classes need data from other classes more than their own data
  • Controllers contain business logic
  • Repository classes make business decisions
  • Utility classes accumulate unrelated methods
  • Methods require many unrelated parameters
  • Developers are afraid to modify one class because many modules depend on it
  • Unit testing requires large amounts of setup and mocking

A senior reviewer should not reject code merely because a known smell name can be applied.

The important questions are:

  • What responsibility is misplaced?
  • What future change will become difficult?
  • Is the smell already causing duplication, coupling, bugs, or test complexity?
  • Can the design be improved with a small, practical refactoring?
  • Would refactoring actually provide value, or would it create unnecessary abstractions?

Recognizing design smells helps developers prevent maintainability problems before they become expensive architectural problems.

2. What This Topic Means

Recognizing design smells means identifying recurring structural patterns that indicate weak object-oriented design.

Common smells include:

  • God class
  • Large service class
  • Long method
  • Feature envy
  • Shotgun surgery
  • Divergent change
  • Primitive obsession
  • Long parameter list
  • Data clumps
  • Inappropriate intimacy
  • Excessive coupling
  • Circular dependencies
  • Anemic domain model
  • Duplicate business rules
  • Middleman classes
  • Speculative generality
  • Utility-class dumping grounds
  • Boolean flag methods
  • Switch statements representing growing business variants
  • Leaky abstractions

Consider:

JAVA
@Service
public class OrderService {
    public void createOrder() {
    }

    public void cancelOrder() {
    }

    public void calculateTax() {
    }

    public void sendEmail() {
    }

    public void processPayment() {
    }

    public void reserveInventory() {
    }

    public void generateInvoice() {
    }

    public void exportCsv() {
    }
}

The class may initially appear convenient because all order-related functions are together.

Over time, however, it becomes responsible for:

  • Order workflow
  • Tax calculation
  • Payment integration
  • Inventory integration
  • Notifications
  • Invoice generation
  • Exporting

This is a design smell because multiple independently changing responsibilities are accumulating in one class.

The smell is not simply "the class has many methods."

The real problem is that unrelated reasons for change are being combined.

3. Why It Matters in Real Projects

Readability

Poorly designed classes are harder to understand.

A developer opening a 2,000-line service must determine:

  • Which methods belong together
  • Which state is shared
  • Which dependencies matter for each method
  • Which rules are business-critical
  • Which methods are safe to modify

Well-separated responsibilities reduce cognitive load.

Maintainability

Design smells increase change cost.

If adding a payment method requires changes in:

  • Controller
  • Service
  • Mapper
  • Validator
  • Utility
  • Repository
  • Notification service

the system may suffer from shotgun surgery.

Debugging

Duplicated business rules make debugging difficult.

Suppose eligibility logic exists in three classes:

JAVA
controller
service
batch processor

A production issue may occur in only one execution path.

Developers must first discover which implementation was used.

Reliability

Duplicated decisions can become inconsistent.

For example:

JAVA
amount > 10000

in one class and:

JAVA
amount >= 10000

in another.

Both may represent the same business rule but produce different results.

Testability

Large tightly coupled classes require many mocks.

A service constructor containing fifteen dependencies is often a warning sign that too many responsibilities are present.

Team Development

Large shared classes create merge conflicts.

If many developers modify the same service class for unrelated features, parallel development becomes difficult.

Scalability of the Codebase

Design smells accumulate.

A small God service can become a central dependency used by dozens of modules.

Once that happens, refactoring becomes much more expensive.

4. Core Concept

A design smell is best treated as a signal for investigation, not an automatic rule violation.

The reviewer should examine three things:

Responsibility

Ask:

What is this class or method responsible for?

If the answer requires many unrelated statements, the responsibility may be too broad.

Change Pattern

Ask:

Which types of changes cause this code to change?

For example, if OrderService changes whenever:

  • Payment rules change
  • Tax rules change
  • Email templates change
  • Inventory APIs change
  • Invoice requirements change

the class has multiple reasons for change.

Dependency Pattern

Ask:

What does this class know about?

A class that depends on:

  • Five repositories
  • Three external clients
  • Two mappers
  • Four business services
  • Security context
  • HTTP request
  • Event publisher

may be coordinating too many concerns.

Important Principle

Smells should be evaluated using context.

For example:

JAVA
300-line class

is not automatically bad.

A cohesive parser implementing one complex algorithm may reasonably contain substantial code.

Meanwhile:

JAVA
80-line class

may still have poor design if it mixes unrelated responsibilities.

Focus on cohesion, ownership, dependency direction, change patterns, and maintainability rather than arbitrary line-count rules.

5. Important Rules

  • Treat smells as investigation signals, not automatic defects.
  • Review responsibilities before reviewing class size.
  • Look for multiple independent reasons for change.
  • Identify duplicated business decisions, not only duplicated code.
  • Question constructors with many unrelated dependencies.
  • Question methods with many unrelated parameters.
  • Avoid extracting classes only to reduce line count.
  • Keep business decisions close to the domain responsibility they belong to.
  • Avoid central utility classes containing unrelated business logic.
  • Watch for classes that repeatedly manipulate another object's data.
  • Watch for feature changes that require edits across many modules.
  • Prefer cohesive services over one large "manager" service.
  • Avoid unnecessary abstraction when a simple implementation is sufficient.
  • Review switch statements that continuously grow with new business types.
  • Review boolean flag parameters because they often hide multiple behaviors.
  • Watch for repeated groups of parameters that may represent a missing concept.
  • Do not introduce design patterns merely to eliminate a smell name.
  • Refactor incrementally and preserve business behavior with tests.
  • Consider architectural boundaries, not only individual methods.
  • Use history and change frequency when evaluating design quality.

6. Bad Code Example

Consider an e-commerce checkout service.

JAVA
import java.math.BigDecimal;
import org.springframework.stereotype.Service;

@Service
public class CheckoutService {
    private final OrderRepository orderRepository;
    private final CustomerRepository customerRepository;
    private final InventoryRepository inventoryRepository;
    private final PaymentClient paymentClient;
    private final EmailService emailService;
    private final SmsService smsService;
    private final TaxService taxService;
    private final DiscountRepository discountRepository;
    private final AuditService auditService;

    public CheckoutService(
            OrderRepository orderRepository,
            CustomerRepository customerRepository,
            InventoryRepository inventoryRepository,
            PaymentClient paymentClient,
            EmailService emailService,
            SmsService smsService,
            TaxService taxService,
            DiscountRepository discountRepository,
            AuditService auditService) {
        this.orderRepository = orderRepository;
        this.customerRepository = customerRepository;
        this.inventoryRepository = inventoryRepository;
        this.paymentClient = paymentClient;
        this.emailService = emailService;
        this.smsService = smsService;
        this.taxService = taxService;
        this.discountRepository = discountRepository;
        this.auditService = auditService;
    }

    public CheckoutResult checkout(
            Long customerId,
            Long productId,
            int quantity,
            String couponCode,
            String paymentType,
            boolean sendEmail,
            boolean sendSms) {

        Customer customer =
                customerRepository.findById(customerId)
                        .orElseThrow();

        ProductInventory inventory =
                inventoryRepository.findByProductId(productId);

        if (inventory.getAvailableQuantity() < quantity) {
            throw new IllegalStateException(
                    "Insufficient inventory"
            );
        }

        BigDecimal amount =
                inventory.getPrice()
                        .multiply(
                                BigDecimal.valueOf(quantity)
                        );

        if (couponCode != null) {
            Discount discount =
                    discountRepository
                            .findByCode(couponCode);

            if (discount != null) {
                amount =
                        amount.subtract(
                                amount.multiply(
                                        discount.getPercentage()
                                )
                        );
            }
        }

        BigDecimal tax =
                taxService.calculate(
                        amount,
                        customer.getState()
                );

        BigDecimal finalAmount =
                amount.add(tax);

        boolean paymentSuccessful;

        if ("CARD".equals(paymentType)) {
            paymentSuccessful =
                    paymentClient
                            .chargeCard(
                                    customer.getId(),
                                    finalAmount
                            );
        } else if ("UPI".equals(paymentType)) {
            paymentSuccessful =
                    paymentClient
                            .chargeUpi(
                                    customer.getId(),
                                    finalAmount
                            );
        } else {
            throw new IllegalArgumentException(
                    "Unsupported payment type"
            );
        }

        if (!paymentSuccessful) {
            return new CheckoutResult(
                    null,
                    "PAYMENT_FAILED"
            );
        }

        inventory.setAvailableQuantity(
                inventory.getAvailableQuantity()
                        - quantity
        );

        inventoryRepository.save(inventory);

        Order order =
                new Order(
                        customerId,
                        productId,
                        quantity,
                        finalAmount
                );

        Order savedOrder =
                orderRepository.save(order);

        if (sendEmail) {
            emailService.sendOrderConfirmation(
                    customer.getEmail(),
                    savedOrder.getId()
            );
        }

        if (sendSms) {
            smsService.sendOrderConfirmation(
                    customer.getPhone(),
                    savedOrder.getId()
            );
        }

        auditService.record(
                "ORDER_CREATED",
                savedOrder.getId()
        );

        return new CheckoutResult(
                savedOrder.getId(),
                "SUCCESS"
        );
    }
}

This code may work correctly.

However, several design smells are visible.

7. Problems in the Bad Code

God Service Smell

CheckoutService performs too many responsibilities:

  • Customer lookup
  • Inventory validation
  • Price calculation
  • Discount application
  • Tax calculation
  • Payment selection
  • Payment processing
  • Inventory mutation
  • Order persistence
  • Email notification
  • SMS notification
  • Auditing

These responsibilities evolve for different reasons.

Long Parameter List

The method accepts:

JAVA
customerId
productId
quantity
couponCode
paymentType
sendEmail
sendSms

The caller must understand parameter ordering and multiple behaviors.

Primitive Obsession

Important concepts are represented as primitive or generic values.

Examples:

JAVA
String paymentType
String couponCode
boolean sendEmail
boolean sendSms

The payment method is a business concept but is represented as unrestricted text.

Boolean Flag Smell

JAVA
boolean sendEmail
boolean sendSms

These flags introduce conditional behavior inside the method.

Additional notification channels could make the method increasingly complex.

Growing Conditional Logic

JAVA
if ("CARD".equals(paymentType))
else if ("UPI".equals(paymentType))

Every new payment type requires changing the checkout service.

This may violate Open/Closed Principle if payment variants continue growing.

Excessive Dependencies

Nine dependencies suggest that the service may be coordinating too many responsibilities.

A large constructor is not automatically wrong, but it is a useful smell indicator.

Business Workflow and Technical Integrations Mixed

The method directly knows how payment variants are processed and which notification services should be called.

Difficult Unit Testing

Testing a simple checkout scenario requires mocking many collaborators.

Change Risk

A change to SMS behavior could require modifying the same class used for payment, pricing, and inventory changes.

This increases regression risk.

8. Code Review Findings

A senior reviewer should investigate the design rather than simply comment "method too long."

Finding 1: Checkout Service Has Low Cohesion

The service owns several responsibilities that can change independently.

Finding 2: Constructor Size Reflects Responsibility Growth

Nine dependencies suggest the class may be serving as an orchestration hub for unrelated implementation details.

Finding 3: Payment Selection Is Embedded in Checkout

Adding a new payment method requires editing the checkout workflow.

Finding 4: Notification Options Are Boolean Flags

The method signature is becoming configuration-heavy.

Finding 5: Domain Concepts Are Represented as Strings

paymentType accepts any string and requires runtime validation.

Finding 6: Pricing Logic Is Mixed With Workflow

Discount and tax calculations are embedded alongside infrastructure coordination.

Finding 7: Method Signature Contains Data Clump

The set:

JAVA
customerId
productId
quantity
couponCode
paymentType

represents a checkout request concept.

Finding 8: Refactoring Should Remain Proportional

The reviewer should not automatically introduce ten interfaces and a full event-driven architecture.

A small number of cohesive components can solve most of the problem.

9. Reviewer Comment Example

  • CheckoutService is currently handling pricing, payment selection, inventory updates, notifications, and auditing. Could we extract the independently changing responsibilities while keeping the checkout workflow here?
  • paymentType is currently a String and requires conditional branching. An enum or payment strategy boundary would make unsupported values harder to introduce.
  • The checkout method has several related parameters. Could we introduce a CheckoutCommand to make the input contract clearer?
  • The sendEmail/sendSms flags are starting to create multiple execution modes inside one method. Consider moving notification selection behind a dedicated notification component.
  • The large constructor appears to reflect several responsibilities in this service. Can we reduce dependencies by grouping cohesive behavior rather than simply hiding them behind a facade?

10. Improved Code

A practical refactoring can separate pricing, payment, and notification responsibilities without creating unnecessary layers.

Checkout Command

JAVA
public record CheckoutCommand(
        Long customerId,
        Long productId,
        int quantity,
        String couponCode,
        PaymentType paymentType) {
}

Payment Type

JAVA
public enum PaymentType {
    CARD,
    UPI
}

Pricing Service

JAVA
import java.math.BigDecimal;
import org.springframework.stereotype.Service;

@Service
public class CheckoutPricingService {
    private final DiscountRepository discountRepository;
    private final TaxService taxService;

    public CheckoutPricingService(
            DiscountRepository discountRepository,
            TaxService taxService) {
        this.discountRepository =
                discountRepository;
        this.taxService = taxService;
    }

    public BigDecimal calculateFinalAmount(
            BigDecimal unitPrice,
            int quantity,
            String couponCode,
            String state) {

        BigDecimal subtotal =
                unitPrice.multiply(
                        BigDecimal.valueOf(quantity)
                );

        BigDecimal discountedAmount =
                applyDiscount(
                        subtotal,
                        couponCode
                );

        BigDecimal tax =
                taxService.calculate(
                        discountedAmount,
                        state
                );

        return discountedAmount.add(tax);
    }

    private BigDecimal applyDiscount(
            BigDecimal amount,
            String couponCode) {

        if (couponCode == null
                || couponCode.isBlank()) {
            return amount;
        }

        Discount discount =
                discountRepository
                        .findByCode(couponCode);

        if (discount == null) {
            return amount;
        }

        return amount.subtract(
                amount.multiply(
                        discount.getPercentage()
                )
        );
    }
}

Payment Processor

JAVA
import java.math.BigDecimal;

public interface PaymentProcessor {
    boolean supports(PaymentType paymentType);

    boolean process(
            Long customerId,
            BigDecimal amount);
}

Card Processor

JAVA
import java.math.BigDecimal;
import org.springframework.stereotype.Component;

@Component
public class CardPaymentProcessor
        implements PaymentProcessor {

    private final PaymentClient paymentClient;

    public CardPaymentProcessor(
            PaymentClient paymentClient) {
        this.paymentClient = paymentClient;
    }

    @Override
    public boolean supports(
            PaymentType paymentType) {
        return paymentType == PaymentType.CARD;
    }

    @Override
    public boolean process(
            Long customerId,
            BigDecimal amount) {
        return paymentClient
                .chargeCard(
                        customerId,
                        amount
                );
    }
}

UPI Processor

JAVA
import java.math.BigDecimal;
import org.springframework.stereotype.Component;

@Component
public class UpiPaymentProcessor
        implements PaymentProcessor {

    private final PaymentClient paymentClient;

    public UpiPaymentProcessor(
            PaymentClient paymentClient) {
        this.paymentClient = paymentClient;
    }

    @Override
    public boolean supports(
            PaymentType paymentType) {
        return paymentType == PaymentType.UPI;
    }

    @Override
    public boolean process(
            Long customerId,
            BigDecimal amount) {
        return paymentClient
                .chargeUpi(
                        customerId,
                        amount
                );
    }
}

Payment Service

JAVA
import java.math.BigDecimal;
import java.util.List;
import org.springframework.stereotype.Service;

@Service
public class PaymentService {
    private final List<PaymentProcessor> processors;

    public PaymentService(
            List<PaymentProcessor> processors) {
        this.processors = processors;
    }

    public boolean process(
            PaymentType paymentType,
            Long customerId,
            BigDecimal amount) {

        PaymentProcessor processor =
                processors.stream()
                        .filter(
                                candidate ->
                                        candidate.supports(
                                                paymentType
                                        )
                        )
                        .findFirst()
                        .orElseThrow(
                                () ->
                                        new IllegalArgumentException(
                                                "Unsupported payment type: "
                                                        + paymentType
                                        )
                        );

        return processor.process(
                customerId,
                amount
        );
    }
}

Notification Service

JAVA
import org.springframework.stereotype.Service;

@Service
public class OrderNotificationService {
    private final EmailService emailService;
    private final SmsService smsService;

    public OrderNotificationService(
            EmailService emailService,
            SmsService smsService) {
        this.emailService = emailService;
        this.smsService = smsService;
    }

    public void sendOrderConfirmation(
            Customer customer,
            Long orderId) {

        emailService.sendOrderConfirmation(
                customer.getEmail(),
                orderId
        );

        smsService.sendOrderConfirmation(
                customer.getPhone(),
                orderId
        );
    }
}

Checkout Service

JAVA
import java.math.BigDecimal;
import org.springframework.stereotype.Service;

@Service
public class CheckoutService {
    private final CustomerRepository customerRepository;
    private final InventoryRepository inventoryRepository;
    private final OrderRepository orderRepository;
    private final CheckoutPricingService pricingService;
    private final PaymentService paymentService;
    private final OrderNotificationService notificationService;
    private final AuditService auditService;

    public CheckoutService(
            CustomerRepository customerRepository,
            InventoryRepository inventoryRepository,
            OrderRepository orderRepository,
            CheckoutPricingService pricingService,
            PaymentService paymentService,
            OrderNotificationService notificationService,
            AuditService auditService) {
        this.customerRepository =
                customerRepository;
        this.inventoryRepository =
                inventoryRepository;
        this.orderRepository =
                orderRepository;
        this.pricingService =
                pricingService;
        this.paymentService =
                paymentService;
        this.notificationService =
                notificationService;
        this.auditService =
                auditService;
    }

    public CheckoutResult checkout(
            CheckoutCommand command) {

        Customer customer =
                customerRepository
                        .findById(
                                command.customerId()
                        )
                        .orElseThrow();

        ProductInventory inventory =
                inventoryRepository
                        .findByProductId(
                                command.productId()
                        );

        validateInventory(
                inventory,
                command.quantity()
        );

        BigDecimal finalAmount =
                pricingService
                        .calculateFinalAmount(
                                inventory.getPrice(),
                                command.quantity(),
                                command.couponCode(),
                                customer.getState()
                        );

        boolean paymentSuccessful =
                paymentService.process(
                        command.paymentType(),
                        customer.getId(),
                        finalAmount
                );

        if (!paymentSuccessful) {
            return new CheckoutResult(
                    null,
                    "PAYMENT_FAILED"
            );
        }

        reduceInventory(
                inventory,
                command.quantity()
        );

        Order savedOrder =
                orderRepository.save(
                        new Order(
                                customer.getId(),
                                command.productId(),
                                command.quantity(),
                                finalAmount
                        )
                );

        notificationService
                .sendOrderConfirmation(
                        customer,
                        savedOrder.getId()
                );

        auditService.record(
                "ORDER_CREATED",
                savedOrder.getId()
        );

        return new CheckoutResult(
                savedOrder.getId(),
                "SUCCESS"
        );
    }

    private void validateInventory(
            ProductInventory inventory,
            int requestedQuantity) {

        if (inventory.getAvailableQuantity()
                < requestedQuantity) {
            throw new IllegalStateException(
                    "Insufficient inventory"
            );
        }
    }

    private void reduceInventory(
            ProductInventory inventory,
            int quantity) {

        inventory.setAvailableQuantity(
                inventory.getAvailableQuantity()
                        - quantity
        );

        inventoryRepository.save(inventory);
    }
}

11. Improved Code Explanation

Checkout Still Owns Workflow

The checkout service remains responsible for coordinating the checkout use case.

It has not been reduced to meaningless pass-through methods.

Pricing Has One Cohesive Responsibility

Discount and tax calculations are grouped because both contribute to checkout price calculation.

Payment Variants Are Isolated

Instead of:

JAVA
if CARD
else if UPI
else if ...

each processor handles one payment variant.

Adding another processor does not require modifying the checkout workflow.

Input Parameters Are Grouped

CheckoutCommand makes the use-case input explicit.

The method signature is easier to understand and evolve.

Payment Type Is Constrained

An enum prevents arbitrary strings such as:

JAVA
"Crd"
"credit"
"ABC"

from silently entering the application.

Notification Logic Has a Clear Owner

Email and SMS coordination no longer clutters the checkout workflow.

Refactoring Remains Practical

The solution does not introduce unnecessary repositories, factories, builders, events, and abstract base classes.

Only independently changing responsibilities were extracted.

12. Bad Code vs Improved Code

AreaBad CodeImproved Code
ResponsibilityOne service handles many concernsResponsibilities are cohesive
Payment variantsLarge conditionalProcessor abstraction
InputLong parameter listCheckoutCommand
Type safetyPayment type is StringPaymentType enum
TestingMany dependencies per testComponents can be tested independently
Change impactOne class changes frequentlyChanges are more localized
ReadabilityLarge workflow mixed with detailsWorkflow remains visible
ExtensibilityNew payment type edits checkoutNew processor can be added
MaintenanceHigh regression riskSmaller change surface

13. Real Project Scenario

Consider a banking application with a CustomerAccountService.

Initially it contains:

  • Account creation
  • Account closure
  • KYC validation
  • Address updates
  • Statement generation

Later developers add:

  • Fraud checks
  • Credit scoring
  • Email alerts
  • SMS alerts
  • PDF generation
  • Account limits
  • Audit history
  • Regulatory reporting

The service eventually has:

  • 20 dependencies
  • 70 methods
  • 4,000 lines
  • Multiple transaction boundaries
  • Multiple scheduled-job entry points

Every banking feature touches the same class.

Problems begin to appear:

  • Merge conflicts occur frequently
  • Developers cannot determine which methods belong together
  • Regression testing becomes expensive
  • One change requires unrelated mocks
  • No developer wants to refactor the class because it is too risky

The smell existed long before the class reached 4,000 lines.

Early warning signs were:

  • Growing constructor
  • Increasing unrelated method groups
  • Multiple reasons for change
  • Repeated integration logic
  • Multiple business concepts sharing the same service

Recognizing the smell earlier allows incremental extraction before the class becomes a critical maintenance bottleneck.

14. Production Impact

Design smells do not always cause immediate runtime failures.

Their biggest impact is often indirect.

Increased Regression Risk

Large highly coupled classes make unrelated features affect each other.

Inconsistent Business Rules

Duplicated decisions may evolve differently.

Slower Incident Resolution

Developers need more time to understand dependencies during production incidents.

Performance Problems Hidden Inside Large Methods

Long workflows may contain:

  • Repeated DB calls
  • Repeated API calls
  • Nested loops
  • Unnecessary loading

These issues become harder to identify when multiple responsibilities are mixed.

Deployment Risk

A small business change may touch many modules.

This increases testing and deployment scope.

Team Bottlenecks

Central classes become merge-conflict hotspots.

Harder Modernization

Replacing:

  • Database technology
  • Payment provider
  • Messaging platform
  • Notification provider

becomes difficult when integrations are spread throughout business code.

15. Common Developer Mistakes

Mistake 1: Treating Every Large Class as a God Class

Size alone does not determine design quality.

Check cohesion and reasons for change.

Mistake 2: Splitting One Class Into Many Meaningless Classes

Example:

JAVA
CheckoutValidatorHelper
CheckoutCalculationHelper
CheckoutProcessingHelper

without clear domain ownership.

This moves complexity rather than improving design.

Mistake 3: Using Utility Classes for Business Logic

Example:

JAVA
OrderUtils.calculateDiscount(...)
OrderUtils.validatePayment(...)
OrderUtils.sendNotification(...)

This often hides missing domain or service responsibilities.

Mistake 4: Creating Interfaces for Everything

An interface is not automatically better design.

Mistake 5: Ignoring Repeated Business Decisions

Developers often focus only on duplicated lines.

More dangerous duplication is duplicated decisions.

Mistake 6: Boolean Parameters Everywhere

Example:

JAVA
processOrder(
        true,
        false,
        true
);

The call gives little information about behavior.

Mistake 7: Strings Representing Business Types

Example:

JAVA
"ADMIN"
"CARD"
"PREMIUM"
"ACTIVE"

Enums or domain types may be safer.

Mistake 8: Huge Manager Classes

Names such as:

JAVA
OrderManager
UserManager
ApplicationManager
CommonService

often deserve additional review because their responsibility may be undefined.

Mistake 9: Extracting Too Early

A second implementation may never appear.

Avoid speculative abstractions.

Mistake 10: Ignoring Change History

A class changed in 40 unrelated PRs during three months is a stronger design signal than line count alone.

16. Edge Cases

Large but Cohesive Algorithm

A class implementing one complex calculation may be long but still cohesive.

Do not split algorithms arbitrarily.

Small Applications

A small CRUD application may reasonably use simple service classes without sophisticated domain separation.

Framework-Generated Patterns

Spring configuration or mapping code may naturally contain repetitive structures.

Not all repetition deserves abstraction.

One-Time Branching

A two-case switch does not automatically require Strategy Pattern.

Watch whether variants are expected to grow.

DTOs With Many Fields

A request object containing many fields is not automatically primitive obsession.

The smell matters when important concepts and validation are repeatedly scattered.

High-Level Orchestrators

An application service may legitimately have several dependencies if its responsibility is orchestrating a complex use case.

The key question is whether those dependencies contribute to one cohesive workflow.

Legacy Code

Large smell-heavy classes may be risky to rewrite completely.

Incremental refactoring is usually safer.

Performance-Critical Code

Introducing additional object layers purely for design aesthetics may add complexity with no benefit.

Measure performance where it actually matters.

17. Performance Considerations

Most design smells are primarily maintainability concerns.

However, some can contribute to performance problems.

N+1 Queries Hidden in Large Services

Example:

JAVA
for (Order order : orders) {
    customerRepository.findById(
            order.getCustomerId()
    );
}

Large service methods can hide repeated database access.

Duplicate External Calls

Duplicated integration logic may cause the same API to be called multiple times.

Feature Envy and Excessive Object Navigation

Code such as:

JAVA
order.getCustomer()
        .getAccount()
        .getPreferences()
        .getCurrency();

can result in unnecessary lazy loading in JPA-based applications.

Over-Abstraction

Excessive abstraction usually affects maintainability more than runtime performance, but unnecessary mapping or object creation can become relevant in high-volume processing.

Refactoring Does Not Automatically Improve Performance

Extracting a method or class usually has negligible impact on performance.

The primary benefit is code structure.

Do not claim performance improvements unless behavior actually changes.

18. Security Considerations

Design smells can indirectly create security issues.

Duplicated Authorization Rules

If authorization exists in several controllers and services, one path may be missed.

God Services With Sensitive Responsibilities

A service handling:

  • Authentication
  • Payment
  • Personal data
  • Logging

may accidentally expose sensitive information between concerns.

Primitive Role Values

Using unrestricted strings such as:

JAVA
role = "ADMIN"

throughout the application increases the risk of inconsistent authorization handling.

Utility Logging

Large utility methods may log entire objects containing:

  • Tokens
  • Payment details
  • Personal information

Business Validation Scattered Across Layers

Critical limits and permissions should not be duplicated in entry points that can be bypassed.

Security concerns should therefore be considered when a smell affects ownership of authorization, validation, or sensitive data.

19. Testing Considerations

Design smells often become obvious while writing tests.

Constructor Test Setup

If a simple unit test requires fifteen mocks, examine whether the class has too many responsibilities.

Test Each Cohesive Component

For the improved checkout design:

Pricing Tests

Test:

  • No coupon
  • Valid coupon
  • Invalid coupon
  • Tax calculation
  • Quantity boundaries

Payment Tests

Test:

  • Card processor selected
  • UPI processor selected
  • Unsupported payment type
  • Failed payment
  • Successful payment

Checkout Tests

Test:

  • Inventory unavailable
  • Payment failure
  • Successful order
  • Inventory reduction
  • Order persistence

Characterization Tests Before Refactoring

Legacy code should first receive tests that capture existing behavior.

These tests provide safety while responsibilities are extracted.

Integration Tests

Use integration tests for boundaries such as:

  • Database persistence
  • Payment client
  • Notification adapters

Do not use integration tests as a substitute for unit-testing core business rules.

Architecture Tests

Tools such as ArchUnit can help detect:

  • Controller-to-repository bypass
  • Circular package dependencies
  • Domain-to-web dependencies

20. Refactoring Guidelines

Refactor smells incrementally.

Step 1: Identify the Smell

Do not begin by extracting classes randomly.

State the problem clearly:

  • Too many responsibilities
  • Duplicated business rule
  • Growing type switch
  • Large parameter list
  • Cross-layer dependency

Step 2: Protect Existing Behavior

Add tests around critical scenarios.

Step 3: Find Cohesive Code

Group methods that use the same:

  • Data
  • Dependencies
  • Business purpose

Step 4: Extract One Responsibility

For example, move pricing calculation out of checkout.

Step 5: Keep Workflow Visible

Do not extract every line until the orchestration becomes impossible to understand.

Step 6: Replace Primitive Concepts Carefully

Change:

JAVA
String paymentType

to:

JAVA
PaymentType

only after checking API and persistence compatibility.

Step 7: Remove Duplicate Logic

Centralize the rule in the correct owner.

Step 8: Re-run Tests

Ensure business behavior has not changed.

Step 9: Evaluate the Result

Ask whether the new design is genuinely easier to understand.

Refactoring that adds more files but does not clarify responsibility may not be an improvement.

21. Best Practices

  • Name classes by clear responsibility.
  • Keep application services focused on cohesive use cases.
  • Keep domain decisions centralized.
  • Use value objects or enums for important business concepts when justified.
  • Use command objects for complex use-case inputs.
  • Extract integration-specific logic from business workflows.
  • Prefer small refactorings over large rewrites.
  • Use tests before structural changes.
  • Review constructor dependency count as a signal.
  • Review change history when identifying hotspots.
  • Keep orchestration readable.
  • Avoid hiding business behavior in generic helpers.
  • Prefer explicit domain terminology.
  • Measure abstractions by value, not by pattern count.

22. Practices to Avoid

God Classes

Avoid classes that become the default home for every feature in a domain.

Generic Manager Classes

Avoid unclear names such as:

JAVA
CommonManager
CoreManager
ApplicationService

when they provide no meaningful responsibility boundary.

Business Logic in Utility Classes

Utilities should not become a substitute for domain design.

Growing String-Based Switches

Repeated switches on business type may indicate a missing abstraction.

Boolean Flag APIs

Avoid methods whose behavior changes substantially based on several booleans.

Excessive Parameter Lists

They are difficult to read and evolve.

Circular Dependencies

They indicate unclear ownership.

Interface Explosion

Avoid interfaces with no architectural purpose.

Speculative Generality

Do not build extension mechanisms for requirements that do not exist.

Refactoring Only for Metrics

Reducing class lines from 500 to 250 does not matter if responsibilities remain unclear.

23. Code Review Checklist

  • Does this class have one clear responsibility?
  • Does the class change for several unrelated business reasons?
  • Is the constructor growing because the class owns too many concerns?
  • Are unrelated dependencies injected into the same service?
  • Is the same business rule implemented in multiple places?
  • Does adding one feature require changes across many unrelated classes?
  • Does this method manipulate another object's data more than its own?
  • Are important business concepts represented only as Strings or booleans?
  • Does this method have several boolean flag parameters?
  • Is there a long parameter list representing a missing request or domain concept?
  • Is a utility class accumulating unrelated business functions?
  • Does a switch statement grow every time a new business type is added?
  • Are controllers or repositories making business decisions?
  • Are classes tightly coupled to vendor-specific SDKs?
  • Does a simple unit test require too many mocks?
  • Are package dependencies circular?
  • Is the proposed abstraction solving a real change problem?
  • Are we extracting code only to reduce method length?
  • Would the refactoring make business behavior easier to locate?
  • Can this change be implemented incrementally with tests?

24. Common Pull Request Review Comments

  1. This service now handles pricing, persistence, notifications, and payment integration. Can we separate the independently changing responsibilities while keeping the use-case orchestration here?
  1. We're adding another branch for paymentType. Since this conditional grows for every new provider, could payment behavior move behind a processor abstraction?
  1. These six parameters appear to represent one request concept. A command object may make the method contract easier to understand and evolve.
  1. The two boolean parameters create four possible execution modes. Can we make the intended behavior explicit rather than controlling the workflow with flags?
  1. This business validation already exists in the batch flow. Let's keep the rule in one shared business component so the implementations cannot drift.
  1. The new helper class still depends on all the same collaborators as the original service. This may only move the code rather than separate responsibility.
  1. This String represents a fixed business type and is compared in several places. Would an enum or domain type reduce invalid values and repeated conditionals?
  1. The CommonUtils class is starting to contain order-specific business logic. This behavior would be easier to maintain in an order-focused component.
  1. This class now has fourteen constructor dependencies. Could we review whether some of these represent separate responsibilities rather than adding another dependency here?
  1. I would avoid introducing another interface at this point unless we have a meaningful boundary or alternative implementation to protect.

25. Code Review Exercise

Review the following Spring Boot service.

Identify:

  • Design smells
  • Responsibility problems
  • Coupling problems
  • Testability concerns
  • Change risks
  • Possible improvements

import java.math.BigDecimal; import org.springframework.stereotype.Service;

@Service public class EmployeeService { private final EmployeeRepository employeeRepository; private final PayrollClient payrollClient; private final EmailService emailService; private final AuditService auditService;

``` public EmployeeService( EmployeeRepository employeeRepository, PayrollClient payrollClient, EmailService emailService, AuditService auditService) { this.employeeRepository = employeeRepository; this.payrollClient = payrollClient; this.emailService = emailService; this.auditService = auditService; }

public void updateEmployee( Long employeeId, String name, String email, String department, BigDecimal salary, boolean notifyEmployee, boolean updatePayroll) {

Employee employee = employeeRepository .findById(employeeId) .orElseThrow();

employee.setName(name); employee.setEmail(email); employee.setDepartment(department); employee.setSalary(salary);

if (salary.compareTo( BigDecimal.valueOf(100000) ) > 0) { employee.setGrade("SENIOR"); } else { employee.setGrade("STANDARD"); }

employeeRepository.save(employee);

if (updatePayroll) { payrollClient.updateSalary( employeeId, salary ); }

if (notifyEmployee) { emailService.send( email, "Profile updated" ); }

auditService.record( "EMPLOYEE_UPDATED", employeeId ); }

public boolean canApproveExpense( Employee employee, BigDecimal amount) {

if ("SENIOR".equals( employee.getGrade() )) { return amount.compareTo( BigDecimal.valueOf(50000) ) <= 0; }

return amount.compareTo( BigDecimal.valueOf(10000) ) <= 0; } ```

}

Ask:

  • Is EmployeeService cohesive?
  • Does the method signature reveal any smells?
  • Are boolean flags controlling unrelated behavior?
  • Does salary-to-grade logic belong here?
  • Does expense approval belong in the same service?
  • What happens when grade rules change?
  • What happens when multiple notification channels are introduced?
  • What should be refactored first?

26. Exercise Solution

Several design smells are present.

Smell 1: Long Parameter List

updateEmployee() accepts seven parameters.

These values represent an employee update request and related processing options.

Smell 2: Boolean Flags

JAVA
notifyEmployee
updatePayroll

These flags make the method support different workflows.

Possible combinations include:

JAVA
false, false
true, false
false, true
true, true

Smell 3: Multiple Responsibilities

EmployeeService handles:

  • Employee persistence
  • Grade calculation
  • Payroll integration
  • Notification
  • Audit
  • Expense approval

Smell 4: Primitive Business Rules

Grades are represented as strings:

JAVA
"SENIOR"
"STANDARD"

Smell 5: Unrelated Business Rule

Expense approval does not directly belong to employee profile updating.

It may deserve an expense-policy component.

Improved Command

JAVA
import java.math.BigDecimal;

public record UpdateEmployeeCommand(
        Long employeeId,
        String name,
        String email,
        String department,
        BigDecimal salary) {
}

Grade

JAVA
public enum EmployeeGrade {
    STANDARD,
    SENIOR
}

Grade Policy

JAVA
import java.math.BigDecimal;
import org.springframework.stereotype.Component;

@Component
public class EmployeeGradePolicy {

    public EmployeeGrade determineGrade(
            BigDecimal salary) {

        if (salary.compareTo(
                BigDecimal.valueOf(100000)
        ) > 0) {
            return EmployeeGrade.SENIOR;
        }

        return EmployeeGrade.STANDARD;
    }
}

Expense Approval Policy

JAVA
import java.math.BigDecimal;
import org.springframework.stereotype.Component;

@Component
public class ExpenseApprovalPolicy {

    public boolean canApprove(
            EmployeeGrade grade,
            BigDecimal amount) {

        BigDecimal limit =
                grade == EmployeeGrade.SENIOR
                        ? BigDecimal.valueOf(50000)
                        : BigDecimal.valueOf(10000);

        return amount.compareTo(limit) <= 0;
    }
}

Employee Update Service

JAVA
import org.springframework.stereotype.Service;

@Service
public class EmployeeUpdateService {
    private final EmployeeRepository employeeRepository;
    private final EmployeeGradePolicy gradePolicy;
    private final PayrollClient payrollClient;
    private final EmailService emailService;
    private final AuditService auditService;

    public EmployeeUpdateService(
            EmployeeRepository employeeRepository,
            EmployeeGradePolicy gradePolicy,
            PayrollClient payrollClient,
            EmailService emailService,
            AuditService auditService) {
        this.employeeRepository =
                employeeRepository;
        this.gradePolicy =
                gradePolicy;
        this.payrollClient =
                payrollClient;
        this.emailService =
                emailService;
        this.auditService =
                auditService;
    }

    public void update(
            UpdateEmployeeCommand command) {

        Employee employee =
                employeeRepository
                        .findById(
                                command.employeeId()
                        )
                        .orElseThrow();

        employee.setName(
                command.name()
        );

        employee.setEmail(
                command.email()
        );

        employee.setDepartment(
                command.department()
        );

        employee.setSalary(
                command.salary()
        );

        employee.setGrade(
                gradePolicy.determineGrade(
                        command.salary()
                )
        );

        employeeRepository.save(employee);

        payrollClient.updateSalary(
                employee.getId(),
                employee.getSalary()
        );

        emailService.send(
                employee.getEmail(),
                "Profile updated"
        );

        auditService.record(
                "EMPLOYEE_UPDATED",
                employee.getId()
        );
    }
}

Why This Is Better

Command Object

The employee update input is represented explicitly.

Grade Policy

Grade calculation has one owner.

Future salary thresholds can be changed and tested independently.

Expense Policy

Expense approval no longer sits inside an unrelated employee-update service.

Enum

Employee grade becomes type-safe.

Workflow Is Clearer

The update service remains responsible for coordinating an employee-update use case.

A further refactoring to asynchronous notification or payroll events should only be introduced if actual project requirements justify it.

27. Interview Perspective

Design smells commonly appear in senior Java interviews and code-review rounds.

An interviewer may show a service containing:

  • 15 dependencies
  • 1,500 lines
  • Multiple external clients
  • Business rules
  • Utility methods

and ask:

"What would you change?"

A weak answer is:

"This violates SOLID. I would split it."

A stronger answer explains:

  • Which responsibilities are unrelated
  • Which changes occur independently
  • Which dependencies belong together
  • Which business rules should have one owner
  • Which extraction should happen first
  • What tests are required before refactoring
  • Which abstractions are unnecessary

Interviewers may also ask:

"What is a God class?"

A strong answer should go beyond size.

A God class centralizes too much knowledge or behavior and becomes the dependency or modification point for many unrelated concerns.

Another common scenario is:

"Would you create Strategy Pattern for this switch?"

The correct answer depends on context.

If the switch has two stable cases, a strategy may be unnecessary.

If every new payment type requires modifying multiple branches across the system, a strategy can localize change.

28. Interview Questions and Answers

Basic Question

Question: What is a design smell?

Answer:

A design smell is a structural warning sign suggesting that responsibilities, dependencies, or abstractions may be poorly organized.

It is not automatically a bug.

It indicates code that deserves closer review because future changes may become harder.

Intermediate Question

Question: How do you recognize a God class?

Answer:

Look for more than line count.

Typical indicators include:

  • Many unrelated responsibilities
  • Many dependencies
  • Many unrelated reasons for change
  • Large groups of unrelated methods
  • Many modules depending on the class
  • Difficult unit-test setup
  • Frequent merge conflicts

Advanced Question

Question: Why is shotgun surgery a design smell?

Answer:

Shotgun surgery occurs when one business change requires small modifications across many classes.

It suggests that related behavior or knowledge is scattered.

The risk is that developers may miss one location, causing inconsistent behavior.

Scenario-Based Question

Question: A service has twelve dependencies. Does that automatically mean it should be split?

Answer:

No.

Dependency count is a signal.

First determine whether all dependencies support one cohesive use case.

A complex orchestration service may legitimately have several collaborators.

If dependencies belong to unrelated responsibilities, extraction may be appropriate.

Code-Review Question

Question: What would you review in this method?

JAVA
process(
        Long customerId,
        String type,
        boolean notify,
        boolean audit,
        boolean retry
);

Answer:

Review:

  • Long parameter list
  • Primitive business type represented as String
  • Several boolean behavior flags
  • Whether the method performs multiple workflows
  • Whether a command object would improve clarity
  • Whether the flags indicate missing cohesive components

Real-Project Question

Question: How would you safely refactor a God service in production?

Answer:

Use incremental refactoring:

  1. Add characterization tests.
  2. Identify one cohesive responsibility.
  3. Extract that responsibility.
  4. Redirect existing calls.
  5. Verify tests and production behavior.
  6. Repeat gradually.

Avoid rewriting the entire service at once.

Design Question

Question: When should a switch statement be replaced with polymorphism or Strategy Pattern?

Answer:

Consider replacement when:

  • Variants continue growing
  • Each variant has substantial behavior
  • Similar switches appear in multiple places
  • New variants require modifying stable workflow code

Keep the switch when the cases are few, stable, and straightforward.

Refactoring Question

Question: Why can excessive class extraction make code worse?

Answer:

Too many tiny classes can:

  • Hide workflow
  • Increase navigation
  • Add indirection
  • Require unnecessary interfaces
  • Make ownership less obvious

The objective is higher cohesion and clearer responsibilities, not maximum class count.

29. Quick Rule to Remember

A design smell matters when code ownership, dependencies, or change behavior make the next safe modification harder than it should be.

30. Final Takeaway

Recognizing design smells is one of the most valuable skills during Java Pull Request reviews.

Developers should remember that smells are not mechanical rules.

A large class, switch statement, utility method, or multiple dependencies are not automatically bad.

The reviewer should understand:

  • Responsibility
  • Cohesion
  • Dependency structure
  • Business-rule ownership
  • Change frequency
  • Test complexity
  • Duplication
  • Expected future variation

Pay particular attention to:

  • God classes
  • Large services
  • Long parameter lists
  • Boolean flags
  • Primitive obsession
  • Duplicate business rules
  • Growing conditional logic
  • Circular dependencies
  • Utility dumping grounds
  • Excessive coupling
  • Shotgun surgery
  • Feature envy
  • Speculative abstractions

When a smell is found, do not immediately introduce a complex pattern.

Find the smallest refactoring that improves ownership.

Developers should avoid:

  • Moving code without clarifying responsibility
  • Creating interfaces mechanically
  • Splitting classes only by line count
  • Adding patterns before requirements justify them
  • Performing large rewrites without tests

During Pull Request review, the most useful question is often:

If this business requirement changes six months from now, how many places will a developer need to understand and modify?

Good design keeps that answer small, predictable, and aligned with the responsibility being changed.