Small and Focused Methods

1. Introduction

A small and focused method performs one clear unit of work at one level of abstraction. Its name tells the reader why the work exists, its inputs and output are understandable, and its body does not mix unrelated responsibilities.

In a Java code review, method size is not judged only by counting lines. A 35-line method that performs one cohesive mapping operation may be easier to maintain than a 12-line method that validates input, queries a database, calls a payment gateway, sends an email, and converts exceptions. The review question is therefore not simply, “Is this method short?” It is, “Does this method have one reason to change?”

2. What This Topic Means

Small and focused methods separate business operations into meaningful steps. For example, an order-placement use case may need to:

• Validate the request.
• Load the customer and product data.
• Calculate the payable amount.
• Reserve stock.
• Collect payment.
• Save the order.
• Publish an event.

The public service method can coordinate these steps, while focused private methods or dedicated collaborators implement them. This gives the code a readable flow without hiding important behavior.

A focused method usually has:

• One responsibility.
• One consistent abstraction level.
• A meaningful name.
• A small, explicit set of inputs.
• A predictable result or side effect.
• Few branches and limited nesting.

“Small” is a design outcome, not a fixed rule such as five, ten, or twenty lines. Extracting every two lines into a separate method can make code harder to follow. The extraction must reveal intent or isolate a responsibility.

3. Why It Matters in Real Projects

Readability

Reviewers can understand an orchestration method as a sequence of business actions. Meaningful method names reduce the need to mentally interpret low-level conditions and implementation details.

Maintainability

When validation, pricing, persistence, and integration logic are separated, a change in one area is less likely to affect another. The code also produces smaller merge conflicts because developers can modify different units of behavior.

Debugging

Focused methods create clear boundaries for logging, breakpoints, metrics, and stack traces. A failure in reserveInventory() is easier to investigate than a failure somewhere in a 200-line process() method.

Reliability

Smaller units make error paths, state changes, and transaction boundaries easier to inspect. Reviewers are more likely to notice missing validation, partial updates, and incorrect exception handling.

Team development

Developers can review and test a focused change without repeatedly understanding an entire large method. Clear boundaries also make ownership and future refactoring easier.

Performance

Method extraction itself normally has no meaningful performance cost in business applications because the JVM can inline frequently executed small methods. The important performance benefit is visibility: focused code makes database calls inside loops, repeated external calls, and unnecessary collection passes easier to detect.

4. Core Concept

The core principle is the Single Responsibility Principle applied at method level: a method should do one coherent job and should have one primary reason to change.

One level of abstraction

An orchestration method should read at a high level:

public OrderResponse placeOrder(CreateOrderRequest request) {
    validateRequest(request);
    Customer customer = loadActiveCustomer(request.customerId());
    Order order = createOrder(request, customer);
    collectPayment(order);
    return toResponse(saveOrder(order));
}

It should not suddenly contain low-level operations such as building SQL strings, parsing raw JSON, calculating individual tax rates, or formatting an email body. Those details belong in focused methods or collaborators.

Cohesion

Every statement in a method should contribute directly to the method’s purpose. If a statement belongs to a different business responsibility, the method is losing cohesion.

Command-query separation

Where practical, a method should either return information or change state. A method named findCustomer() should not also update the customer’s last-login time and send a notification. Mixed commands and queries create surprising behavior and make tests complicated.

Side effects

A focused method may have a side effect, but the side effect should be obvious from its name and contract. Names such as saveOrder, reserveInventory, and publishOrderCreatedEvent communicate state changes better than handleOrderData.

5. Important Rules

• Give each method one clearly expressible purpose.
• Keep statements at a consistent abstraction level.
• Use a verb-based name that describes intent rather than implementation.
• Avoid boolean parameters that switch between unrelated behaviors.
• Prefer a small parameter object when many values always travel together.
• Keep validation separate from persistence and external integrations.
• Make side effects explicit through method names and boundaries.
• Use guard clauses to reduce deeply nested branches.
• Extract duplicated business rules to one authoritative place.
• Do not extract code merely to satisfy a line-count target.
• Move behavior to a dedicated class when private-method extraction leaves the original class with several responsibilities.
• Keep transaction boundaries aligned with the business operation, not with arbitrary method size.

6. Bad Code Example

The following service method validates an order, queries repositories, calculates totals, reserves inventory, charges a payment, persists data, and sends a notification.

