10 признаков говнокода в мобильной Kotlin разработке или как писать чистый и поддерживаемый код

10 признаков проблемного кода в Kotlin для Android Позвольте сразу объясниться: заголовок намеренно провокационный и субъективный. Он призван не оскорбить, а привлечь внимание к важной теме. Качество кода часто отодвигается на второй план в погоне за сроками и функциональностью. Когда мы только начинаем программировать, наша единственная цель заключается в том, чтобы заставить код работать. И в этот момент любые разговоры о "качестве кода" кажутся абстрактной теорией. Однако с опытом приходит понимание: код, который просто работает сегодня, может стать кошмаром завтра, когда потребуется его изменить, расширить или исправить ошибку. Ниже мы рассмотрим десять характерных признаков, которые часто указывают на потенциальные проблемы в коде на Kotlin. Но важно понимать, что ни один из этих признаков не является абсолютным приговором. Каждый из них дает повод остановиться и спросить себя: "А почему здесь именно так? Есть ли для этого веская причина?" Бывают ситуации, когда нарушение этих "правил" совершенно оправдано. Быстрый прототип, временное решение, специфические требования производительности, работа с легаси кодом - все это может быть вескими причинами для сознательного выбора в пользу "неидеального" решения. Главное, чтобы этот выбор был осознанным. Не нужно принимать приведенный ниже материал за свод заповедей. Это скорее инструмент для рефлексии. Давайте вместе попробуем разобраться, когда эти признаки действительно указывают на проблему, а когда они всего лишь являются следствием разумного компромисса.

1. Метод-монстр, который делает слишком много

Сигнал: Один метод выполняет множество разнородных операций: парсит данные, валидирует их, выполняет расчёты, сохраняет в базу данных, логирует события и отправляет уведомления. Он превращается в многострочную процедуру, в которой смешаны различные уровни абстракции.

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

// ПЛОХО: метод в ViewModel делает всё 
fun processOrder(rawInput: String){
    // Парсинг JSON вручную
    val json = JSONObject(rawInput)
    val email = json.getString("email")
    val password = json.getString("password")
	val amount = json.getDouble("amount")
	val isVip = json.getBoolean("isVip")
	val userId = json.getLong("userId")

    // Валидация
    if (amount <= 0) {
	 _orderStatus.value = OrderStatus.Error("Invalid amount")
	 return
	}
	
	// 3. Бизнес-логика: расчёт итоговой суммы с учётом скидки
	val baseDiscount = if (isVip) 0.15 else 0.05
	val seasonalBonus = if (Calendar.getInstance().get(Calendar.MONTH) == Calendar.DECEMBER) 0.1 else 0.0
	val totalDiscount = (baseDiscount + seasonalBonus).coerceAtMost(0.25)
	val finalAmount = amount * (1 - totalDiscount)

    // 4. Вызов API для подтверждения заказа
	val response = api.submitOrder(userId, finalAmount).execute()
	if (!response.isSuccessful) {
		_orderStatus.value = OrderStatus.Error("Order failed")
		return
	}

    // 5. Сохранение в Room
    orderDao.insert(OrderEntity(userId, finalAmount, System.currentTimeMillis()))

    // 6. Логирование через Timber
	Timber.d("Order processed for user $userId, final amount: $finalAmount")

    // 7. Обновление UI
	_orderStatus.value = OrderStatus.Success(finalAmount)
	
	// 8. Отправка аналитики
	analytics.logEvent("order_submitted", bundleOf("amount" to finalAmount, "user_type" to if (isVip) "vip" else "regular"))
}

Почему это может быть проблемой:

Такой подход создаёт сразу несколько серьёзных трудностей. Во-первых, тестировать этот код становится чрезвычайно сложно - чтобы проверить корректность расчёта, вам придётся эмулировать парсинг строки, валидацию и даже работу с базой данных. Во-вторых, страдает переиспользуемость: логику расчёта нельзя вынести и применить в другом месте, она намертво вплетена в конкретный метод. В-третьих, код становится хрупким - любое изменение, например, формата входных данных или структуры базы, затронет сразу все аспекты обработки заказа. Наконец, такой метод нарушает принцип единственной ответственности (SRP), что означает, что его придётся менять по множеству разных причин: при изменении бизнес-правил, формата логирования, способа отправки уведомлений и так далее.

Что делать:

Разделите логику по уровням абстракции. Каждая функция или компонент должны решать одну конкретную задачу. Начните с простого - вынесите отдельные шаги в приватные функции того же класса

// УЛУЧШЕНИЕ: выделение функций внутри класса
// Data класс для хранения распарсенных данных
private data class OrderData(
    val email: String,
    val password: String,
    val amount: Double,
    val isVip: Boolean,
    val userId: Long
)

fun processOrder(rawInput: String) {
    // 1. Парсинг данных
    val orderData = parseOrderData(rawInput)
    if (orderData == null) {
        _orderStatus.value = OrderStatus.Error("Invalid data")
        return
    }
    
    // 2. Валидация
    if (!validateOrderData(orderData)) {
        return
    }
    
    // 3. Расчёт скидки
    val finalAmount = calculateFinalAmount(orderData)
    
    // 4. Подтверждение заказа
    if (!confirmOrderWithApi(orderData.userId, finalAmount)) {
        return
    }
    
    // 5. Сохранение в БД
    saveOrderToDatabase(orderData.userId, finalAmount)
    
    // 6. Логирование
    logOrderProcessing(orderData.userId, finalAmount)
    
    // 7. Обновление UI
    updateUiStatus(finalAmount)
    
    // 8. Аналитика
    sendAnalytics(orderData, finalAmount)
}

// Шаг 1: Парсинг JSON
private fun parseOrderData(rawInput: String): OrderData? {
    return try {
        val json = JSONObject(rawInput)
        OrderData(
            email = json.getString("email"),
            password = json.getString("password"),
            amount = json.getDouble("amount"),
            isVip = json.getBoolean("isVip"),
            userId = json.getLong("userId")
        )
    } catch (e: Exception) {
        Timber.e(e, "Failed to parse order data")
        null
    }
}

// Шаг 2: Валидация
private fun validateOrderData(orderData: OrderData): Boolean {
    return when {
        orderData.amount <= 0 -> {
            _orderStatus.value = OrderStatus.Error("Invalid amount")
            false
        }
        orderData.email.isBlank() -> {
            _orderStatus.value = OrderStatus.Error("Invalid email")
            false
        }
        else -> true
    }
}

// Шаг 3: Бизнес-логика расчета суммы
private fun calculateFinalAmount(orderData: OrderData): Double {
    val baseDiscount = if (orderData.isVip) 0.15 else 0.05
    val seasonalBonus = if (isDecember()) 0.1 else 0.0
    val totalDiscount = (baseDiscount + seasonalBonus).coerceAtMost(0.25)
    return orderData.amount * (1 - totalDiscount)
}

// Вспомогательная функция для проверки месяца
private fun isDecember(): Boolean {
    return Calendar.getInstance().get(Calendar.MONTH) == Calendar.DECEMBER
}

// Шаг 4: API вызов
private fun confirmOrderWithApi(userId: Long, finalAmount: Double): Boolean {
    return try {
        val response = api.submitOrder(userId, finalAmount).execute()
        if (!response.isSuccessful) {
            _orderStatus.value = OrderStatus.Error("Order failed")
            false
        } else {
            true
        }
    } catch (e: Exception) {
        _orderStatus.value = OrderStatus.Error("Network error")
        false
    }
}

// Шаг 5: Сохранение в БД
private fun saveOrderToDatabase(userId: Long, finalAmount: Double) {
    try {
        orderDao.insert(OrderEntity(userId, finalAmount, System.currentTimeMillis()))
    } catch (e: Exception) {
        Timber.e(e, "Failed to save order to database")
    }
}

