1. Introduction
Single Responsibility at method level means that a method should perform one clearly defined job and should have one primary reason to change.
In a real Java backend, a method often starts small and gradually accumulates validation, database access, calculations, external API calls, logging, exception handling, and notification logic. Although such a method may still compile and pass basic tests, it becomes difficult to understand, test, debug, and safely modify.
During Pull Request review, the reviewer should ask whether the method represents one meaningful operation or hides several unrelated operations behind one name. A method named processOrder() may legitimately coordinate an order workflow, but it should not directly contain every validation rule, pricing formula, SQL-related decision, payment request, and email construction detail.
The goal is not to make every method extremely short. The goal is to keep each method focused, cohesive, and understandable at one level of abstraction.
2. What This Topic Means
At method level, the Single Responsibility Principle has three practical parts:
- The method has one clear purpose that can be described without using several unrelated "and" statements.
- Most statements inside the method operate at the same level of abstraction.
- A change in one business or technical concern does not force modifications to unrelated logic in the same method.
For example, calculateOrderTotal() should calculate a total. It should not also save the order, charge a card, update inventory, and send a confirmation email.
A coordinating method may call several focused methods or collaborators. Coordination itself can be its single responsibility. The important distinction is between orchestrating a workflow and implementing every detail of the workflow in one place.
Method-level responsibility is therefore about cohesion, not only line count. A 40-line method implementing one readable algorithm may be more focused than a 10-line method that validates data, changes persistent state, and calls an external service.
3. Why It Matters in Real Projects
Readability
A focused method communicates its intention through its name and structure. A reviewer can understand the workflow without mentally tracking unrelated variables, branches, and side effects.
Maintainability
Business rules change frequently. If discount calculation, payment processing, and notification formatting are separated, a change to one concern is less likely to damage another.
Debugging
Focused methods create natural diagnostic boundaries. When payment fails, developers can investigate payment behavior without stepping through validation, pricing, persistence, and email code in the same method.
Reliability
Large mixed-responsibility methods often have partial-failure problems. An order might be saved before payment fails, or payment might succeed before an unrelated email exception causes the entire request to report failure. Clear responsibilities make transaction and failure boundaries visible.
Team Development
Smaller cohesive changes produce easier reviews and fewer merge conflicts. Developers can change pricing, notifications, or validation independently when those responsibilities are properly separated.
Testability
Focused methods and collaborators allow each rule or side effect to be tested independently. Tests require less setup and failures identify the broken behavior more precisely.
Performance and scalability are not automatically improved by extracting methods. However, separation makes expensive database or external-service calls easier to identify, measure, and optimize.
4. Core Concept
The core concept is one responsibility and one abstraction level per method.
Consider these three levels:
- Workflow level: validate an order, reserve stock, collect payment, and confirm the order.
- Business-rule level: determine whether a customer qualifies for a discount.
- Technical-detail level: build an HTTP request or execute a repository query.
A workflow method may coordinate the steps, but it should delegate the detailed rules and integrations. Mixing all three levels forces the reader to repeatedly switch context.
A practical test is to complete this sentence:
This method is responsible for ________.
If the answer requires several independent activities, the method probably needs to be decomposed. Another signal is multiple reasons to change. For example, one method should not change because the discount policy changed, the payment provider changed, and the confirmation-email template changed.
Single responsibility does not mean one statement per method. Excessive extraction can scatter simple logic across many files and make navigation harder. Extract behavior when it has a distinct purpose, rule, side effect, failure mode, or testing need.
5. Important Rules
- Give every method a name that describes one observable purpose.
- Keep business rules separate from database, HTTP, messaging, and formatting details.
- Keep statements inside a method at a consistent abstraction level.
- Treat workflow orchestration as a valid responsibility, but delegate detailed steps.
- Separate queries from commands where practical; avoid methods that both fetch information and unexpectedly modify state.
- Make side effects visible through method names and collaborator boundaries.
- Extract code when it has an independent reason to change or requires independent tests.
- Do not extract trivial one-line methods only to reduce line count.
- Pass only the data a focused method needs instead of a large mutable context object.
- Define transaction boundaries intentionally after responsibilities are separated.
- Preserve meaningful exception types rather than catching every exception in a large method.
- Avoid boolean parameters that make one method perform different jobs.
- Prefer return values that express the result of the method's single purpose.
6. Bad Code Example
The following Spring service mixes request validation, customer lookup, pricing, persistence, payment integration, status updates, logging, and email creation in one method.
@Service
public class OrderService {
private static final Logger log = LoggerFactory.getLogger(OrderService.class);
private final OrderRepository orderRepository;
private final CustomerRepository customerRepository;
private final PaymentGateway paymentGateway;
private final JavaMailSender mailSender;
public OrderService(OrderRepository orderRepository, CustomerRepository customerRepository, PaymentGateway paymentGateway, JavaMailSender mailSender) {
this.orderRepository = orderRepository;
this.customerRepository = customerRepository;
this.paymentGateway = paymentGateway;
this.mailSender = mailSender;
}
@Transactional
public OrderResponse processOrder(OrderRequest request) {
if (request == null || request.customerId() == null || request.items() == null || request.items().isEmpty()) {
throw new IllegalArgumentException("Invalid order request");
}
Customer customer = customerRepository.findById(request.customerId())
.orElseThrow(() -> new IllegalArgumentException("Customer not found"));
BigDecimal total = BigDecimal.ZERO;
for (OrderItemRequest item : request.items()) {
if (item.quantity() <= 0 || item.unitPrice() == null || item.unitPrice().signum() < 0) {
throw new IllegalArgumentException("Invalid order item");
}
total = total.add(item.unitPrice().multiply(BigDecimal.valueOf(item.quantity())));
}
if (customer.isPremium()) {
total = total.multiply(new BigDecimal("0.90"));
}
Order order = new Order(customer.getId(), total, OrderStatus.CREATED);
orderRepository.save(order);
try {
PaymentResult payment = paymentGateway.charge(request.paymentToken(), total);
if (!payment.successful()) {
order.setStatus(OrderStatus.PAYMENT_FAILED);
orderRepository.save(order);
throw new PaymentException("Payment was declined");
}
order.setPaymentReference(payment.reference());
order.setStatus(OrderStatus.CONFIRMED);
orderRepository.save(order);
SimpleMailMessage message = new SimpleMailMessage();
message.setTo(customer.getEmail());
message.setSubject("Order confirmed");
message.setText("Your order " + order.getId() + " total is " + total);
mailSender.send(message);
log.info("Order {} processed for customer {} with payment token {}", order.getId(), customer.getId(), request.paymentToken());
return new OrderResponse(order.getId(), order.getStatus(), total);
} catch (Exception exception) {
log.error("Order processing failed", exception);
throw new RuntimeException("Unable to process order");
}
}
}7. Problems in the Bad Code
Multiple Responsibilities
processOrder() performs at least eight jobs:
- Request validation
- Item validation
- Customer retrieval
- Price calculation
- Premium-discount calculation
- Order persistence
- Payment processing
- Email construction and delivery
- Operational logging
- Exception translation
The method can change for unrelated reasons, including a new discount policy, payment provider, email format, or validation rule.
Mixed Abstraction Levels
High-level workflow statements appear beside low-level details such as BigDecimal multiplication, mutable entity updates, and SimpleMailMessage construction. The reader cannot see the business flow without reading every implementation detail.
Unclear Transaction Boundary
The database transaction remains open while a remote payment call and email call are executed. This can hold a database connection and locks longer than necessary. A remote timeout may keep the transaction active for seconds.
Incorrect Failure Semantics
The broad catch (Exception) catches the intentionally thrown PaymentException and converts it to a generic RuntimeException. Callers lose the distinction between a declined payment, a mail failure, a repository failure, and an unexpected programming error.
If email delivery fails after the payment succeeds, the method reports the entire order as failed. Depending on transaction behavior, database changes might roll back while the external payment cannot be rolled back automatically.
Sensitive Data Exposure
The log statement includes the payment token. Tokens or payment credentials must not be written to application logs.
Weak Domain Validation
All invalid cases use generic IllegalArgumentException messages. The API cannot reliably map them to useful client responses, and support teams receive little diagnostic information.
Difficult Unit Testing
Testing a simple pricing rule requires mocking repositories, a payment gateway, and mail infrastructure. Tests become large and fragile because every responsibility is coupled to the same method.
Duplicate Persistence Calls
The method explicitly saves the same mutable entity several times. JPA dirty checking may make some calls unnecessary. More importantly, persistence behavior is mixed with state-transition decisions, making the intended lifecycle unclear.
8. Code Review Findings
A senior reviewer should notice the following:
- The method name suggests one operation, but its body contains several independently changing concerns.
- Validation, calculation, persistence, integration, and presentation-related email formatting are mixed.
- The method holds a transaction across remote network operations.
- Payment success and email failure are treated as one atomic failure even though the external charge cannot participate in the database transaction.
catch (Exception)destroys meaningful failure information.- A payment token is logged, creating a sensitive-data exposure risk.
- The method is difficult to test because business rules depend on unrelated infrastructure mocks.
- The order state transitions are implicit mutable updates instead of a clearly expressed workflow.
- A notification concern is coupled directly to the core order-processing path.
- The response uses the local
totalvariable rather than a completed domain result, increasing the chance of inconsistent values as the workflow evolves.
9. Reviewer Comment Example
processOrder()currently handles validation, pricing, persistence, payment, and notification. Could we keep this method as the workflow coordinator and move these details into focused collaborators? This would let us test pricing and failure handling independently.
The transaction includes the payment and email network calls. Please narrow the database transaction boundary so a slow external service does not keep the transaction open.
A successful payment followed by an email failure is currently returned as a complete order failure. Please separate notification failure from the order/payment result and define the required retry behavior.
Please remove the payment token from this log statement. It is sensitive data and should not appear in application logs.
10. Improved Code
The improved design keeps placeOrder() responsible for workflow orchestration. Focused collaborators own validation, pricing, payment, persistence, and notification. The code below shows the important boundaries without adding an unnecessary framework or design-pattern hierarchy.
@Service
public class OrderApplicationService {
private final OrderValidator orderValidator;
private final CustomerService customerService;
private final OrderPricingService pricingService;
private final OrderPersistenceService persistenceService;
private final PaymentService paymentService;
private final OrderNotificationService notificationService;
public OrderApplicationService(OrderValidator orderValidator, CustomerService customerService, OrderPricingService pricingService, OrderPersistenceService persistenceService, PaymentService paymentService, OrderNotificationService notificationService) {
this.orderValidator = orderValidator;
this.customerService = customerService;
this.pricingService = pricingService;
this.persistenceService = persistenceService;
this.paymentService = paymentService;
this.notificationService = notificationService;
}
public OrderResponse placeOrder(OrderRequest request) {
orderValidator.validate(request);
Customer customer = customerService.getRequiredCustomer(request.customerId());
Money total = pricingService.calculateTotal(request.items(), customer.membershipLevel());
Order order = persistenceService.createPendingOrder(customer.getId(), request.items(), total);
PaymentReceipt receipt = paymentService.collectPayment(order.getId(), request.paymentToken(), total);
Order confirmedOrder = persistenceService.confirmOrder(order.getId(), receipt.reference());
notificationService.sendConfirmationSafely(confirmedOrder, customer.getEmail());
return OrderResponse.from(confirmedOrder);
}
}
@Component
public class OrderValidator {
public void validate(OrderRequest request) {
if (request == null) {
throw new OrderValidationException("Order request is required");
}
if (request.customerId() == null) {
throw new OrderValidationException("Customer ID is required");
}
if (request.paymentToken() == null || request.paymentToken().isBlank()) {
throw new OrderValidationException("Payment token is required");
}
if (request.items() == null || request.items().isEmpty()) {
throw new OrderValidationException("At least one order item is required");
}
request.items().forEach(this::validateItem);
}
private void validateItem(OrderItemRequest item) {
if (item == null || item.productId() == null) {
throw new OrderValidationException("Each item must contain a product ID");
}
if (item.quantity() <= 0) {
throw new OrderValidationException("Item quantity must be greater than zero");
}
if (item.unitPrice() == null || item.unitPrice().signum() < 0) {
throw new OrderValidationException("Item price must not be negative");
}
}
}
@Service
public class OrderPricingService {
private static final BigDecimal PREMIUM_DISCOUNT = new BigDecimal("0.10");
public Money calculateTotal(List<OrderItemRequest> items, MembershipLevel membershipLevel) {
BigDecimal subtotal = items.stream()
.map(this::lineTotal)
.reduce(BigDecimal.ZERO, BigDecimal::add);
BigDecimal discount = membershipLevel == MembershipLevel.PREMIUM
? subtotal.multiply(PREMIUM_DISCOUNT)
: BigDecimal.ZERO;
return Money.inr(subtotal.subtract(discount));
}
private BigDecimal lineTotal(OrderItemRequest item) {
return item.unitPrice().multiply(BigDecimal.valueOf(item.quantity()));
}
}
@Service
public class PaymentService {
private final PaymentGateway paymentGateway;
public PaymentService(PaymentGateway paymentGateway) {
this.paymentGateway = paymentGateway;
}
public PaymentReceipt collectPayment(UUID orderId, String paymentToken, Money total) {
PaymentResult result = paymentGateway.charge(orderId.toString(), paymentToken, total.amount());
if (!result.successful()) {
throw new PaymentDeclinedException(orderId, result.declineReason());
}
return new PaymentReceipt(result.reference());
}
}
@Service
public class OrderPersistenceService {
private final OrderRepository orderRepository;
public OrderPersistenceService(OrderRepository orderRepository) {
this.orderRepository = orderRepository;
}
@Transactional
public Order createPendingOrder(UUID customerId, List<OrderItemRequest> items, Money total) {
Order order = Order.pending(customerId, items, total);
return orderRepository.save(order);
}
@Transactional
public Order confirmOrder(UUID orderId, String paymentReference) {
Order order = orderRepository.findById(orderId)
.orElseThrow(() -> new OrderNotFoundException(orderId));
order.confirm(paymentReference);
return order;
}
}
@Service
public class OrderNotificationService {
private static final Logger log = LoggerFactory.getLogger(OrderNotificationService.class);
private final ApplicationEventPublisher eventPublisher;
public OrderNotificationService(ApplicationEventPublisher eventPublisher) {
this.eventPublisher = eventPublisher;
}
public void sendConfirmationSafely(Order order, String email) {
try {
eventPublisher.publishEvent(new OrderConfirmedEvent(order.getId(), email));
} catch (RuntimeException exception) {
log.error("Unable to publish confirmation event for order {}", order.getId(), exception);
}
}
}In a payment-sensitive production system, use an idempotency key and an outbox, saga, or equivalent recovery mechanism. The example focuses on responsibility boundaries; it does not claim that multiple database and payment operations form one distributed atomic transaction.
11. Improved Code Explanation
Clear Workflow Method
placeOrder() now reads as a sequence of business steps. It coordinates the use case without implementing every rule and technical detail. Workflow orchestration is its single responsibility.
Dedicated Validation
OrderValidator owns request invariants and produces domain-specific validation failures. Adding a quantity rule no longer requires editing payment or notification code.
Isolated Pricing Rules
OrderPricingService performs deterministic calculation. It has no database, payment, or email dependency, so it can be tested with plain unit tests.
Explicit Payment Boundary
PaymentService translates the gateway response into a domain result or a meaningful PaymentDeclinedException. It does not expose payment tokens through logs.
Narrow Transactions
OrderPersistenceService owns database state changes and applies transactions only to database operations. The payment call is not executed while one of these transactions is open.
Separated Notification Failure
Notification publishing is a distinct responsibility. A confirmation-notification problem does not incorrectly convert an already paid and confirmed order into a client-visible payment failure. A production event consumer can retry delivery.
Better Failure Information
Specific exceptions identify validation, missing data, payment decline, and infrastructure failure separately. A global exception handler can map these outcomes to correct HTTP responses and operational alerts.
Visible Side Effects
Repository changes, payment collection, and event publishing are expressed through named collaborators. Reviewers can identify side effects directly from the workflow.
12. Bad Code vs Improved Code
| Area | Bad Code | Improved Code |
|---|---|---|
| Readability | One method mixes workflow and implementation details | Workflow reads as named business steps |
| Maintainability | Unrelated changes modify the same method | Each concern has a focused change location |
| Testability | Pricing tests require infrastructure mocks | Pure pricing logic can be unit tested directly |
| Failure handling | All exceptions become a generic failure | Failures retain domain meaning |
| Transactions | Remote calls run inside a database transaction | Transactions are limited to persistence operations |
| Reliability | Email failure can mask successful payment | Notification failure is isolated and can be retried |
| Security | Payment token is logged | Sensitive payment data is not logged |
| Performance visibility | Network and database work is hidden in a long body | Expensive boundaries are explicit and measurable |
Method extraction itself does not reduce algorithmic complexity. The main gains are cohesion, testability, safe change, and visibility of failure and performance boundaries.
13. Real Project Scenario
Consider a healthcare microservice that schedules a patient procedure. One large method initially performs eligibility validation, insurance authorization, slot reservation, patient-record updates, audit logging, and SMS notification.
Several teams later need to change it:
- The compliance team changes audit requirements.
- The insurance team introduces a second authorization provider.
- Operations adds retry handling for SMS delivery.
- The scheduling team changes how provisional slots expire.
If all behavior is contained in one method, each change risks affecting clinical scheduling and patient data. Reviewers also struggle to determine which failure should roll back a reservation and which failure should be retried independently.
A focused workflow method can coordinate validateEligibility(), requestAuthorization(), reserveSlot(), recordProcedure(), and publishPatientNotification(). Each collaborator then owns one policy or integration. This separation makes regulatory review, testing, and incident diagnosis more reliable.
14. Production Impact
When a mixed-responsibility method reaches production, realistic consequences include:
- A secondary notification failure causes the API to report that the primary transaction failed, leading clients to retry an already completed payment.
- Long-running external calls hold database connections and reduce request throughput.
- A small pricing change accidentally alters order-state or exception-handling behavior.
- Generic exceptions hide whether failures come from validation, the database, or a third-party service.
- Sensitive data is logged because logging details are embedded in unrelated business code.
- Unit tests cover only happy paths because full method setup is too expensive or complicated.
- Partial state is left behind when several side effects fail at different points.
- Incident resolution takes longer because logs and stack traces point to one large method instead of a focused component.
- Multiple developers frequently modify the same method, increasing merge conflicts and review effort.
15. Common Developer Mistakes
- Treating a method as focused only because it has one public name.
- Measuring responsibility only by number of lines.
- Extracting private methods but leaving all infrastructure dependencies and failure behavior coupled in one class.
- Creating vague helper names such as
handleData(),doProcess(), orexecuteStep(). - Mixing validation with state mutation so invalid input can partially modify an entity.
- Catching
Exceptionaround an entire workflow instead of handling expected failures at the correct boundary. - Keeping a database transaction open across HTTP, payment, email, or messaging calls.
- Using a boolean flag such as
process(order, true, false)to switch between different responsibilities. - Combining calculation with persistence because the calculated value is immediately saved.
- Extracting every statement into a method, creating excessive navigation without meaningful cohesion.
- Moving code to a utility class even though it belongs to a business capability.
- Assuming that separate methods automatically provide separate transactions in Spring. Self-invocation and proxy behavior must be considered.
- Returning
nullfor multiple failure types instead of expressing an expected result or throwing a meaningful exception. - Ignoring compensation, retry, and idempotency after separating external side effects from database work.
16. Edge Cases
Reviewers should consider edge cases at the boundary owned by each responsibility:
nullrequest, customer ID, item, price, or payment token- Empty item list
- Zero or negative quantity
- Negative price or an unexpectedly large monetary amount
- Duplicate product entries that may need consolidation or rejection
- Customer deleted or deactivated between validation and persistence
- Concurrent inventory or order-state modification
- Payment timeout where the final provider outcome is unknown
- Duplicate client retry after payment succeeds
- Repository failure after payment succeeds
- Event publication or notification failure after confirmation
- Repeated delivery of the same confirmation event
- Currency scale and rounding behavior
- Maximum request size and unusually large item collections
Separation helps assign these cases correctly. Pricing handles rounding; payment handles timeouts and idempotency; persistence handles optimistic locking; notification handles repeated event delivery.
17. Performance Considerations
Single responsibility is primarily a design and maintainability concern, not an automatic performance optimization. Extracting a private Java method usually has negligible cost because the JVM can inline small methods.
Important real-project considerations include:
- A focused persistence component makes database call count easier to review. Avoid introducing one query per item when extracting logic.
- A workflow should not keep database transactions open during slow external API calls.
- Pricing over
nitems remainsO(n)whether implemented in one method or a focused service. - Avoid copying a large item collection between every layer without need. Prefer immutable views or appropriate domain objects.
- Do not create remote microservice calls merely to enforce source-code separation. A separate method or in-process component is sufficient unless deployment independence is required.
- Measure payment, repository, and notification latency separately. Focused boundaries make metrics meaningful.
- For large orders, validate and calculate in a single traversal when doing so remains clear and does not merge unrelated responsibilities.
The improved design may add object and method-call boundaries, but these costs are normally insignificant compared with database and network I/O. Do not sacrifice clarity based on unmeasured micro-optimization concerns.
18. Security Considerations
Method-level responsibility affects security when security-sensitive logic is buried among unrelated behavior.
- Validate input before performing state changes or external calls.
- Keep authorization checks at a clearly visible application boundary. Extraction must not accidentally bypass them.
- Never pass or log payment tokens, passwords, secrets, access tokens, or unnecessary personal data.
- Use separate audit behavior where required, but ensure the business operation cannot silently skip mandatory audit records.
- Avoid generic exception responses that expose stack traces or internal provider messages.
- Keep output encoding and message construction appropriate to the destination.
- Ensure callers cannot invoke extracted internal operations without the authorization enforced by the original entry point.
- Treat personally identifiable information according to retention and masking policies.
There is no special vulnerability caused solely by a long method. The risk is that mixed concerns make authorization, validation, logging, and data exposure harder to see and review.
19. Testing Considerations
Unit Tests
Test each focused responsibility without unrelated infrastructure:
OrderValidatoraccepts a valid request.OrderValidatorrejects a null request, empty item list, invalid quantity, missing product, and negative price.OrderPricingServicecalculates a standard total.OrderPricingServiceapplies the premium discount exactly once.- Pricing uses the required scale and rounding mode.
PaymentServicereturns a receipt for a successful gateway result.PaymentServicethrowsPaymentDeclinedExceptionfor a decline.PaymentServicepreserves the order ID in the failure for traceability.
Example focused pricing test:
class OrderPricingServiceTest {
private final OrderPricingService pricingService = new OrderPricingService();
@Test
void shouldApplyPremiumDiscountToSubtotal() {
List<OrderItemRequest> items = List.of(
new OrderItemRequest(UUID.randomUUID(), 2, new BigDecimal("500.00")),
new OrderItemRequest(UUID.randomUUID(), 1, new BigDecimal("200.00"))
);
Money total = pricingService.calculateTotal(items, MembershipLevel.PREMIUM);
assertThat(total.amount()).isEqualByComparingTo("1080.00");
}
}Workflow Test
Test that the application service calls the steps in the required business order and returns the confirmed result. Avoid asserting private implementation details. Verify important effects such as "payment is not requested when validation fails."
Integration Tests
- Persist a pending order and confirm it using the real JPA mapping and test database.
- Verify optimistic-lock behavior for concurrent order updates.
- Use a payment stub or contract test to verify request and response mapping.
- Verify the event or outbox record is created after confirmation.
- Verify API exception mapping for validation errors, declines, and unexpected infrastructure failures.
Exception and Recovery Tests
- Simulate a payment timeout and verify the order remains in a recoverable state.
- Simulate event publication failure and verify a confirmed order is not reported as unpaid.
- Retry the same payment request and verify idempotent behavior.
- Verify sensitive payment data does not appear in captured logs.
The most important testing benefit is that each test has a small reason to fail. A discount test should not fail because an email mock was not configured.
20. Refactoring Guidelines
Use the following sequence to refactor safely without changing business behavior:
- Add characterization tests around the current public method, including current edge cases and side effects.
- Identify the high-level workflow and write down its ordered steps.
- Mark independent responsibilities such as validation, calculation, persistence, integration, and notification.
- Extract deterministic logic first because it is easiest to verify.
- Give every extracted method or component a business-specific name.
- Keep existing input, output, exceptions, and side-effect order unchanged during the first extraction.
- Run tests after each small extraction.
- Move infrastructure calls behind focused collaborators.
- Re-evaluate transaction boundaries before changing annotations.
- Add specific failure handling only after the structural refactor is stable.
- Compare logs, database changes, external requests, and API responses before and after the refactor.
- Introduce asynchronous events, outbox processing, or retry behavior as a separate change when possible.
Do not combine a large responsibility refactor with new pricing rules or API behavior unless the change is unavoidable. Structural and behavioral changes in one Pull Request are harder to verify.
21. Best Practices
- Let public application methods describe complete use cases at a high level.
- Keep deterministic business calculations free from I/O.
- Use domain-specific names such as
collectPayment()andconfirmOrder()rather than vague verbs. - Put transaction annotations on methods that own a clear persistence unit of work.
- Model expected failures with meaningful exception or result types.
- Make external side effects explicit and idempotent where retries are possible.
- Prefer constructor injection so dependencies reveal what a component needs.
- Keep validation close to the boundary or domain invariant it protects.
- Extract a collaborator when a responsibility needs separate configuration, tests, metrics, retries, or ownership.
- Use private methods for small internal decomposition when a separate class would add no value.
- Keep the coordinating method readable from top to bottom.
- Review responsibility together with failure and transaction boundaries, not only method length.
22. Practices to Avoid
- God methods: They hide several rules and side effects behind one entry point.
- Arbitrary line limits: A method is not automatically wrong because it exceeds a fixed number of lines.
- One-line wrapper chains: Excessive delegation creates navigation cost without isolating a real responsibility.
- Boolean mode switches: They make one method implement multiple workflows.
- Generic helper classes:
CommonUtilsoften becomes a dumping ground for unrelated behavior. - Broad exception catches: They erase failure meaning and may hide programming defects.
- Remote calls inside long database transactions: They increase connection usage, lock time, and rollback confusion.
- Hidden side effects: A method named
calculateTotal()must not save records or send messages. - Logging sensitive inputs: Responsibility extraction does not justify exposing tokens or personal data.
- Premature microservices: Source-code responsibility boundaries do not require independent deployment.
- Refactoring and behavior changes together: Reviewers cannot easily distinguish moved code from new logic.
- Calling extracted transactional methods through
this: In common Spring proxy configuration, self-invocation does not activate the expected transaction interceptor.
23. Code Review Checklist
- Can the method's responsibility be described in one clear sentence?
- Does the method name accurately describe all of its observable behavior?
- Does the method contain unrelated validation, calculation, persistence, integration, or formatting logic?
- Are statements written at a consistent level of abstraction?
- Is the method coordinating steps or implementing every detail itself?
- Does the method have several unrelated reasons to change?
- Are side effects visible through clear method and collaborator names?
- Does a calculation method unexpectedly access a database or external service?
- Does a query method unexpectedly change state?
- Is a database transaction held open during a remote call?
- Can expected failures be distinguished by the caller?
- Could a secondary failure incorrectly report that the primary operation failed?
- Can each business rule be tested without mocking unrelated infrastructure?
- Are extracted methods named by purpose rather than implementation?
- Has extraction introduced duplicate database or external API calls?
- Are authorization, validation, and audit requirements still enforced after delegation?
- Is sensitive information excluded from logs and exception messages?
- Are idempotency and partial failure handled for external side effects?
- Is a new class justified, or would a focused private method be sufficient?
- Has the refactor preserved business behavior and operation order?
24. Common Pull Request Review Comments
placeOrder()currently validates input, calculates pricing, persists state, charges payment, and sends email. Could we keep it as an orchestrator and extract these independently changing concerns?- This method name suggests a calculation, but it also saves the entity. Please make the side effect explicit or separate the command from the calculation.
- The payment call is made inside the database transaction. Can we narrow the transaction boundary to avoid holding a connection during network I/O?
- Please extract the discount rule into a focused pricing method so it can be tested without repository and gateway mocks.
- The broad catch converts validation, payment, and mail failures into the same response. Please preserve meaningful failure types at their respective boundaries.
- Email delivery should not change the result of an already successful payment. Can we publish a retryable notification event after confirmation?
handleOrderData()does not explain the extracted method's responsibility. Could we rename it to the business operation it performs?- This boolean parameter selects two different workflows. Separate methods would make each responsibility and call site clearer.
- Please confirm that moving this method did not change Spring transaction behavior; an internal call through
thiswill not use the proxy in the usual configuration. - The extracted validator still performs a repository update. Validation should not partially mutate persistent state when it fails.
25. Code Review Exercise
Review the following employee-onboarding method. Identify the responsibilities, code smells, bug risks, security concerns, transaction issues, and possible improvements. Do not focus only on method length.
@Service
public class EmployeeOnboardingService {
private final EmployeeRepository employeeRepository;
private final PayrollClient payrollClient;
private final PasswordEncoder passwordEncoder;
private final JavaMailSender mailSender;
@Transactional
public Employee onboard(EmployeeRequest request) {
if (request.email() == null || !request.email().contains("@")) {
throw new RuntimeException("Bad input");
}
if (employeeRepository.existsByEmail(request.email())) {
throw new RuntimeException("Already exists");
}
Employee employee = new Employee();
employee.setName(request.name().trim());
employee.setEmail(request.email().toLowerCase());
employee.setPassword(passwordEncoder.encode(request.temporaryPassword()));
employee.setStatus("ACTIVE");
employeeRepository.save(employee);
try {
payrollClient.createEmployee(employee.getId(), request.bankAccountNumber(), request.salary());
SimpleMailMessage message = new SimpleMailMessage();
message.setTo(employee.getEmail());
message.setSubject("Welcome");
message.setText("Your temporary password is " + request.temporaryPassword());
mailSender.send(message);
System.out.println("Created employee " + employee.getEmail() + " bank account " + request.bankAccountNumber());
return employee;
} catch (Exception exception) {
employeeRepository.delete(employee);
throw new RuntimeException("Onboarding failed");
}
}
}Identify:
- Every distinct responsibility inside
onboard() - Problems in validation and normalization
- Transaction and external-call risks
- Sensitive-data exposure
- Incorrect compensation assumptions
- Testing difficulties
- A production-quality responsibility split
26. Exercise Solution
Senior Review Findings
The method contains request validation, uniqueness checking, data normalization, password hashing, entity construction, persistence, payroll integration, email construction, logging, compensation, and exception translation.
Important issues include:
contains("@")is not sufficient email validation.name().trim()throws if the name is null.- Email normalization should happen before the uniqueness query; otherwise case variants may bypass an application-level check.
- An application-level
existsByEmail()check is still vulnerable to a race. The database needs a unique constraint. - Salary and bank-account input are not validated.
- A temporary password is included in email text and may be exposed in mail systems and logs. A time-limited activation link is safer.
- The bank-account number is printed to standard output.
- The database transaction stays active across payroll and email calls.
- Deleting the employee does not undo a successful remote payroll creation.
- Catching every exception hides whether payroll, mail, persistence, or code failed.
- Email failure deletes the local employee even after payroll creation succeeds.
- Returning a JPA entity exposes persistence details and possibly sensitive fields.
- The method is difficult to unit test because all responsibilities require simultaneous setup.
Improved Java Code
@Service
public class EmployeeOnboardingApplicationService {
private final EmployeeRequestValidator validator;
private final EmployeeRegistrationService registrationService;
private final PayrollProvisioningService payrollProvisioningService;
private final OnboardingEventPublisher eventPublisher;
public EmployeeOnboardingApplicationService(EmployeeRequestValidator validator, EmployeeRegistrationService registrationService, PayrollProvisioningService payrollProvisioningService, OnboardingEventPublisher eventPublisher) {
this.validator = validator;
this.registrationService = registrationService;
this.payrollProvisioningService = payrollProvisioningService;
this.eventPublisher = eventPublisher;
}
public EmployeeResponse onboard(EmployeeRequest request) {
ValidatedEmployeeRequest validatedRequest = validator.validateAndNormalize(request);
Employee employee = registrationService.registerPendingEmployee(validatedRequest);
PayrollProfile payrollProfile = payrollProvisioningService.provision(employee.getId(), validatedRequest);
Employee activeEmployee = registrationService.activateEmployee(employee.getId(), payrollProfile.reference());
eventPublisher.publishEmployeeActivated(activeEmployee.getId(), activeEmployee.getEmail());
return EmployeeResponse.from(activeEmployee);
}
}
@Component
public class EmployeeRequestValidator {
private final EmailValidator emailValidator;
public EmployeeRequestValidator(EmailValidator emailValidator) {
this.emailValidator = emailValidator;
}
public ValidatedEmployeeRequest validateAndNormalize(EmployeeRequest request) {
if (request == null) {
throw new EmployeeValidationException("Employee request is required");
}
String name = requireText(request.name(), "Employee name is required");
String email = requireText(request.email(), "Email is required").toLowerCase(Locale.ROOT);
if (!emailValidator.isValid(email)) {
throw new EmployeeValidationException("Email format is invalid");
}
if (request.salary() == null || request.salary().signum() <= 0) {
throw new EmployeeValidationException("Salary must be greater than zero");
}
if (request.bankAccountToken() == null || request.bankAccountToken().isBlank()) {
throw new EmployeeValidationException("Tokenized bank account is required");
}
return new ValidatedEmployeeRequest(name, email, request.salary(), request.bankAccountToken());
}
private String requireText(String value, String message) {
if (value == null || value.isBlank()) {
throw new EmployeeValidationException(message);
}
return value.trim();
}
}
@Service
public class EmployeeRegistrationService {
private final EmployeeRepository employeeRepository;
public EmployeeRegistrationService(EmployeeRepository employeeRepository) {
this.employeeRepository = employeeRepository;
}
@Transactional
public Employee registerPendingEmployee(ValidatedEmployeeRequest request) {
Employee employee = Employee.pending(request.name(), request.email());
try {
return employeeRepository.saveAndFlush(employee);
} catch (DataIntegrityViolationException exception) {
throw new EmployeeAlreadyExistsException(request.email(), exception);
}
}
@Transactional
public Employee activateEmployee(UUID employeeId, String payrollReference) {
Employee employee = employeeRepository.findById(employeeId)
.orElseThrow(() -> new EmployeeNotFoundException(employeeId));
employee.activate(payrollReference);
return employee;
}
}
@Service
public class PayrollProvisioningService {
private final PayrollClient payrollClient;
public PayrollProvisioningService(PayrollClient payrollClient) {
this.payrollClient = payrollClient;
}
public PayrollProfile provision(UUID employeeId, ValidatedEmployeeRequest request) {
return payrollClient.createEmployee(employeeId.toString(), request.bankAccountToken(), request.salary());
}
}
@Component
public class OnboardingEventPublisher {
private final ApplicationEventPublisher eventPublisher;
public OnboardingEventPublisher(ApplicationEventPublisher eventPublisher) {
this.eventPublisher = eventPublisher;
}
public void publishEmployeeActivated(UUID employeeId, String email) {
eventPublisher.publishEvent(new EmployeeActivatedEvent(employeeId, email));
}
}Why the Changes Are Useful
- The application method owns orchestration and shows the business sequence clearly.
- Validation and normalization happen before state changes.
- The database unique constraint remains the final defense against concurrent duplicate registration.
- Only a tokenized bank-account reference crosses the application boundary.
- Password distribution is removed; the event consumer can send a time-limited account-activation link.
- Persistence transactions do not include remote payroll or email latency.
- Payroll provisioning is independently testable and can use an idempotency key based on employee ID.
- The employee remains
PENDINGif payroll fails, allowing a retry or operational recovery instead of unsafe local deletion. - Activation notification is a separate outcome and can be delivered through an outbox for reliable processing.
- The API returns a response DTO rather than the persistence entity.
For strict delivery guarantees, persist the employee state change and an outbox event in the same database transaction. The payroll workflow may require a saga or reconciliation job because a remote payroll system cannot join the local database transaction.
27. Interview Perspective
Interviewers rarely want only the definition "a method should do one thing." They usually test whether the candidate can apply the idea without over-engineering.
Typical interview discussion areas include:
- Identifying multiple responsibilities in a service method
- Explaining why method length alone is an incomplete signal
- Distinguishing orchestration from mixed implementation detail
- Refactoring a legacy method without changing behavior
- Designing test boundaries for business rules and integrations
- Handling transactions across database and external API operations
- Explaining Spring proxy limitations after extracting transactional methods
- Deciding between a private method and a separate service
- Preserving authorization, audit, and exception behavior during extraction
- Handling partial failures and idempotent retries
A strong answer connects design quality to change risk, testing, failure boundaries, and production behavior. It also acknowledges that too many tiny methods or services can reduce readability.
28. Interview Questions and Answers
Basic Question
Question: What does Single Responsibility mean at method level?
Answer: A method should have one clear purpose and one primary reason to change. Its statements should be cohesive and generally operate at the same abstraction level. The rule is about responsibility, not an arbitrary number of lines.
Intermediate Question
Question: Is a method that validates, saves, and sends an email violating method-level Single Responsibility?
Answer: Usually yes, because validation rules, persistence behavior, and notification behavior change for different reasons and have different failure modes. A coordinating method may call three focused operations, but it should not implement all their details in one body.
Advanced Question
Question: When should behavior be extracted to a private method versus a separate class?
Answer: Use a private method when the behavior is an internal detail with no independent dependency, policy, ownership, reuse, or testing requirement. Use a separate collaborator when it has its own business rules, infrastructure dependency, configuration, retry policy, metrics, transaction boundary, or reason to change. Do not create a class for every few lines.
Scenario-Based Question
Question: An order method saves an order, charges payment, and sends email inside one transaction. How would you review it?
Answer: I would separate database state changes, payment integration, and notification. A local transaction should not stay open during network calls. I would define pending and confirmed states, make payment requests idempotent, and publish notification through a retryable event or outbox. I would also define recovery for payment success followed by persistence failure because the external payment cannot roll back with the database.
Code-Review Question
Question: What signals reveal that a method has too many responsibilities?
Answer: Signals include a name containing vague verbs, several unrelated dependency calls, variables belonging to different domains, mixed validation and mutation, broad exception handling, multiple comments separating phases, inconsistent abstraction levels, boolean mode flags, and tests that require many unrelated mocks.
Real-Project Question
Question: How would you refactor a 300-line production method safely?
Answer: First capture current behavior with characterization tests and record side effects and exception behavior. Identify workflow steps, then extract deterministic calculations and validations in small commits. Next isolate persistence and external integrations while preserving execution order. Reassess transaction boundaries separately, run tests after every extraction, and compare API results, database changes, messages, and external calls. I would avoid adding new business behavior during the structural refactor.
Spring Boot Question
Question: What transaction problem can appear after extracting methods inside the same Spring service?
Answer: In the usual proxy-based transaction model, calling an @Transactional method through this does not pass through the Spring proxy, so the annotation may not take effect as expected. Put the transactional operation on an injected collaborator, call it through the proxy, or deliberately use another supported transaction mechanism.
Design Trade-Off Question
Question: Can applying Single Responsibility make code worse?
Answer: Yes. Extracting every statement into a separate method or class can create indirection, weak names, and navigation overhead. Separation is valuable when it isolates a meaningful rule, side effect, failure boundary, or reason to change. Cohesion and clarity are the objective, not the maximum number of methods.
29. Quick Rule to Remember
If a method changes for unrelated reasons or mixes workflow with implementation details, keep the workflow and separate the details.
30. Final Takeaway
The developer should remember that a focused method expresses one purpose at a consistent abstraction level. A workflow method may coordinate several steps, but detailed validation, calculation, persistence, external integration, and notification behavior should not be buried together.
The reviewer should check more than line count. Look for unrelated reasons to change, hidden side effects, broad exception handling, long transaction boundaries, mixed failure semantics, and tests that require unrelated mocks.
Production code should avoid god methods, vague helpers, boolean-driven multi-purpose behavior, remote calls inside long database transactions, and refactors that accidentally change authorization or business behavior. Separate responsibilities where the separation creates clearer change, testing, failure, and operational boundaries, while avoiding unnecessary classes and indirection.