@Service
public class OrderService {
    private final CustomerRepository customerRepository;
    private final ProductRepository productRepository;
    private final OrderRepository orderRepository;
    private final InventoryClient inventoryClient;
    private final PaymentClient paymentClient;
    private final NotificationClient notificationClient;

    public OrderService(CustomerRepository customerRepository,
                        ProductRepository productRepository,
                        OrderRepository orderRepository,
                        InventoryClient inventoryClient,
                        PaymentClient paymentClient,
                        NotificationClient notificationClient) {
        this.customerRepository = customerRepository;
        this.productRepository = productRepository;
        this.orderRepository = orderRepository;
        this.inventoryClient = inventoryClient;
        this.paymentClient = paymentClient;
        this.notificationClient = notificationClient;
    }

    @Transactional
    public OrderResponse placeOrder(CreateOrderRequest 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"));
        if (!customer.isActive()) {
            throw new IllegalStateException("Customer is inactive");
        }
        BigDecimal total = BigDecimal.ZERO;
        List<OrderItem> orderItems = new ArrayList<>();
        for (CreateOrderItemRequest itemRequest : request.items()) {
            Product product = productRepository.findById(itemRequest.productId())
                    .orElseThrow(() -> new IllegalArgumentException("Product not found"));
            if (itemRequest.quantity() <= 0) {
                throw new IllegalArgumentException("Quantity must be positive");
            }
            BigDecimal lineTotal = product.getPrice().multiply(BigDecimal.valueOf(itemRequest.quantity()));
            total = total.add(lineTotal);
            orderItems.add(new OrderItem(product.getId(), product.getPrice(), itemRequest.quantity(), lineTotal));
        }
        InventoryReservation reservation = inventoryClient.reserve(request.items());
        if (!reservation.successful()) {
            throw new IllegalStateException("Inventory reservation failed");
        }
        PaymentResult payment = paymentClient.charge(request.paymentToken(), total);
        if (!payment.successful()) {
            inventoryClient.release(reservation.reservationId());
            throw new IllegalStateException("Payment failed: " + payment.message());
        }
        Order order = new Order(customer.getId(), orderItems, total, payment.transactionId());
        Order savedOrder = orderRepository.save(order);
        notificationClient.sendOrderConfirmation(customer.getEmail(), savedOrder.getId(), total);
        return new OrderResponse(savedOrder.getId(), savedOrder.getStatus(), savedOrder.getTotal());
    }
}

7. Problems in the Bad Code

Too many responsibilities

placeOrder() contains request validation, customer eligibility, product lookup, pricing, mapping, inventory integration, payment handling, compensation, persistence, notification, and response mapping. Changes to any of those responsibilities require editing the same method.

Mixed abstraction levels

High-level workflow steps are mixed with low-level details such as BigDecimal multiplication, entity construction, error-message composition, and inventory compensation.

Database calls inside a loop

productRepository.findById() is called once per item. A large order creates an N+1-style access pattern and increases latency and database load.

Hidden business policy

Rules such as positive quantity, active customer, and release-on-payment-failure are buried inside procedural code. They are difficult to locate, reuse, and test independently.

Broad exception types

IllegalArgumentException and IllegalStateException do not provide a useful domain contract. An API exception handler may not be able to return the correct status and error code.

Fragile compensation

Inventory is released only when the gateway returns an unsuccessful result. If paymentClient.charge() throws a timeout exception, the reservation may remain allocated.

Long database transaction

The method is transactional while making inventory, payment, and notification network calls. This can keep a database transaction open during slow remote operations, consume connections, and create lock contention.

Notification coupled to success path

If saving succeeds but notification fails, the transaction and API behavior are unclear. Retrying the full request could charge the customer twice unless idempotency is implemented.

Difficult unit tests

Testing a pricing boundary requires configuring customer, repository, inventory, payment, persistence, and notification dependencies. The test setup becomes larger than the behavior under test.

8. Code Review Findings

A senior reviewer should notice that:

• The method cannot be described with one precise action below “do everything required to place an order.”
• Validation rules and orchestration are mixed.
• Per-item repository access may cause excessive queries.
• Remote calls occur within a database transaction.
• Payment exceptions do not trigger inventory compensation.
• Notification delivery is part of the synchronous critical path.
• Generic exceptions make API error mapping unclear.
• The method is difficult to test because unrelated behaviors cannot be isolated.
• A retry and idempotency strategy is missing for a payment operation.
• Extraction into private methods alone may not be enough; pricing, inventory, payment, and notification are separate collaborators.

