Java Code Review: What Seniors Look For and How Not to Fail a Code Review

Java Code Review: What Seniors Look For and How Not to Fail a Code Review

You wrote the code, tested it locally, ran the tests — everything is green. You open a Pull Request, and a couple of hours later you see a dozen comments from a senior colleague. Sound familiar? A Java code review is not a bureaucratic procedure, but a real exam of a developer's maturity. Seniors see code differently: they don't just check if it works, they look at how long it will work and how difficult it will be to change.

In this article, we'll break down the most frequent comments experienced Java developers leave during code reviews. You'll learn how to write clean, maintainable code and pass reviews on the first try.

1. Ignoring SOLID Principles and Clean Architecture

This is perhaps the most common comment from seniors. When you write a 200-line method or a class that does everything under the sun — it immediately stands out. A senior won't waste time analyzing such code; they'll simply ask you to break it down.

Example of bad code:

public class OrderService {    public void processOrder(Order order) {        // 1. Validation        if (order.getItems().isEmpty()) {            throw new IllegalArgumentException("Order must have items");        }        // 2. User check        User user = userRepository.findById(order.getUserId());        if (user == null || user.isBlocked()) {            throw new SecurityException("User not valid");        }        // 3. Discount calculation        double discount = 0;        if (order.getTotal() > 1000) {            discount = 0.1;        }        // 4. Saving        order.setDiscount(discount);        orderRepository.save(order);        // 5. Sending email        emailService.sendOrderConfirmation(order);    }}

What a senior will say: "This method violates the Single Responsibility Principle. It is responsible for validation, calculation, saving, and sending notifications. Split it into separate methods or extract them into different services."

How to fix it:

public class OrderService {    private final OrderValidator validator;    private final DiscountCalculator discountCalculator;    private final OrderRepository orderRepository;    private final NotificationService notificationService;

    public void processOrder(Order order) {        validator.validate(order);        double discount = discountCalculator.calculate(order);        order.setDiscount(discount);        orderRepository.save(order);        notificationService.sendOrderConfirmation(order);    }}

In Java development, the Single Responsibility Principle is fundamental. If you see the word "and" in a method (validate and save and send), it's time to refactor.

2. Working with null and NPE (NullPointerException)

NullPointerException is the bane of Java developers. Seniors hate unexpected NPEs and always pay attention to how you handle null. A typical comment: "What happens if the method returns null?"

Example:

public String getUserEmail(Long userId) {    User user = userRepository.findById(userId);    return user.getEmail().toLowerCase();}

Senior's comment: "If the user is not found, user will be null — we'll get an NPE. Use Optional or at least a null check."

Fixed version:

public String getUserEmail(Long userId) {    return userRepository.findById(userId)            .map(User::getEmail)            .map(String::toLowerCase)            .orElseThrow(() -> new UserNotFoundException("User not found: " + userId));}

Using Optional is the standard in modern Java.