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:
@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:
controller
service
batch processorA 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:
amount > 10000in one class and:
amount >= 10000in 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:
300-line classis not automatically bad.
A cohesive parser implementing one complex algorithm may reasonably contain substantial code.
Meanwhile:
80-line classmay 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.
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:
customerId
productId
quantity
couponCode
paymentType
sendEmail
sendSmsThe caller must understand parameter ordering and multiple behaviors.
Primitive Obsession
Important concepts are represented as primitive or generic values.
Examples:
String paymentType
String couponCode
boolean sendEmail
boolean sendSmsThe payment method is a business concept but is represented as unrestricted text.
Boolean Flag Smell
boolean sendEmail
boolean sendSmsThese flags introduce conditional behavior inside the method.
Additional notification channels could make the method increasingly complex.
Growing Conditional Logic
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:
customerId
productId
quantity
couponCode
paymentTyperepresents 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
public record CheckoutCommand(
Long customerId,
Long productId,
int quantity,
String couponCode,
PaymentType paymentType) {
}Payment Type
public enum PaymentType {
CARD,
UPI
}Pricing Service
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
import java.math.BigDecimal;
public interface PaymentProcessor {
boolean supports(PaymentType paymentType);
boolean process(
Long customerId,
BigDecimal amount);
}Card Processor
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
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
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
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
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:
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:
"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
| Area | Bad Code | Improved Code |
|---|---|---|
| Responsibility | One service handles many concerns | Responsibilities are cohesive |
| Payment variants | Large conditional | Processor abstraction |
| Input | Long parameter list | CheckoutCommand |
| Type safety | Payment type is String | PaymentType enum |
| Testing | Many dependencies per test | Components can be tested independently |
| Change impact | One class changes frequently | Changes are more localized |
| Readability | Large workflow mixed with details | Workflow remains visible |
| Extensibility | New payment type edits checkout | New processor can be added |
| Maintenance | High regression risk | Smaller 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:
CheckoutValidatorHelper
CheckoutCalculationHelper
CheckoutProcessingHelperwithout clear domain ownership.
This moves complexity rather than improving design.
Mistake 3: Using Utility Classes for Business Logic
Example:
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:
processOrder(
true,
false,
true
);The call gives little information about behavior.
Mistake 7: Strings Representing Business Types
Example:
"ADMIN"
"CARD"
"PREMIUM"
"ACTIVE"Enums or domain types may be safer.
Mistake 8: Huge Manager Classes
Names such as:
OrderManager
UserManager
ApplicationManager
CommonServiceoften 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:
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:
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:
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:
String paymentTypeto:
PaymentTypeonly 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:
CommonManager
CoreManager
ApplicationServicewhen 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
This service now handles pricing, persistence, notifications, and payment integration. Can we separate the independently changing responsibilities while keeping the use-case orchestration here?
We're adding another branch for paymentType. Since this conditional grows for every new provider, could payment behavior move behind a processor abstraction?
These six parameters appear to represent one request concept. A command object may make the method contract easier to understand and evolve.
The two boolean parameters create four possible execution modes. Can we make the intended behavior explicit rather than controlling the workflow with flags?
This business validation already exists in the batch flow. Let's keep the rule in one shared business component so the implementations cannot drift.
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.
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?
The CommonUtils class is starting to contain order-specific business logic. This behavior would be easier to maintain in an order-focused component.
This class now has fourteen constructor dependencies. Could we review whether some of these represent separate responsibilities rather than adding another dependency here?
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
EmployeeServicecohesive? - 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
notifyEmployee
updatePayrollThese flags make the method support different workflows.
Possible combinations include:
false, false
true, false
false, true
true, trueSmell 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:
"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
import java.math.BigDecimal;
public record UpdateEmployeeCommand(
Long employeeId,
String name,
String email,
String department,
BigDecimal salary) {
}Grade
public enum EmployeeGrade {
STANDARD,
SENIOR
}Grade Policy
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
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
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?
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:
- Add characterization tests.
- Identify one cohesive responsibility.
- Extract that responsibility.
- Redirect existing calls.
- Verify tests and production behavior.
- 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.