9. Reviewer Comment Example

placeOrder() currently mixes validation, pricing, persistence, and three integrations. Could we keep this method as the workflow coordinator and move each business responsibility behind a focused method or collaborator?

We call productRepository.findById() for every item. Please load all required products in one query and verify that every requested ID was found.

The payment and notification network calls run while the database transaction is open. Can we narrow the transaction boundary and publish the confirmation after commit?

If paymentClient.charge() throws, the inventory reservation is not released. Please centralize compensation and cover both unsuccessful responses and exceptions.

Could we replace these generic exceptions with domain exceptions so the API layer can return stable error codes?

10. Improved Code

The improved design keeps placeOrder() as a readable workflow coordinator. Focused collaborators own validation, order creation, inventory, payment, and persistence. A post-commit event separates notification from the main transaction.

@Service
public class OrderApplicationService {
    private final OrderRequestValidator requestValidator;
    private final CustomerService customerService;
    private final OrderFactory orderFactory;
    private final InventoryService inventoryService;
    private final PaymentService paymentService;
    private final OrderPersistenceService persistenceService;

    public OrderApplicationService(OrderRequestValidator requestValidator,
                                   CustomerService customerService,
                                   OrderFactory orderFactory,
                                   InventoryService inventoryService,
                                   PaymentService paymentService,
                                   OrderPersistenceService persistenceService) {
        this.requestValidator = requestValidator;
        this.customerService = customerService;
        this.orderFactory = orderFactory;
        this.inventoryService = inventoryService;
        this.paymentService = paymentService;
        this.persistenceService = persistenceService;
    }

    public OrderResponse placeOrder(CreateOrderRequest request) {
        requestValidator.validate(request);
        Customer customer = customerService.getActiveCustomer(request.customerId());
        Order order = orderFactory.create(customer, request.items());
        InventoryReservation reservation = inventoryService.reserve(order.getItems());
        PaymentReceipt receipt = collectPaymentOrReleaseInventory(order, request.paymentToken(), reservation);
        Order savedOrder = persistenceService.savePaidOrder(order, receipt, customer.getEmail());
        return OrderResponse.from(savedOrder);
    }

    private PaymentReceipt collectPaymentOrReleaseInventory(Order order,
                                                            String paymentToken,
                                                            InventoryReservation reservation) {
        try {
            return paymentService.collect(paymentToken, order.getTotal(), order.getRequestId());
        } catch (RuntimeException exception) {
            inventoryService.release(reservation.id());
            throw exception;
        }
    }
}

@Component
public class OrderFactory {
    private final ProductRepository productRepository;
    private final OrderPricingService pricingService;

    public OrderFactory(ProductRepository productRepository, OrderPricingService pricingService) {
        this.productRepository = productRepository;
        this.pricingService = pricingService;
    }

    public Order create(Customer customer, List<CreateOrderItemRequest> itemRequests) {
        Map<Long, Product> productsById = loadProducts(itemRequests);
        List<OrderItem> orderItems = createOrderItems(itemRequests, productsById);
        Money total = pricingService.calculateTotal(orderItems);
        return Order.pending(customer.getId(), orderItems, total);
    }

    private Map<Long, Product> loadProducts(List<CreateOrderItemRequest> itemRequests) {
        Set<Long> productIds = itemRequests.stream()
                .map(CreateOrderItemRequest::productId)
                .collect(Collectors.toSet());
        Map<Long, Product> productsById = productRepository.findAllById(productIds).stream()
                .collect(Collectors.toMap(Product::getId, Function.identity()));
        if (productsById.size() != productIds.size()) {
            throw new ProductNotFoundException(findMissingProductIds(productIds, productsById.keySet()));
        }
        return productsById;
    }

    private List<Long> findMissingProductIds(Set<Long> requestedIds, Set<Long> foundIds) {
        return requestedIds.stream()
                .filter(id -> !foundIds.contains(id))
                .toList();
    }

    private List<OrderItem> createOrderItems(List<CreateOrderItemRequest> requests,
                                             Map<Long, Product> productsById) {
        return requests.stream()
                .map(request -> OrderItem.from(productsById.get(request.productId()), request.quantity()))
                .toList();
    }
}