// Шаг 6: Логирование
private fun logOrderProcessing(userId: Long, finalAmount: Double) {
    Timber.d("Order processed for user $userId, final amount: $finalAmount")
}

// Шаг 7: Обновление UI
private fun updateUiStatus(finalAmount: Double) {
    _orderStatus.value = OrderStatus.Success(finalAmount)
}

// Шаг 8: Аналитика
private fun sendAnalytics(orderData: OrderData, finalAmount: Double) {
    analytics.logEvent(
        "order_submitted",
        bundleOf(
            "amount" to finalAmount,
            "user_type" to if (orderData.isVip) "vip" else "regular"
        )
    )
}

Этот подход уже значительно улучшает читаемость и упрощает модульное тестирование. Но если логика становится сложнее, или вы работаете в команде, где важна тестируемость и поддерживаемость, стоит пойти дальше - выделить отдельные компоненты с чёткими интерфейсами и использовать внедрение зависимостей (DI). (Ниже приводится несколько облегченный в учебных целях код.)

// ХОРОШО: разделение ответственностей через DI
// Domain модели
data class Order(val userId: Long, val amount: Double, val isVip: Boolean)
data class ProcessedOrder(val order: Order, val finalAmount: Double, val discountApplied: Double)

// Ошибки
sealed class OrderError(message: String) : Exception(message) {
    class ParseError(message: String) : OrderError(message)
    class ValidationError(message: String) : OrderError(message)
    class ApiError(message: String) : OrderError(message)
}

// Интерфейсы
interface OrderParser { fun parse(json: String): Order? }
interface DiscountCalculator { fun calculate(order: Order): Double }
interface OrderRepository { suspend fun save(order: Order, finalAmount: Double, discount: Double) }

//Реализации
class JsonOrderParser : OrderParser {
    override fun parse(json: String): Order? = runCatching {
        JSONObject(json).run {
            Order(
                getLong("userId"),
                getDouble("amount"),
                getBoolean("isVip")
            )
        }
    }.getOrNull()
}

class DiscountCalculatorImpl : DiscountCalculator {
    override fun calculate(order: Order): Double {
        val base = if (order.isVip) 0.15 else 0.05
        val seasonal = if (isDecember()) 0.1 else 0.0
        val discount = (base + seasonal).coerceAtMost(0.25)
        return order.amount * (1 - discount)
    }
    
    private fun isDecember() = Calendar.getInstance()[Calendar.MONTH] == Calendar.DECEMBER
}

class OrderRepositoryImpl(private val dao: OrderDao) : OrderRepository {
    override suspend fun save(order: Order, finalAmount: Double, discount: Double) {
        dao.insert(OrderEntity(
            userId = order.userId,
            originalAmount = order.amount,    // Сохраняем исходную сумму
            finalAmount = finalAmount,        // Сохраняем итоговую сумму
            discountApplied = discount,       // Сохраняем размер скидки
            isVip = order.isVip,
            timestamp = System.currentTimeMillis()
		))
    }
}

///////////
//UseCase//
///////////

class ProcessOrderUseCase(
    private val parser: OrderParser,
    private val calculator: DiscountCalculator,
    private val repository: OrderRepository,
    private val api: ApiService
) {
    suspend operator fun invoke(json: String): Result<ProcessedOrder> {
        // ФИКС: используем elvis operator для nullable
        val order = parser.parse(json) ?: return Result.failure(
            OrderError.ParseError("Failed to parse JSON")
        )
        
        // Валидация
        if (order.amount <= 0) {
            return Result.failure(OrderError.ValidationError("Amount must be positive"))
        }
        
        val finalAmount = calculator.calculate(order)
		val discountAmount = order.amount - finalAmount
        val processedOrder = ProcessedOrder(order, finalAmount, discountAmount)
        
        // API вызов
        return runCatching {
            api.submitOrder(order.userId, finalAmount)
            repository.save(order, finalAmount, discountAmount)
            processedOrder
        }.fold(
			onSuccess = { Result.success(it) },
			onFailure = { Result.failure(OrderError.ApiError(it.message ?: "API call failed")) }
		)
    }
}

/////////////
//ViewModel//
/////////////
sealed class OrderStatus {
    object Idle : OrderStatus()
    object Loading : OrderStatus()
    data class Success(val amount: Double) : OrderStatus()
    data class Error(val message: String) : OrderStatus()
}

class OrderViewModel(
    private val processOrder: ProcessOrderUseCase
) : ViewModel() {
    
    private val _status = MutableStateFlow<OrderStatus>(OrderStatus.Idle)
    val status = _status.asStateFlow()
    
    fun process(json: String) = viewModelScope.launch {
        _status.value = OrderStatus.Loading
        
        when (val result = processOrder(json)) {
            is Result.Success -> {
                _status.value = OrderStatus.Success(result.value.finalAmount)
            }
            is Result.Failure -> {
                val errorMessage = when (val error = result.exception) {
                    is OrderError.ParseError -> "Failed to parse: ${error.message}"
                    is OrderError.ValidationError -> "Validation failed: ${error.message}"
                    is OrderError.ApiError -> "Server error: ${error.message}"
                    else -> "Unknown error"
                }
                _status.value = OrderStatus.Error(errorMessage)
            }
        }
    }
}

/////////////////////////////////////////////////////
// Внедрение зависимостей (минимальное для примера)//
/////////////////////////////////////////////////////

// Простой Service Locator вместо Dagger/Hilt
object ServiceLocator {
    val orderDao: OrderDao by lazy { Database.instance.orderDao() }
    val api: ApiService by lazy { RetrofitFactory.create() }
    
    val orderParser: OrderParser by lazy { JsonOrderParser() }
    val calculator: DiscountCalculator by lazy { DiscountCalculatorImpl() }
    val repository: OrderRepository by lazy { OrderRepositoryImpl(orderDao) }
    
    val processOrderUseCase: ProcessOrderUseCase by lazy {
        ProcessOrderUseCase(orderParser, calculator, repository, api)
    }
}

// В Activity/Fragment
val viewModel = OrderViewModel(ServiceLocator.processOrderUseCase)

Контекст: В быстром прототипе, одноразовом скрипте или при работе с легаси кодом подобный "монолитный" подход может быть оправданным компромиссом. Однако как только появляются требования к тестированию, расширению функциональности или командной работе, пора задуматься о разделении ответственностей.

2. «Магические» значения без пояснения

Сигнал: В коде встречаются числа, строки или другие литералы без пояснения их смысла: 0.15, "active", 3000. Такие значения требуют от читателя знаний «вне текста» - только автор кода помнит, что они означают и откуда взялись.

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

// ПЛОХО: что означает "active"? А если завтра API начнёт присылать "ACTIVE" или "enabled"?
if (user.status == "active") {
    showProfile()  // <- отступ добавлен для читаемости
}

// ПЛОХО: через сколько миллисекунд? Зачем именно 3000? Это таймаут подключения или задержка анимации?
handler.postDelayed(updateRunnable, 3000)

// ПЛОХО: откуда взялось 0.2? Это налог, комиссия или скидка?
val total = amount * 0.2

Почему это может быть проблемой:

Магические значения создают сразу несколько проблем. Во-первых, код становится трудно читать и понимать - каждый раз приходится гадать, что означает то или иное число или строка. Во-вторых, такой код сложно изменять: если значение "active" нужно заменить на "enabled", придётся вручную искать все вхождения по всему проекту, рискуя что-то пропустить. В-третьих, опечатки в строковых литералах ("activ" вместо "active") не обнаруживаются компилятором, что приводит к трудноуловимым багам. Наконец, отсутствует автодополнение в IDE, что замедляет разработку и повышает вероятность ошибок.

Что делать:

Замените магические значения на самодокументируемые, типобезопасные конструкции.

Вариант 1: Используйте enum или sealed class (предпочтительно для ограниченного набора значений)

