Code Review на Kotlin: 7 ловушек, которые убивают ваш проект

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

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

Начать курс

Почему code review на Kotlin — это не про "а я бы написал иначе"?

Code review (или ревью кода) — это священный ритуал в любой уважающей себя команде. Но когда речь заходит о Kotlin, многие разработчики впадают в ступор. "Тут же всё так лаконично!", "Ну, это же expression-based язык" — думаете вы, пролистывая пулл-реквест. И зря.

Kotlin красив, но коварен. Он даёт вам мощные инструменты: scope-функции, sealed-классы, корутины. И ровно столько же возможностей выстрелить себе в ногу. В этой статье мы разберём, на что реально обращать внимание при code review на Kotlin, чтобы ваш код не превратился в "макаронный монстр" с нуллабельными сайд-эффектами.

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

1. Null Safety: когда "?" — это не панацея

Kotlin гордится своей Null Safety. И правильно делает. Но на code review вы обязаны проверять не просто наличие "!!", а логику работы с nullable-типами.

Что точно должно насторожить?

Бесконтрольное использование "!!". Если разработчик ставит !! — он должен объяснить в комментарии, почему это значение гарантированно не null. Если объяснения нет — это баг.
Цепочки из safe call (?.) и elvis (?:). Они часто маскируют логические ошибки. Пример:

// Проблема: если user?.address?.city == null, 
// мы молча вернём "Unknown" без логирования
val city = user?.address?.city ?: "Unknown"

Как надо? Если null — это исключительная ситуация, используйте requireNotNull или checkNotNull. Если это норма поведения — логируйте.

val city = user?.address?.city
    ?: run {
        logger.warn("User $userId has no city")
        "Unknown"
    }

На code review kotlin всегда спрашивайте: "А что если этот null — это баг, который мы сейчас замолчали?"

2. Scope-функции: let, apply, run — используйте с умом

Scope-функции — это мощь Kotlin. Но на практике я вижу, как их используют как молоток для любых гвоздей. Проверка кода Kotlin должна включать анализ: "Зачем здесь именно let, а не run?"

Типичные грабли:

let для проверки on null. Ок, но не вкладывайте их друг в друга:

// Ужас
user?.let { u ->
    address?.let { a ->
        process(u, a)
    }
}

// Хорошо
if (user != null && address != null) {
    process(user, address)
}

apply для мутации объекта. Это его родная стихия. Но если вы используете apply внутри let — стоп. Это пахнет спагетти.

Помните: with — для группировки вызовов, also — для сайд-эффектов (логирование), run — для вычислений в контексте. Не путайте.

3. Иммутабельность: var — это красная тряпка

Kotlin пропагандирует val. И это прекрасно. Но на ревью кода Kotlin я часто вижу, как разработчики пишут var там, где можно обойтись val с копированием.

// Плохо
var result = mutableListOf<Int>()
items.forEach { result.add(it * 2) }

// Хорошо
val result = items.map { it * 2 }

Правило простое: если вы видите var — спросите: "Почему не val?" Чаще всего ответ вас удивит: "Я не подумал".

Исключение — только если мутация действительно нужна (например, в циклах с накоплением). Но даже там часто можно заменить на fold или reduce.

4. Sealed-классы и when: не забывайте про else

Sealed-классы — это элегантная альтернатива enum и иерархиям. Но они работают идеально только в паре с when.

На code review проверьте: если when исчерпывает все варианты sealed-класса, else не нужен. Если же есть else — это баг или лень.

sealed class NetworkResult {
    data class Success(val data: String) : NetworkResult()
    data class Error(val code: Int) : NetworkResult()
    object Loading : NetworkResult()
}

// Хорошо: все ветки покрыты
fun handle(result: NetworkResult) = when (result) {
    is NetworkResult.Success -> showData(result.data)
    is NetworkResult.Error -> showError(result.code)
    NetworkResult.Loading -> showLoader()
    // else не нужен!
}

Если вы добавили новый подкласс, а when с else это проглотит — вы получите рантайм-баг. Компилятор вас не спасёт.

5. Корутины: Dispatchers и Structured Concurrency

Корутины — это must-have для асинхронного кода. Но проверка кода на Kotlin без анализа корутин — это профанация.

Что проверять обязательно:

Не используйте GlobalScope. Точка. Исключение — только демо-проекты. В production всегда должен быть свой scope (viewModelScope, lifecycleScope или кастомный).
Dispatchers.IO не для всего. Если вы пишете withContext(Dispatchers.IO) { ... }, убедитесь, что внутри реально IO-операция (сеть, диск). Если там просто вычисления — используйте Dispatchers.Default.
Не блокируйте корутины. Вызов runBlocking внутри suspend-функции — это моветон и верный путь к deadlock.

// Плохо
fun loadData() {
    GlobalScope.launch {
        // ...
    }
}

// Хорошо
fun loadData(scope: CoroutineScope) {
    scope.launch {
        // ...
    }
}

На code review kotlin всегда обращайте внимание на то, как управляется жизненный цикл корутин. Утечка корутины — это утечка памяти и расход батареи на мобильных устройствах.