@Service
public class OrderPersistenceService {
    private final OrderRepository orderRepository;
    private final ApplicationEventPublisher eventPublisher;

    public OrderPersistenceService(OrderRepository orderRepository,
                                   ApplicationEventPublisher eventPublisher) {
        this.orderRepository = orderRepository;
        this.eventPublisher = eventPublisher;
    }

    @Transactional
    public Order savePaidOrder(Order order, PaymentReceipt receipt, String customerEmail) {
        order.markPaid(receipt.transactionId());
        Order savedOrder = orderRepository.save(order);
        eventPublisher.publishEvent(OrderPlacedEvent.from(savedOrder, customerEmail));
        return savedOrder;
    }
}

@Component
public class OrderNotificationListener {
    private final NotificationService notificationService;

    public OrderNotificationListener(NotificationService notificationService) {
        this.notificationService = notificationService;
    }

    @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
    public void sendConfirmation(OrderPlacedEvent event) {
        notificationService.sendOrderConfirmation(event.email(), event.orderId(), event.total());
    }
}

11. Improved Code Explanation

1. placeOrder() now exposes the business workflow through meaningful method calls.
2. OrderRequestValidator owns structural request rules, so invalid input is rejected before integrations begin.
3. CustomerService owns customer lookup and eligibility rules.
4. OrderFactory batches product lookup instead of executing one query per line item.
5. OrderPricingService owns monetary calculations and can be tested separately.
6. collectPaymentOrReleaseInventory() gives the compensation rule a clear name and handles thrown runtime failures as well as domain failures converted by PaymentService.
7. OrderPersistenceService creates a narrow database transaction around the state change and publishes the event while that transaction is active.
8. OrderPlacedEvent separates notification delivery from the order transaction; the listener runs only after the transaction commits successfully.
9. Domain-specific exceptions provide stable behavior for the REST exception handler.
10. Each collaborator can change and be tested without modifying the workflow coordinator.

This design still requires production decisions. For example, a reliable outbox may be better than an in-process event if notification delivery must survive an application crash. Payment and inventory operations also need idempotency and a documented recovery strategy. Small methods make those policies visible; they do not automatically solve distributed consistency.

12. Bad Code vs Improved Code

13. Real Project Scenario

Consider a healthcare claims microservice. An original processClaim() method validates member eligibility, loads provider contracts, calculates the covered amount, detects duplicate claims, saves adjudication results, creates an audit record, and sends a message to a downstream payment system.

A regulatory change modifies only the provider-contract calculation. In the monolithic method, the developer must edit code surrounded by eligibility, persistence, and messaging logic. Reviewers cannot easily confirm that unrelated behavior remains unchanged.

With focused methods and collaborators, ClaimPricingService.calculateCoveredAmount() owns the changed rule. The Pull Request contains a small implementation change and targeted parameterized tests. The coordinator remains unchanged, and reviewers can verify the regulatory rule without mentally simulating the complete claim workflow.

14. Production Impact

If a large, multi-responsibility method reaches production, realistic consequences include:

• A small business-rule change accidentally affecting unrelated persistence or integration behavior.
• Long debugging sessions because logs and stack traces identify only a generic processing method.
• Slow requests caused by hidden database calls inside loops.
• Connection-pool pressure from remote calls performed inside transactions.
• Partial updates when failures occur between side effects.
• Duplicate payment or notification actions during retries.
• Low test coverage because the method requires excessive setup.
• Frequent merge conflicts when multiple developers edit the same method.
• Delayed Pull Request reviews because the change is difficult to understand confidently.

15. Common Developer Mistakes

• Treating a fixed line limit as the definition of a focused method.
• Extracting methods named processData(), handle(), or doWork() that hide rather than reveal intent.
• Creating many one-line forwarding methods with no business meaning.
• Keeping all extracted methods in one oversized service instead of introducing cohesive collaborators.
• Passing ten unrelated parameters after extraction.
• Using boolean flags such as process(order, true, false) to select different workflows.
• Mixing validation with data mutation, making failed validation leave partial state.
• Hiding network or database calls inside innocent-looking methods such as getTotal().
• Catching exceptions in every small method and losing the original context.
• Splitting a transaction without understanding atomicity requirements.
• Making focused methods public only so they can be unit tested.
• Extracting code before protecting existing behavior with tests.
• Confusing method size with class design; a class can contain small methods and still have too many responsibilities.