// ХОРОШО: типобезопасность, автодополнение, невозможность опечататься
enum class UserStatus(val apiValue: String) {
    ACTIVE("active"),
    INACTIVE("inactive"),
    PENDING("pending"),
    BANNED("banned");

	companion object {
		fun fromApi(value: String): UserStatus =
			values().find { it.apiValue == value }
				?: throw IllegalArgumentException("Unknown user status: '$value'")
	}
}

// На границе слоя данных преобразуем строку из API в enum
val parsedStatus = UserStatus.fromApi(userJson.getString("status"))

// Теперь в бизнес-логике используем типобезопасное сравнение
if (parsedStatus == UserStatus.ACTIVE) {
    showProfile()
}

// Или для сложных состояний - sealed class защищает от незамеченных случаев
sealed class PaymentResult {
    data class Success(val transactionId: String) : PaymentResult()
    data class Error(val code: Int, val message: String) : PaymentResult()
    object Processing : PaymentResult()
}

// Компилятор проверит, что все варианты обработаны
when (paymentResult) {
    is PaymentResult.Success -> { /* ... */ }
    is PaymentResult.Error -> { /* ... */ }
    PaymentResult.Processing -> { /* ... */ }
}

Вариант 2: Если нельзя изменить источник данных (внешний API, легаси система), используйте константы

// ХОРОШО: константы с говорящими именами
object ApiConstants {
    const val STATUS_ACTIVE = "active"
    const val STATUS_INACTIVE = "inactive"
    const val STATUS_PENDING = "pending"
}

object TimeoutValues {
    const val CONNECTION_TIMEOUT_MS = 3000L
    const val READ_TIMEOUT_MS = 5000L
    const val ANIMATION_DELAY_MS = 300L
}

// Использование
if (user.status == ApiConstants.STATUS_ACTIVE) { ... }
handler.postDelayed(updateRunnable, TimeoutValues.CONNECTION_TIMEOUT_MS)

Вариант 3: Для вычисляемых значений или параметров используйте функции с понятными именами

// ВМЕСТО ЭТОГО:
val price = amount * 0.2

// ЛУЧШЕ ТАК:
private const val STANDARD_TAX_RATE = 0.2

val price = calculateTax(amount)

private fun calculateTax(amount: Double): Double = amount * STANDARD_TAX_RATE

Особый случай: Android-ресурсы

// ВМЕСТО магических чисел для размеров
val padding = 16
val textSize = 14

// ИСПОЛЬЗУЙТЕ ресурсы
val padding = resources.getDimensionPixelSize(R.dimen.default_padding) // целое число пикселей
val textSize = resources.getDimension(R.dimen.text_medium) // дробное значение в пикселях

// Или хотя бы константы с указанием единиц измерения
private const val DEFAULT_PADDING_DP = 16
private const val MEDIUM_TEXT_SIZE_SP = 14

Нюанс: Конечно константы - это шаг в правильном направлении, но они не обеспечивают типобезопасность в той же мере, что и enum. Строковая константа ApiConstants.STATUS_ACTIVE всё ещё может быть случайно перепутана с другой строковой константой. Поэтому рассматривайте константы как временное решение для работы с внешними системами, а в пределах вашего приложения стремитесь к типобезопасным решениям. Помните принцип: «Сделать неверное состояние невозможным для представления» - enum и sealed class помогают достичь этого.

3. Неосознанная работа с null

Сигнал: Глубокие цепочки безопасных вызовов (?.), оператор Элвиса (?:) или даже !! используются без понимания, допустим ли null в данном контексте. Особенно тревожны случаи, когда null - признак ошибки, но код молча подставляет значение по умолчанию.

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

// ПЛОХО: что, если profile.name обязан существовать? Скрываем ошибку
user?.profile?.name?.let { showWelcome(it) } ?: showWelcome("Guest")

// ПЛОХО: скрытый NPE через !! - падение будет неинформативным
val button = findViewById<Button>(R.id.submit_button)!!
button.setOnClickListener { ... }

// ПЛОХО: lateinit без гарантии инициализации
class MyFragment : Fragment() {
    private lateinit var analytics: AnalyticsService
    
    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        super.onViewCreated(view, savedInstanceState)
        // Инициализация только при определенных условиях
        if (BuildConfig.DEBUG) {
            analytics = DebugAnalytics()
        } // В релизе останется неинициализированным!
    }
}

// НЕ ИДЕАЛЬНО: уже лучше, но всё ещё скрывает проблему
val button = findViewById<Button>(R.id.submit_button)
if (button != null) {
    button.setOnClickListener { ... }
} else {
    Log.e("MyActivity", "Button not found") // А пользователь видит просто белый экран
}

Почему это может быть проблемой:

Некорректная работа с null - один из самых распространённых источников ошибок в Kotlin. Если null является результатом нарушения инварианта (например, авторизованный пользователь обязан иметь профиль с именем), то молчаливая подстановка значения по умолчанию ("Guest") скрывает проблему. Это приводит к скрытым багам, которые проявляются только в определённых сценариях, некорректной аналитике (вы думаете, что у вас 100 пользователей, а на деле 90 из них - "гости" из-за ошибки) и постепенной потере доверия к данным в системе. С другой стороны, если null - законная часть домена (например, у пользователя может не быть отчества), то его отсутствие должно обрабатываться явно, но без паники и молчаливого игнорирования.

Что делать:

Различайте два принципиально разных случая:

Случай 1: null допустим (доменная опциональность)

// ХОРОШО: middleName может отсутствовать - это нормально
val displayName = listOfNotNull(
    user.firstName,
    user.middleName?.takeIf { it.isNotBlank() },
    user.lastName
).joinToString(" ")

// Или через Элвис с явным указанием причины
val email = user.email ?: "Email not provided"  // Явно указываем, что email может отсутствовать

Случай 2: null недопустим (нарушение инварианта)

// ХОРОШО: явно указываем, что это ошибка с понятным сообщением
val userId = requireNotNull(intent.getStringExtra("USER_ID")) {
    "Activity launched without required USER_ID extra. " +
    "Make sure to use IntentBuilder.createUserDetailIntent(userId)"
}

// Для сложных объектов с подробным сообщением
val profileName = user.profile?.name ?: error(
    "Invariant violated: authorized user must have a profile name. " +
    "User ID: ${user.id}, registration date: ${user.createdAt}. " +
    "Check user migration scripts."
)

// Для View: лучше явная проверка, чем неявный краш
val submitButton = findViewById<Button>(R.id.submit_button) ?: throw IllegalStateException(
    "Submit button (ID: submit_button) not found in layout. " +
    "Check if the layout R.layout.${javaClass.simpleName.replace("Activity", "")} contains this view."
)
submitButton.setOnClickListener { ... }

Дополнительно: проектируйте типы так, чтобы null был невозможен

// ВМЕСТО nullable полей, которые "иногда" должны быть не null
class User(val id: String, val profile: Profile?) // Когда profile null? При регистрации?

// ЛУЧШЕ: явно моделируйте состояния
sealed class User {
    data class Unregistered(val email: String, val tempId: String) : User()
    data class Registered(val id: String, val profile: Profile) : User()
}

// Теперь компилятор заставляет обработать все случаи
when (user) {
    is User.Unregistered -> showRegistrationForm(user.email)
    is User.Registered -> showProfile(user.profile)
}

Ключевой принцип: сделайте ожидания явными. Если значение обязано существовать, заставьте код упасть с понятным, действенным сообщением при нарушении этого условия. Если значение может отсутствовать. то обработайте это осознанно, но не прячьте проблему за значением по умолчанию.

