Code Review · Java

Java Code Review: Long Method / God Method

Review a Java service method that mixes validation, pricing, payment, persistence, inventory, and notifications in one place.

Code Review Exercise

Review the following service method. The code is not meant to be completely broken. That is what makes this kind of review realistic: the method may work, but it is hard to understand, test, and change.

java

public class OrderService {
    private final OrderRepository orderRepository;
    private final InventoryService inventoryService;
    private final EmailService emailService;
    private final PaymentService paymentService;

    public OrderResult processOrder(Order order) {
        if (order == null) {
            return new OrderResult(false, "Order is required");
        }

        if (order.getItems() == null || order.getItems().isEmpty()) {
            return new OrderResult(false, "Order must contain items");
        }

        double total = 0;
        for (OrderItem item : order.getItems()) {
            if (item.getQuantity() <= 0) {
                return new OrderResult(false, "Invalid quantity");
            }

            if (!inventoryService.hasStock(item.getSku(), item.getQuantity())) {
                return new OrderResult(false, "Item is out of stock");
            }

            total += item.getPrice() * item.getQuantity();
        }

        if (order.getCustomer().isVip()) {
            total = total * 0.90;
        }

        if (total > 500) {
            total = total - 25;
        }

        PaymentResult payment = paymentService.charge(
            order.getCustomer().getPaymentToken(),
            total
        );

        if (!payment.isSuccessful()) {
            return new OrderResult(false, "Payment failed");
        }

        for (OrderItem item : order.getItems()) {
            inventoryService.reserve(item.getSku(), item.getQuantity());
        }

        order.setTotal(total);
        order.setStatus("PAID");
        orderRepository.save(order);

        emailService.sendConfirmation(order.getCustomer().getEmail(), order);

        return new OrderResult(true, "Order processed");
    }
}

What would you mention in a code review?

Review Findings

Issue #1: The method has too many responsibilities

processOrder() validates input, checks inventory, calculates discounts, charges payment, reserves stock, saves the order, sends email, and builds the response. That is too much for one method to own.

Issue #2: Business rules are mixed into workflow code

VIP discounts, large-order discounts, invalid quantity checks, and stock checks are all embedded directly inside the method. As these rules grow, this method will become harder to safely modify.

Issue #3: The method is hard to test

To test one discount rule, a test may need to set up inventory, payment, repository, and email dependencies. Smaller methods or collaborators would allow more focused tests.

Issue #4: Side effects happen in several places

The method charges a payment, reserves inventory, saves the order, and sends an email. These side effects make failure handling more important and make the method harder to reason about.

Issue #5: Magic values and strings hide business meaning

Values like 0.90, 500, 25, and "PAID" are business rules. Constants, enums, or dedicated policy objects would make the intent clearer.

A Better Review Comment

A weak review comment is simply:

This method is too long.

A stronger review comment explains why the length is a problem:

This method mixes validation, pricing, payment, inventory, persistence, and notifications.
Could we extract the validation and pricing rules first? That would make this easier to test and safer to change.

Possible Refactoring Direction

You do not need to design the perfect architecture during a code review. A good first step is to name the responsibilities that are currently hidden inside the method.

java

public OrderResult processOrder(Order order) {
    validateOrder(order);

    Money total = pricingService.calculateTotal(order);

    paymentService.charge(order.getCustomer(), total);

    inventoryService.reserveItems(order.getItems());

    Order savedOrder = orderRepository.save(markAsPaid(order, total));

    emailService.sendConfirmation(savedOrder);

    return OrderResult.success(savedOrder.getId());
}

This is not automatically perfect, but it makes the workflow easier to scan and gives each responsibility a name.

What Not To Overdo

Do not suggest splitting every five lines into a separate method just to reduce line count. The goal is not smaller code for its own sake.

The goal is to separate responsibilities, improve testability, and make business rules easier to understand.

Interview Follow-Ups

Is a long method always bad?

No. Length alone is not the real issue. The bigger concern is whether the method mixes unrelated responsibilities or becomes hard to test and change.

What is a god method?

A god method is a method that knows too much or does too much. It often coordinates many unrelated business rules, side effects, and infrastructure calls.

What is the first refactoring you would suggest?

Start by extracting responsibilities with clear names, such as validation, pricing, payment, inventory reservation, and notification.

How do you avoid over-engineering the fix?

Refactor toward the pain. If pricing rules are changing often, extract pricing first. If tests are hard to write, extract the dependencies that make testing difficult.

What Interviewers Expect

A junior reviewer may say the method is long.

A stronger reviewer explains the engineering impact:

Maintainability: too many responsibilities in one method.
Testability: hard to test one rule without setting up everything else.
Readability: business rules are hidden inside workflow code.
Design: likely candidates for extraction are validation, pricing, payment, and notification.