16. Edge Cases

Reviewers should check the following applicable cases:

• null request, customer ID, item list, product ID, or payment token.
• Empty item collections.
• Zero or negative quantities.
• Duplicate product IDs and whether they should be merged or rejected.
• Products deleted or deactivated between validation and reservation.
• Monetary rounding and currency consistency.
• Very large item lists that may exceed query parameter limits.
• Payment timeout where the gateway processed the charge but the application did not receive a response.
• Inventory release failure after payment failure.
• Concurrent orders competing for the same stock.
• Duplicate client retries and idempotency-key reuse.
• Database failure after successful payment.
• Notification failure after the order commits.

Focused methods help assign each edge case to the correct owner. They do not remove the need for an end-to-end recovery policy.

17. Performance Considerations

Method-call overhead

In typical Spring Boot applications, the overhead introduced by a few focused Java method calls is negligible compared with database and network latency. The JIT compiler can inline frequently called small methods. Avoid retaining a monolithic method based on speculative call-overhead concerns.

Database access

Extraction should make query behavior explicit. Load multiple entities in batches, avoid repository calls inside loops, and verify generated SQL. A focused method named loadProducts() is a useful review boundary, but its implementation must still be efficient.

Collection processing

Do not create multiple streams and intermediate collections merely to make each step a separate method. Measure large-data paths and combine passes when it materially reduces CPU or memory without harming clarity.

External calls

Focused integration methods make timeout, retry, circuit-breaker, and idempotency policies easier to apply. Never assume a smaller method makes a remote call inexpensive.

Object creation

Parameter objects and domain values improve contracts, but excessive temporary objects can matter in a proven high-throughput hot path. Use profiling data before sacrificing readability.

18. Security Considerations

Small methods are not a security control by themselves, but they make security responsibilities reviewable.

• Keep authorization explicit and early in the workflow. Do not hide it in a generic helper.
• Validate untrusted input before using it in queries, logs, or integrations.
• Avoid passing raw payment tokens or secrets through many methods.
• Do not include sensitive values in exception messages or PR examples.
• Keep audit logging separate from general diagnostic logging and ensure required audit events cannot be silently skipped.
• Preserve authenticated user context when work moves to asynchronous handlers.
• Use repository parameter binding; method extraction does not make string-built SQL safe.
• Verify that splitting a method does not accidentally bypass an authorization or validation step on another entry path.

19. Testing Considerations

Unit tests for focused behavior

Each business rule should have direct tests. For example, an order-pricing service should test ordinary totals, multiple items, rounding, discounts, and invalid quantities without mocking payment or notification clients.

class OrderPricingServiceTest {
    private final OrderPricingService pricingService = new OrderPricingService();

    @Test
    void calculatesTotalForMultipleItems() {
        List<OrderItem> items = List.of(
                new OrderItem(1L, new BigDecimal("100.00"), 2),
                new OrderItem(2L, new BigDecimal("50.00"), 1));
        Money total = pricingService.calculateTotal(items);
        assertThat(total.amount()).isEqualByComparingTo("250.00");
    }

    @Test
    void rejectsNonPositiveQuantity() {
        assertThatThrownBy(() -> new OrderItem(1L, new BigDecimal("100.00"), 0))
                .isInstanceOf(InvalidQuantityException.class);
    }
}

Orchestration tests

Test that the coordinator calls responsibilities in the required order and stops when a critical step fails. Avoid asserting private-method calls; assert observable behavior and collaborator interactions.

Negative and exception tests

• Inactive customer rejects the order before stock or payment calls.
• Inventory failure prevents payment.
• Payment rejection releases inventory.
• Payment timeout triggers the defined reconciliation or compensation behavior.
• Persistence failure after payment follows the recovery policy.
• Notification failure does not roll back a committed order.

Integration tests

Use repository integration tests to verify the batch product query and transaction behavior. Use contract or stub-server tests for payment and inventory response mapping. Verify after-commit event handling separately.

Regression tests before refactoring

Before breaking apart legacy code, capture its current observable behavior with characterization tests. These tests should cover successful processing, validation failures, integration failures, and side-effect ordering.

20. Refactoring Guidelines