Дополнительно для Android:

  • Избегайте !! для View - он превращает компиляторную проверку в runtime-ошибку без контекста. Вместо этого используйте безопасный вызов с явной проверкой или View Binding/Data Binding.
  • Будьте осторожны с lateinit - используйте только там, где инициализация гарантирована (например, в onCreate). Для зависимостей предпочитайте внедрение через конструктор или DI-контейнеры.
  • Используйте View Binding - он генерирует nullable-типы по техническим причинам, но на практике view никогда не бывает null, если она есть в layout. Это позволяет обращаться к ней напрямую, не опасаясь NPE, и избегать как !!, так и лишних проверок.
  • В реактивных потоках (StateFlow<User?>) используйте filterNotNull() или mapNotNull(), чтобы явно отделить обработку наличия/отсутствия данных. Ещё лучше - использовать sealed class для представления состояния загрузки данных.

4. Игнорирование идиом Kotlin

Сигнал: Код написан так, будто это Java: ArrayList, ручные циклы for (i in 0 until size), геттеры/сеттеры, утилитные классы, колбэки вместо лямбд. Такой код работает, но игнорирует философию Kotlin - делать код безопасным, кратким и выразительным.

Пример проблемного кода (часто встречается в Android):

// ПЛОХО: Java-стиль в Kotlin
val items = ArrayList<String>() // Мутабельный список без причины
items.add("a")
items.add("b")

// Ручной цикл с индексами
for (i in 0 until items.size) {
    println("${i}: ${items[i]}")
}

// Колбэк в стиле Java
api.loadUser(object : UserCallback {
    override fun onSuccess(user: User) { ... }
    override fun onError(e: Exception) { ... }
})

// Утилитный класс вместо extension-функции
object StringUtils {
    fun isEmailValid(email: String): Boolean {
        return Patterns.EMAIL_ADDRESS.matcher(email).matches()
    }
}

// JavaBean вместо data class
class User {
    private var name: String = ""
    
    fun getName(): String = name
    fun setName(name: String) { this.name = name }
}

Почему это может быть проблемой:

Использование Java-паттернов в Kotlin лишает вас ключевых преимуществ языка. Во-первых, вы теряете безопасность - immutable коллекции по умолчанию предотвращают случайные изменения данных. Во-вторых, код становится более многословным и трудным для чтения. В-третьих, отсутствие использования современных конструкций, таких как корутины и лямбды, ведёт к callback hell и усложнению асинхронного кода. Наконец, вы упускаете возможность использовать мощные возможности языка, такие как data class, которые автоматически генерируют полезные методы (equals, hashCode, toString, copy).

Что делать:

Используйте встроенные возможности Kotlin осознанно:

// ХОРОШО: Kotlin-идиомы
val items = listOf("a", "b", "c") // Immutable by default

// Простой обход
items.forEach(::println)

// С индексами
items.forEachIndexed { index, value ->
    println("$index: $value")
}

// Цепочки операций над коллекциями
val activeUsers = users
    .filter { it.isActive }
    .sortedBy { it.name }
    .take(10)

// Корутины вместо колбэков
lifecycleScope.launch {
    try {
        val user = api.loadUser()
        showProfile(user)
    } catch (e: Exception) {
        showError(e.message)
    }
}

// Extension functions вместо утилитных классов
fun String.isValidEmail() = Patterns.EMAIL_ADDRESS.matcher(this).matches()

if (email.isValidEmail()) { ... }

// Data class вместо JavaBean
data class User(val name: String, val email: String)

// Scope-функции для инициализации объектов
val intent = Intent(this, DetailActivity::class.java).apply {
    putExtra("id", userId)
    putExtra("name", userName)
    flags = Intent.FLAG_ACTIVITY_CLEAR_TOP
}

// Деструктуризация
val (name, email) = user
println("User $name has email $email")

// when вместо сложных if-else цепочек
when (view.visibility) {
    View.VISIBLE -> { ... }
    View.INVISIBLE -> { ... }
    View.GONE -> { ... }
}

Но помните: Идиомы - не догма. Иногда for (i in items.indices) или даже ручной цикл - самый читаемый способ, особенно если вам нужны сложные манипуляции с индексами. Главное - осознанный выбор, а не слепое следование правилам.

Дополнительно для Android:

  • Предпочитайте View Binding - он устраняет findViewById и делает код типобезопасным.
  • Замените object : OnClickListener {} на лямбды, если интерфейс имеет один метод (SAM conversion): button.setOnClickListener { /* ... */ }.
  • Используйте delegation: by lazy для отложенной инициализации, by viewModels() для ViewModel, и другие делегаты для типовых задач.
  • Предпочитайте immutable коллекции (listOf, mapOf) мутабельным, если не требуется изменение.
  • Для обработки жизненного цикла используйте Lifecycle-Aware компоненты и корутины с lifecycleScope.

5. Избыточная мутабельность

Сигнал: Повсеместное использование var и MutableList, особенно в публичном API класса или модуля. Мутабельные поля и коллекции, доступные для изменения извне, создают неявные зависимости и делают поведение кода непредсказуемым.

Пример проблемного кода (типично для Android):

// ПЛОХО: публичный изменяемый список
class UserViewModel : ViewModel() {
    var score = 0 // Может быть изменён извне без контроля
    val users = mutableListOf<User>() // Любой может добавить/удалить
    val userState = MutableStateFlow<User?>(null) // Любой может изменить состояние!

    fun loadUsers() {
        users.clear()
        users.addAll(api.getUsers()) // Что, если кто-то читает список в этот момент?
    }
}

// ПЛОХО: var в Android компонентах
class MainActivity : AppCompatActivity() {
    var currentUser: User? = null // Потеряется при повороте экрана
    
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        // currentUser будет null после конфигурационного изменения
    }
}

// ПЛОХО: публичный MutableLiveData
class ProfileViewModel : ViewModel() {
    val userData = MutableLiveData<User>() // UI может изменять данные напрямую
}

Почему это может быть проблемой:

Избыточная мутабельность превращает код в хрупкую конструкцию, где изменения в одной части программы могут неожиданно сломать другую. Во-первых, это создаёт условия для гонок состояний - когда несколько потоков или корутин одновременно изменяют одни и те же данные, что приводит к ConcurrentModificationException или потере обновлений. Во-вторых, нарушается инкапсуляция - внутреннее состояние класса становится частью публичного контракта, и любое его изменение требует пересмотра всего API. В-третьих, такой код сложно тестировать, так как тесты зависят от порядка выполнения и могут быть недетерминированными. Особенно критично это в Android, где конфигурационные изменения (поворот экрана) могут уничтожить и заново создать Activity, потеряв все мутабельные поля.

Что делать:

Следуйте принципу: «иммутабельность по умолчанию, мутабельность - по необходимости».

// ХОРОШО: сокрытие мутабельности через StateFlow
class UserViewModel : ViewModel() {
    // Приватное мутабельное состояние, публичный иммутабельный поток
    private val _score = MutableStateFlow(0)
    val score: StateFlow<Int> = _score.asStateFlow()
    
    private val _users = MutableStateFlow<List<User>>(emptyList())
    val users: StateFlow<List<User>> = _users.asStateFlow()

    fun updateScore(newScore: Int) {
        _score.value = newScore
        // Можно добавить бизнес-логику
    }

    fun loadUsers() {
        viewModelScope.launch {
            _users.value = api.getUsers() // Атомарное обновление всего списка
        }
    }
}

// ЛУЧШЕ: единое иммутабельное состояние через data class
data class UserScreenState(
    val users: List<User> = emptyList(),
    val isLoading: Boolean = false,
    val errorMessage: String? = null,
    val score: Int = 0
)

class UserViewModel : ViewModel() {
    private val _state = MutableStateFlow(UserScreenState())
    val state: StateFlow<UserScreenState> = _state.asStateFlow()
    
    fun updateScore(newScore: Int) {
        _state.update { currentState -> 
            currentState.copy(score = newScore)
        }
    }
}

// Для Android компонентов: сохраняйте состояние через Bundle или ViewModel
class MainActivity : AppCompatActivity() {
    private companion object {
        private const val KEY_CURRENT_USER = "current_user"
    }
    
