Ревью кода на Java: что ищут senior-ы и как не провалить код-ревью

Онлайн-тренажер Python для начинающих

Изучайте Python легко и без перегрузки теорией. Решайте практические задачи с автоматической проверкой, получайте подсказки на русском языке и пишите код прямо в браузере — без необходимости что-либо устанавливать.

Начать курс

Ревью кода на Java: что ищут senior-ы и как не провалить код-ревью

Вы написали код, проверили его локально, запустили тесты — всё зелёное. Открываете Pull Request и через пару часов видите десяток комментариев от старшего коллеги. Знакомо? Ревью кода на Java — это не бюрократическая процедура, а настоящий экзамен на зрелость разработчика. Senior-ы видят код иначе: они не просто проверяют, работает ли он, а смотрят, как долго он будет работать и как сложно его будет менять.

В этой статье мы разберём самые частые замечания, которые оставляют опытные Java-разработчики на код-ревью. Вы узнаете, как писать чистый, поддерживаемый код и проходить ревью с первого раза.

1. Игнорирование принципов SOLID и чистой архитектуры

Это, пожалуй, самое частое замечание senior-ов. Когда вы пишете метод на 200 строк или класс, который делает всё на свете — это сразу бросается в глаза. Senior не будет тратить время на разбор такого кода, он просто попросит его разбить.

Пример плохого кода:

public class OrderService {
    public void processOrder(Order order) {
        // 1. Валидация
        if (order.getItems().isEmpty()) {
            throw new IllegalArgumentException("Order must have items");
        }
        // 2. Проверка пользователя
        User user = userRepository.findById(order.getUserId());
        if (user == null || user.isBlocked()) {
            throw new SecurityException("User not valid");
        }
        // 3. Расчёт скидки
        double discount = 0;
        if (order.getTotal() > 1000) {
            discount = 0.1;
        }
        // 4. Сохранение
        order.setDiscount(discount);
        orderRepository.save(order);
        // 5. Отправка email
        emailService.sendOrderConfirmation(order);
    }
}

Что скажет senior: «Этот метод нарушает Single Responsibility Principle. Он отвечает за валидацию, расчёт, сохранение и отправку уведомлений. Раздели на отдельные методы или вынеси в разные сервисы.»

Как исправить:

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);
    }
}

В Java-разработке принцип единственной ответственности — это база. Если вы видите в методе слово «and» (проверить и сохранить и отправить), значит, пора рефакторить.

2. Работа с null и NPE (NullPointerException)

NullPointerException — бич Java-разработчиков. Senior-ы ненавидят неожиданные NPE и всегда обращают внимание на то, как вы работаете с null. Типичное замечание: «А что будет, если метод вернёт null?»

Пример:

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

Замечание senior-а: «Если пользователь не найден, user будет null — получим NPE. Используй Optional или хотя бы проверку.»

Исправленный вариант:

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

Использование Optional — это стандарт в современной Java. Senior-ы также советуют не передавать Optional в параметрах методов и не хранить его в полях класса. Он создан только для возвращаемых значений.

3. Магические числа и строки

Ещё один пункт, который мгновенно привлекает внимание на код-ревью. Если вы пишете в коде числа вроде 1000, 60 * 60 * 24 или строки типа "ACTIVE" — ждите комментарий.

Пример:

if (order.getTotal() > 1000) {
    discount = 0.1;
}
if (user.getStatus().equals("ACTIVE")) {
    // ...
}

Замечание: «Что такое 1000 и 0.1? Сделай константы или вынеси в конфигурацию. Строку 'ACTIVE' лучше заменить на enum.»

Исправление:

private static final BigDecimal DISCOUNT_THRESHOLD = BigDecimal.valueOf(1000);
private static final BigDecimal DISCOUNT_RATE = BigDecimal.valueOf(0.1);

// ...
if (order.getTotal().compareTo(DISCOUNT_THRESHOLD) > 0) {
    discount = DISCOUNT_RATE;
}
// И для статуса:
if (user.getStatus() == UserStatus.ACTIVE) { ... }

Магические числа делают код непонятным и сложным в поддержке. Через месяц вы сами не вспомните, что значит 1000. Senior-ы ценят читаемость превыше всего.

4. Неправильная работа с исключениями

Ошибки в Java — это неотъемлемая часть. Но есть большая разница между тем, как junior и senior обрабатывают исключения. Типичные ошибки:

Проглатывание исключений: пустой catch блок — это антипаттерн.
Слишком общие исключения: ловить Exception или Throwable — плохая практика.
Бросание RuntimeException без смысла: лучше использовать конкретные исключения.

Плохой пример:

try {
    // какой-то код
} catch (Exception e) {
    // ничего не делаем
}

Замечание senior-а: «Ты только что проглотил ошибку. Мы никогда не узнаем, что пошло не так. Минимум — залогируй, лучше — пробрось дальше или обработай осмысленно.»

Хороший пример:

try {
    paymentService.processPayment(order);
} catch (PaymentDeclinedException e) {
    log.warn("Payment declined for order {}: {}", order.getId(), e.getMessage());
    throw new OrderProcessingException("Unable to process order", e);
}

Помните: код-ревью Java часто проваливается именно из-за плохой обработки ошибок. Senior смотрит: «А что будет, если внешний сервис упадёт? А если БД недоступна?»