1. Identify the method’s observable inputs, outputs, exceptions, database changes, and external side effects.
2. Add characterization tests for the current behavior before structural changes.
3. Mark distinct responsibilities inside the method: validation, calculation, mapping, persistence, and integrations.
4. Extract one cohesive block at a time and give it an intent-revealing name.
5. Keep the first refactor behavior-preserving; do not combine structural changes with new business rules.
6. Run unit and integration tests after every extraction.
7. Replace long parameter lists with an existing domain object or a purposeful parameter object.
8. Move extracted methods to collaborators when they represent separate responsibilities or need independent dependencies.
9. Revisit transaction boundaries only after documenting atomicity and failure behavior.
10. Confirm logging, metrics, exception mapping, authorization, and audit behavior remain intact.
11. Compare query counts and external-call counts before and after the refactor.
12. Make small commits so reviewers can distinguish movement from behavior changes.

21. Best Practices

• Make the public application method describe the use-case flow.
• Name private methods after business intent, such as verifyCustomerEligibility().
• Put calculations in pure methods where practical.
• Keep I/O and side effects at visible boundaries.
• Use domain exceptions with stable error codes.
• Keep validation close to the domain that owns the rule.
• Prefer constructor injection for required collaborators.
• Use immutable request, response, and value objects where suitable.
• Document non-obvious transaction and compensation decisions.
• Use batch repository operations for collections.
• Keep methods cohesive even when some are longer than an arbitrary limit.
• Use static analysis metrics such as cognitive complexity as review signals, not automatic design decisions.

22. Practices to Avoid

• God methods: They concentrate unrelated behavior and create many reasons to change.
• Meaningless extraction: Methods such as doStep1() move lines without revealing intent.
• Boolean-driven behavior: Flags often indicate two methods or strategies are hiding in one method.
• Hidden side effects: A query-like name should not write to a database or invoke an external service.
• Long parameter lists: They make method contracts difficult to understand and easy to call incorrectly.
• Excessive private-method chains: Readers should not need to jump through ten files or methods to understand a simple calculation.
• Premature helper classes: A generic CommonUtils class usually becomes a collection of unrelated behavior.
• Swallowed exceptions: Small boundaries should preserve causes and translate exceptions only at an appropriate layer.
• Transaction annotations added blindly: Spring proxy behavior and business atomicity must be understood.
• Mixing refactoring with feature work: It obscures behavioral changes and makes regression review harder.

23. Code Review Checklist

• Can the method’s responsibility be described in one clear sentence?
• Does every statement contribute directly to that responsibility?
• Does the method stay at one level of abstraction?
• Does the name communicate business intent and side effects?
• Are validation, calculation, persistence, and integrations separated appropriately?
• Are database or external-service calls hidden inside loops?
• Does the method have excessive parameters or boolean control flags?
• Are branches and nesting making the method hard to reason about?
• Are exception and compensation paths as clear as the success path?
• Is a database transaction kept open during remote calls?
• Can important business rules be tested without unrelated mocks?
• Would extracted behavior belong in a dedicated collaborator instead of another private method?
• Has extraction preserved authorization, audit, logging, and transaction behavior?
• Are method boundaries meaningful rather than created to satisfy a line limit?
• Are query counts, remote-call counts, and memory use still appropriate?

24. Common Pull Request Review Comments

1. processClaim() validates input, calculates benefits, saves entities, and publishes messages. Please keep it as the coordinator and extract these responsibilities behind clear interfaces.
2. This helper is named getCustomerDetails(), but it also updates the database. Please make the side effect explicit or separate the query from the command.
3. The new boolean argument selects two different workflows. Could we expose two intention-revealing methods instead?
4. Product lookup occurs once per item. Please replace this with a batch query and validate missing IDs after loading.
5. This extraction leaves nine parameters. These values appear to belong to the same operation; please introduce a focused command or domain object.
6. The remote call is executed inside a transactional method. Please document the required atomicity and narrow the transaction if the call does not need to hold database resources.
7. handleData() does not tell readers what business rule is applied. Please rename it to describe the outcome.
8. The catch block now swallows the original exception. Please preserve the cause and translate it at the integration boundary.
9. Please add a test proving that inventory is released when the payment client throws, not only when it returns a failure response.
10. This one-line method adds another navigation step but does not reveal intent or isolate behavior. I suggest keeping the expression at the call site.

25. Code Review Exercise

Review the following employee-access method. Identify its problems, code smells, production risks, and suitable improvements. Do not assume that making the method shorter is the only goal.