    // Локальное состояние, которое сохраняем/восстанавливаем
    private var currentUser: User? = null
    
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        // Восстановление состояния
        currentUser = savedInstanceState?.getParcelable(KEY_CURRENT_USER)
    }
    
    override fun onSaveInstanceState(outState: Bundle) {
        super.onSaveInstanceState(outState)
        currentUser?.let { outState.putParcelable(KEY_CURRENT_USER, it) }
    }
}

// А ещё лучше: выносите состояние в ViewModel, которая переживает конфигурационные изменения
class UserViewModel : ViewModel() {
    // ViewModel автоматически сохраняет состояние при правильной настройке
    private val _user = MutableStateFlow<User?>(null)
    val user: StateFlow<User?> = _user.asStateFlow()
}

// Для коллекций - операторы, возвращающие новые экземпляры
val original = listOf(1, 2, 3)
val extended = original + 4 // [1, 2, 3, 4]
val filtered = original.filter { it > 1 } // [2, 3]

// Для data class используйте copy()
val user = User(name = "Alex", email = "a@example.com")
val updatedUser = user.copy(name = "Alexander")

Дополнительно для Android:

  • В ViewModel используйте StateFlow или LiveData с приватным мутабельным источником и публичным иммутабельным представлением.
  • В Activity и Fragment избегайте var для данных, которые должны сохраняться - используйте savedInstanceState, ViewModel или onSaveInstanceState с делегатами.
  • Для списков в RecyclerView используйте DiffUtil с иммутабельными данными - это эффективнее, чем мутировать список и вызывать notifyDataSetChanged().
  • Вместо мутабельных параметров в функциях возвращайте новые объекты - это делает код более предсказуемым и тестируемым.

Однако: В определённых сценариях мутабельность не только допустима, но и необходима. При обработке больших объёмов данных в фоне использование MutableList может быть значительно эффективнее из-за меньшего количества аллокаций. В алгоритмах с аккумуляторами (например, вычисление суммы) или в критичных к производительности участках кода (hot paths) мутабельность может быть оправдана. Главное, локализовать эту мутабельность и не допускать её распространения по всей кодовой базе.

6. Повторение того, что уже есть в stdlib

Сигнал: Ручные циклы for или while для поиска, фильтрации, агрегации или преобразования данных, хотя Kotlin предоставляет богатый набор функций в стандартной библиотеке. Разработчик "изобретает велосипед", не используя готовые, оптимизированные решения.

Пример проблемного кода (часто встречается в Android):

// ПЛОХО: ручной поиск первого активного пользователя
fun findFirstActiveUser(users: List<User>): User? {
    for (user in users) {
        if (user.isActive) {
            return user
        }
    }
    return null
}

// ПЛОХО: ручная фильтрация и преобразование
val activeUserNames = mutableListOf<String>()
for (user in users) {
    if (user.isActive) {
        activeUserNames.add(user.name.uppercase())
    }
}

// ПЛОХО: ручное вычисление суммы
var total = 0
for (order in orders) {
    total += order.amount
}

// ПЛОХО: подготовка данных для RecyclerView
val visibleItems = mutableListOf<Item>()
for (item in allItems) {
    if (item.isVisible && !item.isArchived) {
        visibleItems.add(item)
    }
}
adapter.submitList(visibleItems)

Почему это может быть проблемой:

Ручная реализация операций над коллекциями создаёт несколько проблем. Во-первых, такой код длиннее и содержит больше шаблонных конструкций, что увеличивает вероятность ошибок (например, забыть инициализировать аккумулятор или неправильно указать условие выхода). Во-вторых, код становится менее читаемым - вместо ясной декларативной операции ("найти первого активного пользователя") приходится анализировать императивную логику цикла. В-третьих, функции стандартной библиотеки часто оптимизированы под конкретные типы коллекций и могут работать быстрее. Наконец, ручные реализации затрудняют рефакторинг и повторное использование кода.

Что делать:

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

// ХОРОШО: поиск через stdlib
val firstActiveUser = users.find { it.isActive }
val lastActiveUser = users.findLast { it.isActive }

// ХОРОШО: фильтрация и преобразование в одну цепочку
val activeUserNames = users
    .filter { it.isActive }
    .map { it.name.uppercase() }

// Для больших коллекций используйте sequence для ленивых вычислений
val activeUserNames = users.asSequence()
    .filter { it.isActive }
    .map { it.name.uppercase() }
    .toList()

// ХОРОШО: агрегация
val total = orders.sumOf { it.amount }
val average = orders.sumOf { it.amount } / orders.size.toDouble()

// Другие полезные функции:
val userMap = users.associateBy { it.id }          // Map&lt;Id, User&gt;
val grouped = orders.groupBy { it.status }         // Map&lt;Status, List&lt;Order&gt;&gt;
val chunks = items.chunked(10)                    // Разбить на группы по 10
val runningTotal = amounts.runningFold(0) { acc, v -> acc + v } // Накопленная сумма для Kotlin 1.9
// // Для Kotlin < 1.9
// val runningTotal = mutableListOf<Int>().apply {
//    var sum = 0
//    for (amount in amounts) {
//        sum += amount
//        add(sum)
//    }
//}
val distinctCities = users.map { it.city }.distinct() // Уникальные значения

// Для Android: подготовка данных для RecyclerView
val visibleItems = allItems.filter { it.isVisible && !it.isArchived }
adapter.submitList(visibleItems)

// Преобразование списка моделей в список UI-моделей
val uiItems = apiItems.map { it.toUiModel() }

Но помните: Стандартная библиотека - не панацея. В некоторых случаях ручной цикл может быть уместнее:

// СЛОЖНЫЙ СЦЕНАРИЙ: несколько условий выхода + побочные эффекты
var lastActivePremiumIndex = -1
for ((index, user) in users.withIndex()) {
    logAnalytics(user) // Побочный эффект
    
    if (user.isActive) {
        lastActivePremiumIndex = index
        if (user.isPremium) {
            break // Несколько условий для выхода
        }
    }
}

Также будьте осторожны с forEach - его нельзя прервать с помощью break или continue, а return выйдет из всей функции, а не только из цикла. Это делает его неподходящим для сценариев с досрочным выходом.

Дополнительно для Android:

  • Используйте sumOf, maxByOrNull, minByOrNull вместо ручных аккумуляторов для вычислений.
  • Для преобразования списка моделей (например, из API) в UI-модели применяйте map с функцией-преобразователем.
  • Для группировки элементов (например, сообщений по дате или транзакций по категории) используйте groupBy.
  • При работе с большими списками в RecyclerView используйте DiffUtil вместе с иммутабельными данными - это эффективнее, чем обновлять весь список.
  • Функции вроде takeIf и takeUnless помогают избежать вложенных if при проверке условий.
  • Для обработки nullable значений используйте filterNotNull() вместо ручных проверок в цепочках преобразований.

7. Злоупотребление scope-функциями

Сигнал: Цепочки из 4–5 вложенных let, где каждая используется исключительно для проверки на null. Такой код выглядит как «лесенка» и трудно читается. Также тревожный признак - путаница между this и it внутри блоков.

Пример проблемного кода (часто встречается в Android):

// ПЛОХО: вложенные let только для null-checks ("ад вложенности")
user?.let { u ->
    u.profile?.let { p ->
        p.name?.let { n ->
            println(n.uppercase())
        }
    }
}

// ПЛОХО: смесь scope-функций и безопасных вызовов без ясной структуры
intent?.extras?.let { extras ->
    extras.getString("user_id")?.let { userId ->
        loadUser(userId)?.let { user ->
            showProfile(user)
        } ?: showError("User not found")
    } ?: showError("User ID missing")
} ?: showError("Intent malformed")

// ПЛОХО: путаница между this и it
user?.let {
    val name = user.name // Используем user вместо it - опасно, если user может быть null
    // ...
}

