Code review в Kotlin: что проверять и как давать обратную связь

Code review в Kotlin: что проверять и как давать обратную связь

Code review — один из самых эффективных инструментов для поддержания качества кода, обмена знаниями в команде и предотвращения багов. Однако просто «пробежать глазами» по пул-реквесту недостаточно. В языке Kotlin, с его богатой системой типов, корутинами и функциональными возможностями, важно обращать внимание на специфические моменты. В этой статье мы разберем, на что именно смотреть при ревью кода на Kotlin, и как формулировать замечания, чтобы они приносили пользу, а не обиду.

1. Основные принципы конструктивной обратной связи

Прежде чем переходить к техническим деталям, напомним несколько правил, которые делают code review полезным для всех участников.

Фокус на код, а не на авторе

Вместо «Ты написал плохой код» говорите «Этот метод можно упростить с помощью let». Конкретика и нейтральный тон снижают напряжение.

Разделение на категории

Пометьте замечания как:

• Blocking — обязательно исправить (логическая ошибка, утечка ресурсов).
• Nitpick — мелкие придирки (стиль, форматирование).
• Question — уточнение (непонятно намерение).

Объяснение «почему»

Недостаточно сказать «используй filterNotNull()». Добавьте: «Это избавит от лишнего цикла и сделает код самодокументированным». Так автор учится, а не просто исправляет.

2. Null safety и типы: главные союзники Kotlin

Kotlin спроектирован так, чтобы избежать NullPointerException на этапе компиляции. Проверяйте, не нарушена ли эта защита.

Избегайте лишних !! (not-null assertion)

Оператор !! — это крик о помощи. Его использование должно быть исключением, а не правилом.

// Плохо: риск NPE
fun process(user: User?) {
    val name = user!!.name
}

// Хорошо: безопасный вызов
fun process(user: User?) {
    val name = user?.name ?: "Гость"
}

Что проверять: если видите !!, спросите автора, можно ли заменить на ?.let, ?: (elvis) или requireNotNull().

Правильное использование nullable-типов

Не все параметры должны быть nullable. Если функция всегда ожидает значение, объявите тип без вопросительного знака.

// Спорно: null ничего не значит
fun log(message: String?) {
    println(message ?: "null")
}

// Лучше: явное требование
fun log(message: String) {
    println(message)
}

3. Корутины и потоки: асинхронность без боли

Kotlin Coroutines — мощный инструмент, но их легко использовать неправильно. При ревью обращайте внимание на следующие моменты.

Правильный диспетчер для задач

Не запускайте долгие операции (сеть, диск) на Dispatchers.Main.

// Плохо: блокирует UI
GlobalScope.launch(Dispatchers.Main) {
    val data = fetchFromNetwork() // висит
}

// Хорошо: переключение контекста
viewModelScope.launch {
    val data = withContext(Dispatchers.IO) {
        fetchFromNetwork()
    }
    updateUI(data)
}

Утечка корутин и structured concurrency

Используйте viewModelScope, lifecycleScope или coroutineScope вместо GlobalScope. Последний живёт, пока живёт приложение, и легко приводит к утечкам.

// Опасно: утечка
GlobalScope.launch { /* ... */ }

// Безопасно: привязано к жизненному циклу
lifecycleScope.launch { /* ... */ }

4. Коллекции и функциональный стиль: краткость и читаемость

Kotlin предлагает богатую библиотеку для работы с коллекциями. Используйте её вместо явных циклов.

Цепочки вместо мутаций

Избегайте ручного добавления в список, если можно собрать результат через map или filter.

// Плохо: мутация
val result = mutableListOf<String>()
for (item in items) {
    if (item.isValid()) {
        result.add(item.name)
    }
}

// Хорошо: функциональный подход
val result = items
    .filter { it.isValid() }
    .map { it.name }

Проверка на пустоту

Вместо if (list.size > 0) используйте if (list.isNotEmpty()). Это читается как естественный язык.

Sealed class и when

При работе с конечным набором состояний используйте sealed class. Компилятор проверит, что все ветки обработаны.

sealed class Result {
    data class Success(val data: String) : Result()
    data class Error(val message: String) : Result()
}

fun handle(result: Result) = when (result) {
    is Result.Success -> println(result.data)
    is Result.Error -> println(result.message)
    // Компилятор не даст забыть ветку
}

5. Чек-лист для code review на Kotlin

Для удобства соберём ключевые пункты в один список. Пройдитесь по нему перед тем, как одобрить PR.

• Null safety: нет ли лишних !!? Все ли nullable-типы обработаны через ?.let или ?:?
• Корутины: правильный ли диспетчер? Нет ли утечек через GlobalScope? Не забыт ли withContext для IO?
• Функциональные цепочки: можно ли заменить цикл на map/filter/fold?
• Sealed class: все ли ветки when обработаны? Нет ли лишнего else?
• Иммутабельность: используются ли val вместо var там, где это возможно?
<