@Service
public class EmployeeAccessService {
    private final EmployeeRepository employeeRepository;
    private final AccessRepository accessRepository;
    private final IdentityProviderClient identityProviderClient;
    private final EmailClient emailClient;

    @Transactional
    public void updateAccess(Long employeeId, List<String> roles, boolean deactivate) {
        Employee employee = employeeRepository.findById(employeeId).orElse(null);
        if (employee != null) {
            if (deactivate) {
                employee.setActive(false);
                accessRepository.deleteByEmployeeId(employeeId);
                identityProviderClient.disable(employee.getExternalId());
                emailClient.send(employee.getEmail(), "Your access was disabled");
            } else {
                if (roles != null && !roles.isEmpty()) {
                    for (String role : roles) {
                        if (role != null && !role.isBlank()) {
                            accessRepository.save(new EmployeeAccess(employeeId, role));
                        }
                    }
                    identityProviderClient.updateRoles(employee.getExternalId(), roles);
                    emailClient.send(employee.getEmail(), "Your access was updated");
                }
            }
            employeeRepository.save(employee);
        }
    }
}

While reviewing, consider:

• What responsibilities are combined?
• Which failures are silently ignored?
• Which operations may be inefficient or unsafe inside the transaction?
• Is the boolean parameter a clear API?
• What happens after a partial failure?
• Which rules deserve focused methods or collaborators?

26. Exercise Solution

Review findings

• updateAccess() implements two distinct use cases: deactivation and role replacement.
• A missing employee produces a silent no-op, which can appear successful to an API caller.
• Nested conditionals hide the valid execution paths.
• Null, blank, duplicate, and unknown roles are not handled through a clear policy.
• One save() call is executed per role; duplicates may also create duplicate records.
• Existing roles are not clearly replaced, so stale access may remain.
• Repository writes and identity-provider calls are mixed inside one database transaction.
• If the identity provider succeeds and the database transaction rolls back, systems disagree.
• If email fails, it may roll back database work or produce a misleading API failure.
• Deactivation notification wording and timing are coupled to the business transaction.
• Authorization and audit requirements are absent from the visible flow.

Improved code

@Service
public class EmployeeAccessService {
    private final EmployeeService employeeService;
    private final RolePolicy rolePolicy;
    private final AccessPersistenceService accessPersistenceService;
    private final IdentityAccessService identityAccessService;
    private final ApplicationEventPublisher eventPublisher;

    public EmployeeAccessService(EmployeeService employeeService,
                                 RolePolicy rolePolicy,
                                 AccessPersistenceService accessPersistenceService,
                                 IdentityAccessService identityAccessService,
                                 ApplicationEventPublisher eventPublisher) {
        this.employeeService = employeeService;
        this.rolePolicy = rolePolicy;
        this.accessPersistenceService = accessPersistenceService;
        this.identityAccessService = identityAccessService;
        this.eventPublisher = eventPublisher;
    }

    public void replaceRoles(Long employeeId, Collection<String> requestedRoles) {
        Employee employee = employeeService.getActiveEmployee(employeeId);
        Set<String> roles = rolePolicy.validateAndNormalize(requestedRoles);
        identityAccessService.replaceRoles(employee.getExternalId(), roles);
        accessPersistenceService.replaceRoles(employee.getId(), roles);
        eventPublisher.publishEvent(EmployeeRolesChangedEvent.from(employee, roles));
    }

    public void deactivateEmployee(Long employeeId) {
        Employee employee = employeeService.getEmployee(employeeId);
        identityAccessService.disable(employee.getExternalId());
        accessPersistenceService.deactivate(employee.getId());
        eventPublisher.publishEvent(EmployeeDeactivatedEvent.from(employee));
    }
}

@Service
public class AccessPersistenceService {
    private final EmployeeRepository employeeRepository;
    private final AccessRepository accessRepository;

    public AccessPersistenceService(EmployeeRepository employeeRepository,
                                    AccessRepository accessRepository) {
        this.employeeRepository = employeeRepository;
        this.accessRepository = accessRepository;
    }

    @Transactional
    public void replaceRoles(Long employeeId, Set<String> roles) {
        accessRepository.deleteByEmployeeId(employeeId);
        List<EmployeeAccess> accesses = roles.stream()
                .map(role -> new EmployeeAccess(employeeId, role))
                .toList();
        accessRepository.saveAll(accesses);
    }