Почему это может быть проблемой:

Злоупотребление scope-функциями превращает код в труднопроходимые джунгли. Во-первых, глубоко вложенные блоки требуют отслеживания множества параметров (it, u, p и т.д.), что снижает читаемость. Во-вторых, отладка такого кода становится сложнее - точки останова приходится ставить в каждом вложенном блоке. В-третьих, такой код часто избыточен: безопасные вызовы (?.) и оператор Элвиса (?:) решают те же задачи короче и понятнее. Наконец, путаница между thisapply, run) и itlet, also) может привести к тонким багам.

Что делать:

Упрощайте цепочки с помощью безопасных вызовов и оператора Элвиса:

// ХОРОШО: просто и ясно через безопасные вызовы
println(user?.profile?.name?.uppercase())

// ХОРОШО: обработка всех null-случаев через Элвис
val displayName = user?.profile?.name?.uppercase() ?: "Anonymous"

// ХОРОШО: с дополнительной логикой при null
val userName = user?.profile?.name ?: run {
    logger.warn("User has no name, ID: ${user?.id}")
    "Guest"
}

// ХОРОШО: для сложных цепочек используйте временные переменные
val userId = intent?.extras?.getString("user_id")
val user = userId?.let { loadUser(it) }

when {
    userId == null -> showError("User ID missing")
    user == null -> showError("User not found")
    else -> showProfile(user)
}

Когда scope-функции действительно уместны:

// let - преобразование nullable значения с безопасным доступом
val userNames = users?.let { it.map { user -> user.name } } ?: emptyList()

// also - побочные эффекты (логирование, отправка аналитики)
userService.register(user)
    .also { log("User registered with ID: ${it.id}") }

// apply - конфигурация объектов (особенно полезно в Android)
val textView = TextView(context).apply {
    text = "Hello"
    textSize = 16f
    setTextColor(Color.BLACK)
    gravity = Gravity.CENTER
}

// run - выполнение блока кода с возвратом результата
val configuration = resources.run {
    // this - это resources, it - это результат Configuration()
    Configuration().also { newConfig ->
        newConfig.updateFrom(this.configuration) // this = resources
    }
}

// with - работа с объектом как контекстом (без цепочки .apply)
with(recyclerView) {
    layoutManager = LinearLayoutManager(context)
    adapter = myAdapter
    addItemDecoration(DividerItemDecoration(context, VERTICAL))
}

Правило трёх: Если у вас больше трёх вложенных scope-функций или цепочка начинает напоминать пирамиду - остановитесь и подумайте, нельзя ли упростить код. Scope-функции должны делать код чище, а не сложнее.

Дополнительно для Android:

  • Для инициализации View используйте apply - это чище, чем присваивание каждой property отдельно.
  • Для конфигурации объектов, которые не хранятся в переменной, используйте with - например, настройка RecyclerView в onViewCreated.
  • При работе с nullable Bundle или Intent используйте безопасные вызовы (intent?.extras?.getString(...)) вместо цепочек let.
  • Для обработки результатов корутин с возможностью null используйте let или безопасные вызовы: result?.let { process(it) }.
  • Избегайте вложенных let внутри колбэков - это делает код нечитаемым. Вместо этого используйте именованные функции или временные переменные.

8. Жёсткая связь с конкретными реализациями

Сигнал: Создание зависимостей внутри класса с помощью вызова конструктора. Класс сам отвечает за создание своих зависимостей, а не получает их извне. Такие классы знают слишком много о своём окружении и не могут работать в другой среде.

Пример проблемного кода (часто встречается в Android):

// ПЛОХО: жёсткая связь с реализацией
class OrderService {
    private val db = MySQLDatabase() // Как подменить в тесте?
    private val api = Retrofit.Builder()
        .baseUrl("https://api.example.com") // URL захардкожен
        .build()
        .create(ApiService::class.java)
}

// ПЛОХО: использование Context напрямую без абстракции
class UserManager(private val context: Context) {
    fun saveUser(user: User) {
        val prefs = PreferenceManager.getDefaultSharedPreferences(context)
        prefs.edit().putString("user", user.toJson()).apply()
    }
    // Тестировать невозможно - нужен реальный Context
}

// ПЛОХО: ручное создание ViewModel с зависимостями
class MainActivity : AppCompatActivity() {
    private val viewModel = MainViewModel(
        repository = OrderRepository(RetrofitApi()) // Зависимость создана внутри
    )
    // ViewModel будет пересоздаваться при каждом повороте экрана!
}

// ПЛОХО: использование синглтона не решает проблему
class AnalyticsService {
    fun trackEvent(event: String) {
        Firebase.analytics.logEvent(event, null) // Жёсткая связь с Firebase
    }
}

Почему это может быть проблемой:

Жёсткая связь с конкретными реализациями превращает код в хрупкую систему, которую сложно менять и тестировать. Во-первых, такой код невозможно адекватно протестировать - вы не можете подменить реальную базу данных, сетевой API или сторонние сервисы на тестовые заглушки. Во-вторых, это нарушает принцип инверсии зависимостей (DIP), который гласит, что модули высокого уровня не должны зависеть от модулей низкого уровня, а оба должны зависеть от абстракций. В-третьих, код становится сложно расширять: замена библиотеки (например, переход с Retrofit на Ktor) потребует изменений во всех классах, где она используется. В-четвёртых, управление жизненным циклом зависимостей становится сложным, особенно в Android, где разные объекты должны жить разное время (Application, Activity, Fragment, ViewModel).

Что делать:

Внедряйте зависимости через конструктор и зависьте от абстракций, а не от конкретных реализаций:

// ХОРОШО: зависимость через конструктор с интерфейсом
interface Database {
    fun save(order: Order)
}

interface ApiService {
    suspend fun loadOrders(): List<Order>
}

class OrderService(
    private val db: Database,
    private val api: ApiService
) {
    suspend fun processOrders() {
        val orders = api.loadOrders()
        orders.forEach { db.save(it) }
    }
}

// ХОРОШО: полная абстракция над SharedPreferences
interface UserPreferences {
    fun saveUser(user: User)
    fun getUser(): User?
    fun clear()
}

class SharedPrefsUserPreferences(
    private val context: Context
) : UserPreferences {
    override fun saveUser(user: User) {
        val prefs = PreferenceManager.getDefaultSharedPreferences(context)
        prefs.edit().putString("user", user.toJson()).apply()
    }
    
    override fun getUser(): User? = // ...
    
    override fun clear() = // ...
}

// ХОРОШО: использование DI фреймворка (Hilt)
@HiltViewModel
class MainViewModel @Inject constructor(
    private val orderService: OrderService,
    private val userPreferences: UserPreferences
) : ViewModel() {
    // ViewModel корректно переживает поворот экрана
    // Зависимости управляются Hilt
}

// Альтернатива: ручное внедрение (для простых случаев)
class MainActivity : AppCompatActivity() {
    private lateinit var viewModel: MainViewModel
    
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
		// Создаём Retrofit один раз
		val retrofit = Retrofit.Builder()
			.baseUrl("https://api.example.com/")
			.addConverterFactory(GsonConverterFactory.create())
			.build()      
        // Создаём зависимости один раз
        val api: ApiService = retrofit.create(ApiService::class.java)
        val db: Database = AppDatabase.getInstance(applicationContext)
        val orderService = OrderService(db, api)
        val userPreferences = SharedPrefsUserPreferences(this)
        
        // Создаём ViewModel через фабрику
        viewModel = ViewModelProvider(
            this,
            ViewModelFactory(orderService, userPreferences)
        )[MainViewModel::class.java]
    }
}

