Code Review на Java: что искать в чужом коде

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

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

Начать курс

Почему code review — это не про придирки, а про качество

Представьте: вы открываете пулл-реквизит коллеги. Там 500 строк кода, пахнет копипастой, а в одном месте кто-то забыл закрыть ресурс. Ваша задача — не просто найти баги, а сделать код лучше. Code review на Java — это не полицейский надзор, а совместное творчество. Хороший ревьювер спасает команду от техдолга, плохой — демотивирует или пропускает уязвимости.

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

Архитектура и дизайн: смотрим в корень

Прежде чем нырять в детали, оцените общую картину. Плохая архитектура не лечится форматированием.

Нарушение SOLID

Проверьте, не превратился ли класс в «божественный объект». Пример нарушения Single Responsibility:

public class OrderService {
    public void createOrder(Order order) {
        // валидация
        // расчет скидки
        // отправка email
        // логирование
        // сохранение в БД
    }
}

Здесь пять причин для изменения. Правильно — вынести логирование и отправку в отдельные сервисы.

Избыточная сложность

Иногда код решает простую задачу сложным способом. Спросите себя: «Можно ли проще?» Если да — пишите в ревью. Помните принцип KISS.

Безопасность: дыры, которые нельзя пропускать

Код ревью Java обязан включать проверку безопасности. Даже если проект внутренний.

SQL-инъекции

Конкатенация строк — зло:

// ПЛОХО
String query = "SELECT * FROM users WHERE name = '" + userName + "'";
Statement stmt = connection.createStatement();
ResultSet rs = stmt.executeQuery(query);

Правильно — использовать PreparedStatement:

// ХОРОШО
String query = "SELECT * FROM users WHERE name = ?";
PreparedStatement pstmt = connection.prepareStatement(query);
pstmt.setString(1, userName);
ResultSet rs = pstmt.executeQuery();

Утечка чувствительных данных

Проверьте, не логируются ли пароли, токены или номера карт. Используйте @ToString.Exclude в Lombok или явное игнорирование.

Небезопасное сравнение объектов

// ПЛОХО (сравнение ссылок)
if (password == storedPassword) { ... }
// ХОРОШО
if (password.equals(storedPassword)) { ... }

Производительность: узкие места

Не оптимизируйте преждевременно, но и не пропускайте очевидные тормоза.

Ненужные операции в циклах

// ПЛОХО: каждый раз вызывается length()
for (int i = 0; i < list.size(); i++) { ... }
// ХОРОШО
int size = list.size();
for (int i = 0; i < size; i++) { ... }

Использование Stream для простых задач

Иногда обычный цикл читается и работает быстрее:

// Избыточно
list.stream().filter(x -> x > 5).collect(Collectors.toList());
// Просто
List<Integer> result = new ArrayList<>();
for (int x : list) {
    if (x > 5) result.add(x);
}

Утечка памяти

Проверьте, не забыли ли закрыть InputStream, Connection или Session. Используйте try-with-resources:

try (Connection conn = getConnection();
     PreparedStatement stmt = conn.prepareStatement(sql)) {
    // работа
} catch (SQLException e) {
    log.error("DB error", e);
}

Читаемость и стиль: код как книга

Code review checklist обязательно включает читаемость. Код пишут один раз, читают десятки раз.

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

// ПЛОХО
if (status == 3) { ... }
// ХОРОШО
private static final int STATUS_COMPLETED = 3;
if (status == STATUS_COMPLETED) { ... }

Длинные методы

Метод длиннее 30-40 строк — повод задуматься. Разбивайте на логические блоки.

Непонятные имена

List<String> lst → List<String> userEmails
void process() → void processOrderPayment()

Имена должны отвечать на вопрос «что делает?» или «что хранит?».

Тестирование: проверьте, что код тестируем

Если в пулл-реквизите нет тестов, это красный флаг. Но даже если тесты есть, проверьте:

Покрывают ли они граничные случаи (null, пустой список, отрицательные числа)?
Нет ли тестов, которые зависят от порядка выполнения?
Используются ли моки для внешних зависимостей?

// Хороший тест
@Test
void shouldReturnEmptyListWhenNoUsers() {
    UserService service = new UserService(mockRepo);
    when(mockRepo.findAll()).thenReturn(List.of());
    List<User> result = service.getActiveUsers();
    assertTrue(result.isEmpty());
}

Обработка ошибок: не проглатывать исключения

Пустой catch — зло:

// ПЛОХО
try {
    // risky code
} catch (Exception e) {
    // ничего
}
// ХОРОШО
try {
    // risky code
} catch (SpecificException e) {
    log.error("Operation failed: {}", e.getMessage());
    throw new BusinessException("Unable to complete", e);
}

Проверьте, не перехватывается ли Exception там, где нужно ловить конкретные типы.

Работа с коллекциями и Optional

Проверка на null вместо Optional

// ПЛОХО
if (user != null) {
    String name = user.getName();
}
// ХОРОШО
Optional<User> userOpt = findUser(id);
userOpt.ifPresent(user -> processName(user.getName()));

Изменение коллекции во время итерации

// ПЛОХО: ConcurrentModificationException
for (String item : list) {
    if (item.startsWith("_")) {
        list.remove(item);
    }
}
// ХОРОШО
list.removeIf(item -> item.startsWith("_"));