    @Transactional
    public void deactivate(Long employeeId) {
        Employee employee = employeeRepository.findById(employeeId)
                .orElseThrow(() -> new EmployeeNotFoundException(employeeId));
        employee.deactivate();
        accessRepository.deleteByEmployeeId(employeeId);
    }
}

Why the changes help

• Two public methods replace the unclear boolean mode and express separate use cases.
• Missing employees now produce a defined domain exception.
• RolePolicy centralizes normalization, duplicate handling, supported-role validation, and empty-set policy.
• saveAll() avoids one repository call per role.
• Persistence runs in focused transactional methods.
• Events separate notification from the synchronous business flow.
• The identity-provider/database consistency strategy is now visible and can be strengthened with an outbox, reconciliation job, or workflow state as required.
• Tests can target role policy, persistence, identity integration, and orchestration independently.

The exact order of identity-provider and database updates depends on the system’s consistency and recovery requirements. The important review outcome is that partial-failure behavior must be explicit, observable, and recoverable.

27. Interview Perspective

Interviewers often show a long service method and ask how you would review or refactor it. A strong answer should go beyond “split it into smaller methods.” Explain:

• How you identify responsibilities and abstraction levels.
• Why behavior-preserving tests come before refactoring.
• When a private method is enough and when a new collaborator is appropriate.
• How transaction boundaries and remote calls affect the design.
• How focused methods improve testing and failure handling.
• Why line count is a signal rather than a strict rule.
• How you would deliver the refactor in small, reviewable commits.

For senior or Spring Boot interviews, expect follow-up questions about @Transactional proxy behavior, idempotency, after-commit events, outbox patterns, exception translation, and distributed consistency.

28. Interview Questions and Answers

Basic question: What is a small and focused method?

A small and focused method performs one cohesive unit of work and has one primary reason to change. Its name expresses intent, its contract is understandable, and its body stays at a consistent abstraction level. Small does not mean obeying an arbitrary line count.

Intermediate question: What signs show that a method has too many responsibilities?

Common signs include a name containing “and,” many unrelated dependencies, several levels of nesting, boolean mode flags, validation mixed with persistence, and database or external calls mixed with calculations. Another strong sign is that unrelated feature changes repeatedly modify the same method.

Advanced question: When should extracted logic move to another class instead of a private method?

Move it when the logic represents a separate business capability, uses a distinct set of dependencies, needs independent testing or reuse, or changes for a different reason than the original class. Keeping many focused private methods in one class can still leave a God class.

Scenario-based question: A 120-line method is transactional and calls two external services. How would you refactor it?

First, capture current behavior and failure paths with tests. Separate validation, pure calculations, persistence, and integrations while preserving behavior. Then document atomicity requirements and narrow the database transaction so it does not remain open during slow network calls. Finally, define idempotency, compensation, and recovery for partial failures rather than assuming method extraction solves consistency.

Code-review question: Should a reviewer reject every method longer than twenty lines?

No. Line count is a useful warning, not a correctness rule. The reviewer should evaluate cohesion, abstraction level, branching, side effects, testability, and reasons to change. A cohesive mapping method may legitimately be longer, while a short method may still mix unrelated responsibilities.

Real-project question: How do focused methods improve debugging in production?

They give failures meaningful boundaries, so stack traces, metrics, logs, and traces identify a specific operation such as inventory reservation or payment collection. They also make it easier to attach relevant context at the correct integration boundary without logging sensitive data everywhere.

Additional question: Can too many small methods make code worse?

Yes. Excessive extraction creates indirection, vague helper names, scattered control flow, and navigation overhead. Extract only when a method reveals intent, removes duplication, isolates a changing rule, or creates a useful testing or side-effect boundary.

29. Quick Rule to Remember

One method, one clear purpose, one level of abstraction.

30. Final Takeaway

Developers should use focused methods to express business intent, isolate rules, make side effects visible, and create practical testing boundaries. The goal is cohesive design, not the smallest possible line count.

Reviewers should check whether a method mixes validation, calculation, persistence, and integrations; hides expensive calls; uses unclear flags; or makes failure behavior difficult to understand. They should also verify that extraction has not changed transaction, authorization, audit, performance, or exception behavior.

Production code should avoid God methods, meaningless helpers, hidden side effects, long transactions around remote calls, and refactors performed without regression protection. A good method lets the next developer understand what it does, why it exists, how it fails, and where to change it safely.