// Фабрика для ViewModel с зависимостями
class ViewModelFactory(
    private val orderService: OrderService,
    private val userPreferences: UserPreferences
) : ViewModelProvider.Factory {
    override fun <T : ViewModel> create(modelClass: Class<T>): T {
        if (modelClass.isAssignableFrom(MainViewModel::class.java)) {
            @Suppress("UNCHECKED_CAST")
            return MainViewModel(orderService, userPreferences) as T
        }
        throw IllegalArgumentException("Unknown ViewModel class")
    }
}

Как это помогает тестированию:

// Можем создать тестовые реализации
class FakeDatabase : Database {
    private val items = mutableListOf<Order>()
    
    override fun save(order: Order) {
        items.add(order)
    }
    
    fun getAll() = items.toList()
}

class FakeApiService : ApiService {
    var shouldFail = false
    
    override suspend fun loadOrders(): List<Order> {
        if (shouldFail) throw IOException("Network error")
        return listOf(Order("test-1"), Order("test-2"))
    }
}

// В тестах легко подменяем зависимости
@Test
fun testOrderService() {
    val fakeDb = FakeDatabase()
    val fakeApi = FakeApiService()
    val service = OrderService(fakeDb, fakeApi)
    
    // Тестируем без реальных сетевых запросов и БД
}

Контекст: В небольших проектах, скриптах или прототипах сложные системы внедрения зависимостей могут быть излишними. Однако как только появляется необходимость в автоматических тестах, работе в команде или поддержке нескольких конфигураций (debug/production, разные API endpoints), пора задуматься об абстракциях и внедрении зависимостей. Начинать можно с простого внедрения через конструктор, а по мере роста проекта переходить на DI-фреймворки.

Дополнительно для Android:

  • Используйте Hilt - официальный DI-фреймворк от Google, который глубоко интегрирован с Android и упрощает работу с жизненным циклом компонентов.
  • Для более простых проектов рассмотрите Koin - легковесную альтернативу, которая использует чистый Kotlin.
  • Всегда используйте by viewModels() или by activityViewModels() вместо ручного создания ViewModel. Если ViewModel имеет параметры в конструкторе, создайте кастомную фабрику.
  • Абстрагируйте слои данных: Repository должен зависеть от интерфейсов источников данных (локальных и удалённых), а не от конкретных реализаций Room или Retrofit.
  • Рассмотрите использование архитектурных подходов (Clean Architecture, MVVM) - они естественным образом приводят к разделению ответственностей и внедрению зависимостей.

9. Комментарии, дублирующие код

Сигнал: Комментарии, которые просто перефразируют то, что и так очевидно из кода. Такие комментарии не добавляют ценности, но создают шум и риск устаревания. Они часто возникают, когда разработчик не уверен в читаемости своего кода и пытается "подстраховаться".

Пример проблемного кода (часто встречается в Android):

// ПЛОХО: комментарий дублирует название метода
// Загружает пользователя из базы данных
fun loadUser(userId: String): User? {
    return userDao.findById(userId)
}

// ПЛОХО: комментарий повторяет код
// Возвращает сумму заказов
val total = orders.sumOf { it.amount }

// ПЛОХО: "мёртвый" комментарий после рефакторинга
// Проверяем, активен ли пользователь (раньше было isActive(), теперь isVerified())
if (user.isVerified) { ... }

// ПЛОХО: комментарий вместо понятного имени
// a - количество дней, b - ставка
fun calculate(a: Int, b: Double) = a * b

// ПЛОХО: бесполезный TODO без контекста
// TODO: исправить
fun problematicFunction() { ... }

Почему это может быть проблемой:

Дублирующие комментарии создают больше проблем, чем пользы. Во-первых, они увеличивают объём кода, который нужно читать и поддерживать, при этом не неся полезной информации. Во-вторых, такие комментарии быстро устаревают - при изменении кода разработчики часто забывают обновить комментарий, и он начинает вводить в заблуждение. В-третьих, они могут маскировать более серьёзные проблемы, такие как плохие имена переменных или функций - вместо того чтобы дать сущности понятное имя, разработчик пишет поясняющий комментарий. Наконец, они создают "шум", который мешает найти действительно важные комментарии, объясняющие сложные решения или ограничения.

Что делать:

Пишите самодокументирующийся код и комментируйте только то, что невозможно выразить в коде:

// ХОРОШО: понятное имя вместо комментария
fun calculateRentalPrice(days: Int, dailyRate: Double): Double = days * dailyRate

// ХОРОШО: комментарий объясняет "почему" выбор алгоритма
// Используем устойчивую сортировку слиянием вместо быстрой сортировки,
// потому что данные частично отсортированы из кэша
fun stableSort(items: List<Item>): List<Item> = mergeSort(items)

// ХОРОШО: workaround для бага в Android с контекстом
// Обход бага #12345 в WebView Android 10: локальные файлы требуют file:// префикс
webView.loadUrl("file:///android_asset/help.html")

// ХОРОШО: сложное бизнес-правило вынесено в константы с понятными именами
private const val LOYALTY_DISCOUNT_RATE = 0.15
private const val LOYALTY_DISCOUNT_CUTOFF_YEAR = 2023

val discount = if (user.isVip && user.registrationDate.year < LOYALTY_DISCOUNT_CUTOFF_YEAR) 
    LOYALTY_DISCOUNT_RATE 
else 0.0

// ХОРОШО: объяснение, почему код выглядит странно, но его нельзя изменить
// Не удаляем из-за обратной совместимости с API v1
// Клиенты старше 2 лет всё ещё используют этот endpoint
@Deprecated("Используйте /api/v2/orders", level = DeprecationLevel.WARNING)
fun getLegacyOrders(): List<Order> { ... }

// ХОРОШО: TODO с конкретным контекстом и ссылкой на задачу
// TODO: заменить на View Binding, когда minSdk станет 21 (задача #PROJ-456)
val button = findViewById<Button>(R.id.submit)

// ХОРОШО: FIXME с планом исправления
// FIXME: временное решение до выхода библиотеки X версии 2.4.0
// Баг: утечка памяти при быстром скролле в RecyclerView
// Ожидаем фикс в релизе 2.4.0 (15.12.2023), затем удалить этот блок
recyclerView.addOnScrollListener(object : RecyclerView.OnScrollListener() {
    override fun onScrollStateChanged(recyclerView: RecyclerView, newState: Int) {
        // Временный workaround
    }
})

Правило: Если вы можете выразить мысль через имя переменной, функции или тип - делайте это. Комментарии нужны только для объяснения неочевидных решений, ограничений платформы, алгоритмов или бизнес-правил.

Когда комментировать обязательно:

  • Обходы багов (workarounds) с ссылкой на issue tracker
  • Сложные алгоритмы с объяснением выбора
  • Бизнес-правила, которые неочевидны из кода
  • Код, который нельзя изменить из-за обратной совместимости
  • Временные решения с планом их удаления

Дополнительно для Android:

  • Используйте KDoc (/** */) для публичного API библиотек и модулей - это документация, а не комментарии к реализации.
  • Не комментируйте очевидные вещи вроде // Инициализируем RecyclerView или // Устанавливаем слушатель клика.
  • Если вы оставляете // TODO или // FIXME, обязательно укажите причину, контекст и, если возможно, ссылку на задачу в трекере.
  • Регулярно проводите ревизию комментариев и удаляйте устаревшие - они вводят в заблуждение и мешают работе новых разработчиков.
  • При рефакторинге обращайте внимание на комментарии - если они становятся неактуальными, удалите или обновите их.

10. Небрежная обработка ошибок

Сигнал: Использование оператора !!, игнорирование возможных исключений или отсутствие механизма восстановления после ошибки. Такой код работает «в идеальных условиях», но падает в реальном мире, где данные могут быть неполными, сеть нестабильна, а пользователи совершают ошибки.

Пример проблемного кода (часто встречается в Android):

// ПЛОХО: NPE в бою при отсутствии extra
val userId = intent.getStringExtra("user_id")!!.toInt()

// ПЛОХО: JSONException при отсутствии ключа без обработки
val name = json.getString("name") // Упадёт, если ключа "name" нет

// ПЛОХО: NPE при изменении layout или неправильном ID
val button = findViewById<Button>(R.id.submit_button)!!

// ПЛОХО: NumberFormatException на некорректных данных без обработки
val amount = inputField.text.toString().toInt()

// ПЛОХО: исключение в корутине без обработки
viewModelScope.launch {
    val user = api.getUser(userId) // Может выбросить IOException
    updateUI(user) // Не выполнится при ошибке
}

Почему это может быть проблемой:

Небрежная обработка ошибок превращает приложение в хрупкую конструкцию, которая ломается при первом же неожиданном условии. Во-первых, такие сбои происходят непредсказуемо: пользователь может видеть белый экран, перезапуск приложения или просто зависший интерфейс без объяснения причин. Во-вторых, отсутствие корректной обработки ошибок лишает возможности восстановления: приложение не может показать понятное сообщение об ошибке, предложить альтернативное действие или сохранить прогресс пользователя. В-третьих, затрудняется диагностика проблем: краш-репорты в Crashlytics или Firebase Crashlytics часто не содержат необходимого контекста (какие данные вызвали ошибку, какое было состояние приложения). Наконец, плохая обработка ошибок негативно влияет на пользовательский опыт и рейтинг приложения в магазине.

Что делать:

Выбирайте стратегию обработки ошибок в зависимости от контекста и слоя приложения:

// ХОРОШО: безопасные аналоги и явная обработка
val userId = intent.getStringExtra("user_id")?.toIntOrNull()
    ?: run {
        logError("Missing or invalid user_id in intent: $intent")
        showErrorDialog("Не удалось определить пользователя")
        return@activity // или navigateToLoginScreen()
    }

// ХОРОШО: исключения с понятным сообщением и контекстом
val user = userRepository.findById(id) 
    ?: throw IllegalArgumentException(
        "User $id not found. " +
        "Activity: ${javaClass.simpleName}, " +
        "Intent: ${intent?.action}. " +
        "Check user migration scripts."
    )

// ХОРОШО: Result для локальных операций (не используйте в public API на JVM!)
fun parseAmount(input: String): Result<Int> = runCatching {
    require(input.isNotBlank()) { "Input cannot be empty" }
    val value = input.toInt()
    require(value > 0) { "Amount must be positive" }
    value
}

// Использование Result
parseAmount(userInput).fold(
    onSuccess = { amount -> processPayment(amount) },
    onFailure = { error -> showValidationError(error.message ?: "Invalid amount") }
)

// ХОРОШО: sealed class - явная модель ошибок для сложных сценариев
sealed class UserLoadResult {
    data class Success(val user: User) : UserLoadResult()
    data class ValidationError(val field: String, val message: String) : UserLoadResult()
    data class ServerError(val code: Int, val message: String) : UserLoadResult()
    object NetworkError : UserLoadResult()
    object NotFound : UserLoadResult()
}

fun loadUser(id: String): UserLoadResult {
    return when {
        id.isBlank() -> UserLoadResult.ValidationError("id", "User ID cannot be empty")
        else -> {
            try {
                val user = api.getUser(id)
                UserLoadResult.Success(user)
            } catch (e: IOException) {
                UserLoadResult.NetworkError
            } catch (e: HttpException) {
                when (e.code()) {
                    404 -> UserLoadResult.NotFound
                    else -> UserLoadResult.ServerError(e.code(), e.message ?: "Unknown error")
                }
            }
        }
    }
}

// В ViewModel: обработка ошибок в корутинах с преобразованием в UI-состояние
fun loadUserProfile(userId: String) = viewModelScope.launch {
    _uiState.value = ProfileState.Loading
    
    when (val result = repository.loadUser(userId)) {
        is UserLoadResult.Success -> {
            _uiState.value = ProfileState.Loaded(result.user)
        }
        is UserLoadResult.ValidationError -> {
            _uiState.value = ProfileState.Error("Проверьте введённые данные: ${result.message}")
        }
        UserLoadResult.NetworkError -> {
            _uiState.value = ProfileState.Error("Нет соединения с интернетом. Проверьте подключение.")
        }
        UserLoadResult.NotFound -> {
            _uiState.value = ProfileState.Error("Пользователь не найден")
        }
        is UserLoadResult.ServerError -> {
            _uiState.value = ProfileState.Error("Ошибка сервера. Попробуйте позже.")
            logServerError(result.code, result.message)
        }
    }
}

// Для Android View: безопасная работа с findViewById
val button = findViewById<Button>(R.id.submit_button)
if (button != null) {
    button.setOnClickListener { processForm() }
} else {
    Log.w(TAG, "Submit button not found. Layout might have changed.")
    // Можно показать Snackbar или работать без этой кнопки
}

// Или через View Binding (предпочтительно)
// View Binding гарантирует non-null для view, присутствующих в layout
binding.submitButton?.setOnClickListener { processForm() }

Дополнительно для Android:

  • Избегайте оператора !! - он превращает потенциально проверяемую на этапе компиляции проблему в runtime-краш. Вместо этого используйте безопасные вызовы (?.) с явной обработкой.
  • Используйте безопасные методы преобразования: toIntOrNull(), toDoubleOrNull(), toBooleanStrictOrNull() вместо выбрасывающих исключения аналогов.
  • Всегда проверяйте наличие extras в Intent перед использованием. Используйте requireNotNull с понятным сообщением или безопасные аналоги.
  • Для работы с JSON используйте библиотеки с null-safety (Moshi с кодогенерацией, kotlinx.serialization) или явно обрабатывайте исключения при использовании Gson/Jackson.
  • В корутинах используйте try/catch для обработки исключений или SupervisorJob для изоляции падений одних корутин от других.
  • Для критически важных операций (например, сохранение данных) используйте транзакции и механизмы отката при ошибках.
  • Логируйте ошибки с достаточным контекстом (ID пользователя, состояние приложения, версия ОС), но без персональных данных.

Выбор стратегии зависит от контекста:

  • Внутри закрытого модуля или приватного метода - исключения могут быть уместны, если они документированы и обрабатываются на верхнем уровне.
  • На границах системы (парсинг ввода, работа с Intent, сетевые запросы) - предпочитайте возвращать Result или sealed class с явными типами ошибок.
  • В UI-слое (ViewModel, Activity, Fragment) - всегда преобразуйте технические ошибки в понятные состояния интерфейса (сообщения об ошибках, пустые состояния, кнопки повтора).
  • Для фатальных ошибок (неконсистентное состояние, отсутствие обязательных ресурсов) - допустимо выбросить исключение с понятным сообщением, что приведёт к завершению приложения, но с информативным логом и, если возможно, отправкой отчёта об ошибке.

Вместо заключения: пишите осознанно, а не «по правилам»

Мы прошли через десять провокационных признаков, но суть не в том, чтобы запомнить список запретов, а в развитии в себе вкуса к хорошему коду и умение делать осознанный выбор. Рефакторинг это очень важный навык для программиста. Сначала сделайте рабочее решение, потом улучшайте его там, где это действительно важно. Хороший код - не тот, что соответствует всем пунктам из учебника. Хороший код - это код, который решает задачу пользователя и при этом не становится головной болью для вас и вашей команды завтра, через неделю или через год. Это код, который можно понять, изменить и протестировать в вашем конкретном контексте.

Эта статья - не свод законов, а приглашение к размышлению и обмену опытом. Расскажите, с какими «антипаттернами» вам приходилось сталкиваться на практике? Когда в вашей карьере «плохой» код оказывался единственно верным решением?


Новости [1] [2] [3]... Android/ iOS/ J2ME[1] [2] [3])/ Android/ Архив/ Карьера

Яндекс.Метрика
MobiLab.ru © 2005-2026
При использовании материалов сайта ссылка на www.mobilab.ru обязательна