Чистый код. Часть 2

Часть 1
Часть 2 (вы здесь)
Часть 3

Привет! Продолжаем цикл постов про чистый код по мотивам видеолекций Дяди Боба, первая часть тут. В этом посте поговорим про структуру функций.

Количество аргументов функции

Количество аргументов в функции должно быть как можно меньшим. В идеале количество аргументов функции должно быть равно нулю.

fun isSet(): Boolean {
    TODO()
}

Чем плохо большое количество аргументов? Тем, что их сложно запомнить, приходится всегда возвращаться и перечитывать функцию, плюс сложно запомнить их порядок. Например, у нас есть функция findAnyOf(). У неё три аргумента:

val charSequence: CharSequence = "adscasvvds"
charSequence.findAnyOf(setOf("sdc"), 0, true)

Если бы нам IDE не подсказывала (а она подсказывает не в каждом месте), то нам сложно было бы запомнить, в какой последовательности какие параметры должны передаваться и что эти параметры означают. 

Что с Kotlin

Для Kotlin это менее применимо, потому что в Kotlin есть именованные параметры. Но, например, в случае вызова Kotlin из функции Java мы так сделать не можем, поэтому желательно стремиться к уменьшению количества аргументов функции, потому что это сложно читать и понимать.

charSequence.findAnyOf(strings = setOf("sdc"), startIndex = 0, ignoreCase = true)

Еще одно наблюдение. Если функция принимает в себя несколько аргументов, то есть вероятность, что они сильно связаны и представляют из себя класс. Например, есть функция auth(), в нее передаются две переменные — amount и currency.

fun auth(amount: Double, currency: Currency) {
    TODO()
}

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

Передача булевых аргументов

Чаще всего передача булевых аргументов говорит о том, что функция делает сразу две вещи, а это значит, что она нарушает принцип единственности ответственности. Вместо этого у нас должно быть две функции. Например, есть функция createPayment(). Она создает платеж по токену, если isByToken = true, или создает платеж по номеру карту, если isByToken = false.

fun createPayment(isByToken: Boolean) {
    if(isByToken) {
        createTokenPayment()
    } else {
        createCardPayment()
    }
}

Здесь может быть использован полиморфизм.

class TokenPaymentFlow : PaymentFlow {
    fun createPayment() {
        TODO("Create with token")
    }
}

class CardPaymentFlow : PaymentFlow {
    fun createPayment() {
        TODO("Create with pan")
    }
}

Не используйте возвращаемые аргументы

Это очень неявное поведение, читатели этого не ожидают. У нас есть функция auth, которая внутри себя отправляет запрос на авторизацию в шлюз и в этот же платеж устанавливает rcCode, который вернулся от сервиса.

fun auth(payment: Payment, gateway: GatewayService) {
    val rcCode = gateway.sendAuth(payment)
    payment.rcCode = rcCode
}

Такая функция сложна к пониманию тем, что читатель, не знает как устроена функция внутри и не догадывается о том, что внутри в payment устанавливается значение. То есть у нас параметр payment является возвращаемым. Желательно возвращать здесь этот rcCode прямо в возвращаемых параметрах, а не в аргументах. Вот такой код тоже плох. 

fun addError(errors: MutableList<Warning>) {
    errors.add(Warning(WarningType.LEGACY_CODE_DEPENDENCY, "", ""))
}

Есть функция addError, в которую передается MutableList из предупреждений. Дальше этот код добавляет внутри себя в MutableList конкретный warning. Errorsявляется тут возвращаемым аргументом.

Не используйте nullable-аргументы

Nullable-аргументы и передача Nullable-аргументов — тоже плохая практика, потому что чаще всего мы внутри функции проверяем “if null” и, аналогично булевым, ваша функция начинает делать две вещи, то есть она начинает делать что-то одно для amount == null и что-то другое для не amount != null.

fun auth(amount: Int?) {
    if(amount == null) {
        checkCard()
    } else {
        authWithAmount(amount)
    }
}

Эту функцию надо разделить на две. 

Плюс большое количество проверок на null в коде является признаком защитного программирования. Это один из плохих запахов кода (если это непубличный API или библиотека), которые описаны в книге «Рефакторинг».

Stepdown rule (правило понижения) 

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

Такого же хотелось добиться в программировании, чтобы изначально получать самую важную информацию о том, что делает этот код. А к деталям переходить только в том случае, если нам нужно узнать эти детали. Для этого применяется правило stepdown rule — оно говорит о том, в какой последовательности надо размещать функции внутри класса.

У нас уже есть конвенции, которые желательно не нарушать. Например, в Java идут сначала приватные переменные, а правило stepdown rule говорит о том, что самое важное должно быть сверху. Приватные переменные, казалось бы, это уже детали реализации. Их нужно было бы запихнуть в самый конец, но, чтобы всех не обескураживать, мы соответствуем Java-конвенции и оставляем приватные переменные сверху. Дальше мы можем применять тот самый stepdown rule.

class StableDependenciesPrincipleService(
    /** В java уже есть конвенция по тому что переменные класса вверху - не нарушаем чтобы не обескураживать */
    private val moduleInfoService: ModuleInfoDao,
    private val warningDescriptionProperties: WarningDescriptionProperties,
) : WarningsService {

    /** Сначала идет публичная функция. В ней как можно меньше деталей */
    override fun getWarnings(moduleName: String): List<Warning> {
        val errors = mutableSetOf<Warning>()
        val analyzeComponentInstabilityFactor = getInstabilityFactor(moduleName)
        for (dependentToComponent in getDependenciesTo(moduleName)) {
            val error = findStabilityError(dependentToComponent, analyzeComponentInstabilityFactor)
            error.map { errors.add(it) }
        }
        return errors.toList()
    }

        /** Дальше идет первая вызывающая ее функция. Она является дочерней для getWarnings (выделил табуляцией) */
        private fun getInstabilityFactor(moduleName: String): Double {
            val dependenciesCount = getDependenciesCount(moduleName)
            return if (dependenciesCount.directionIn + dependenciesCount.directionOut == 0) {
                0.0
            } else {
                dependenciesCount.directionOut.toDouble() / (dependenciesCount.directionIn + dependenciesCount.directionOut)
            }
        }
        
            private fun getDependenciesCount(moduleName: String): DependenciesCount {
                val moduleInfo = moduleInfoService.getModuleByName(moduleName)
                return DependenciesCount(moduleInfo.connectionsFrom.count(), moduleInfo.connectionsTo.count())
            }

        /**
         * Смысл этой табуляции в том, что было бы удобно свернуть все по умолчанию и разворачивать каждую публичную
         * функцию, чтобы иметь как можно меньше деталей пока ты сам этого не захотел
         */
        private fun getDependenciesTo(moduleName: String): Set<String> {
            return moduleInfoService.getModuleByName(moduleName).connectionsTo.map { it.module.name }.toSet()
        }

        private fun findStabilityError(dependentToComponent: String, analyzeComponentInstabilityFactor: Double) : Optional<Warning> {
            val dependentToComponentInstabilityFactor = getInstabilityFactor(dependentToComponent)
            return if (dependentToComponentInstabilityFactor > analyzeComponentInstabilityFactor) {
                Optional.of(
                    Warning(
                        type = WarningType.STABLE_DEPENDENCY_VIOLATION,
                        message = "Более стабильный модуль $dependentToComponent (I = " +
                            " ${Precision.round(dependentToComponentInstabilityFactor, 3)}) зависит от менее стабильного " +
                            " (I = ${Precision.round(analyzeComponentInstabilityFactor, 3)})",
                        descriptionLink = warningDescriptionProperties.stableDependencyPrincipleLink,
                    )
                )
            } else {
                Optional.empty()
            }
        }

    /** Дядя боб делает следующую публичную функция после всех приватных для первой функции */
    fun otherPublicFunction() {
        TODO()
    }
}

Например, есть класс StableDependenciesService который анализирует ошибки по компонентам системы (подробнее об этом в SOLID для компонентов). В нем сначала идет публичная функция getWarnings(). Она не перегружена деталями. Если ее читать, то мы видим, что идет сравнение нестабильности анализируемого модуля и модулей, которые от него зависят (есть ли неправильные зависимости стабильных компонентов от нестабильных). Самые важные детали у нас находятся как можно выше. Это позволяет нам понимать, что делает функция, не углубляясь в детали (не зная, например, алгоритма расчета нестабильности компонента).

Дальше идет функция getInstabilityFactor(), которая вызывается из публичной функции getWarnings(). Она находится ниже и в ней больше деталей реализации. Именно она содержит алгоритм расчета нестабильности модуля.

Я здесь специально отделил табуляциями функции, чтобы было понятно, какая функция в скоупе какой функции лежит (например, функция getDependenciesCount() лежит только в скоупе функции getInstabilityFactor()) Stepdown rule говорит нам о том, что мы размещаем функции, исходя из их важности и количества деталей. 

Есть случаи, когда в классе несколько публичных функций. Тогда возникает вопрос: что делать? Какую функцию за какой размещать? Чаще всего, по конвенции, публичные функции должны быть на самом верху, а все приватные функции — ниже. Дядя Боб именно это правило нарушает. Oн пишет одну публичную функцию и после этого все приватные, которые находятся в ее скоупе. Это правило необязательное, но он делает так.

Размещайте рядом концептуально родственные функции (из книги)

Есть класс Assert, и в нем есть две функции: assertFalse() и assertTrue().

class Assert {
    fun assertFalse() {
        TODO()
    }
    
    fun assertTrue() {
        TODO()
    }
    
    fun assertEquals() {
        TODO()
    }
}

Эти функции концептуально одинаковые, и желательно, чтобы они были рядом. А функция assertEquals() концептуально отличается от assertFalse() или assertTrue(), поэтому не должна стоять между assertTrue() и assertFalse(). 

Еще один пример.

fun PayResponseDto.toError() {
    TODO()
}
fun PayResponseDto.toSuccess() {
    TODO()
}

fun RefundResponseDto.toError() {
    TODO()
}
fun RefundResponseDto.toSuccess() {
    TODO()
}

Есть концептуально одинаковые функции расширения классовPayResponseDto и RefundresponseDto. Их желательно размещать группируя по классам.

Switch case — не ООП

Switch case чаще всего противоречит объектно-ориентированному программированию, и в большинстве случаев switch case использовать нельзя. В следующей статье мы поговорим о том, где можно использовать switch case, но в случае с ООП switch case вредит. 

Одно из самых больших преимуществ объектно-ориентированного программирования — полиморфизм и инверсия зависимостей, в результате которой модули могут независимо разрабатываться и деплоиться. Switch case нарушает независимость модулей. 

Рассмотрим switch case в функции openGate().

import vehicle.track.Track
import vehicle.car.Car
import vehicle.bus.Bus

fun openGate(vehicle: Any) {
    when(vehicle) {
        /** Source code dependency (import track) */
        is Track -> {
            /** Flow of control dependency */
            vehicle.getCargoDocuments()
            TODO("Проверить документы на груз и взять пошлину")
        }
        is Car -> {
            vehicle.getPassengerVisas()
            TODO("Проверить визы всех пассажиров")
        }
        is Bus -> {
            vehicle.getPublicTransportLicense()
            TODO("Проверить лицензию общественного транспорта")
        }
    }
}

Это функция, которая контролирует открытие пропускного пункта для конкретного транспортного средства. Мы рассматриваем в switch case три случая: когда транспортное средство — грузовик, когда — автомобиль и когда — автобус. Таким образом, мы делаем зависимость flow of control (управления) и source code зависимость (зависимость от исходного кода) в одну сторону. Получается, что вызывающий код является клубком зависимостей: внутри себя он делает import каждого модуля с транспортным средством. 

Что можно было бы сделать с методом openGate(), чтобы он не включал в себя все зависимости? В первую очередь можно было бы инвертировать зависимость и сделать общий интерфейс Vehicle, и убрать все эти поведения (получить документы на груз, получить визы всех пассажиров или получить лицензию на публичные перевозки) в реализацию интерфейсов.

fun openGateInvertedDependency(vehicle: Vehicle) {
    /** Inverted source code dependency (import track) */
    vehicle.getDocuments()
}

То есть у нас бы был интерфейс Vehicle, у него бы была реализация Truck, Car, Bus, внутри которых вызывалось бы разное поведение. Таким образом мы бы инвертировали зависимость, и все модули, в которых разрабатываются Truck, Car и Bus, были бы независимыми от вызывающего класса. Таким образом мы бы могли пользоваться полиморфизмом (возможностью менять конкретную реализацию, не трогая при этом вызывающий код). 

Второй вариант изменения функции openGate() — вынос фабрик в место, где они не будут причинять такого количества боли, как тут.

fun openGateFactories(vehicleDocumentFactory: VehicleDocumentFactory) {
    vehicleDocumentFactory.getDocuments()
}

class VehicleDocumentFactory(private val vehicle: Any) {
    fun getDocuments() {
        when(vehicle) {
            is Car -> TODO()
            else -> TODO()
        }
    }
}

То есть не придется каждый раз перекомпилировать весь проект при изменении в одном пакете. Положить эту фабрику можно в место по типу main.

Side-эффекты

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

В функциональном программировании любая функция — это математическая функция. То есть каждый раз, когда вы вызываете одну и ту же функцию, она возвращает один и тот же результат. Другими словами, в функциональном программировании у функции нет side-эффектов. Это считается одним из главных преимуществ функционального программирования. 

Что такое side-эффект? Это изменение которое меняет состояние системы. Рассмотрим метод doSomeStuff(). Он устанавливает переменную someVar, которой пользуется другой метод. Такая функция называется функцией с side-эффектом.

class SomeClass(var someVar: String) {

    fun doSomeStuff() {
        someVar = "blah-blah"
    }

    fun set(varValue: String) {
        someVar = varValue
    }
}

Side-эффекты очень сложно дебажить и самая большая их проблема — проблема временного связывания. Рассмотрим класс File, у которого есть две функции: open() и close().

class File {
    /** Метод open оставляет систему в измененном состоянии */
    fun open() {
        TODO()
    }

    /** Временное связывание - это когда после open обязательно надо вызвать close в правильном порядке */
    fun close() {
        TODO()
    }
}

Вызывая метод open, нам обязательно нужно вызвать метод close. Это называется временным связыванием. Ошибки, связанные с временным связыванием очень сложно находить.

С классом File понятно, что нужно вызвать close() после open() ( потому что часто работаем с похожими классами). Но часто бывают такие случаи, когда вы не знаете о том, что нужно вызвать другую функцию, чтобы оставить систему в правильном состоянии. С такими проблемами временного связывания можно бороться, написав функцию-обертку. Эта техника называется “pass a block”. То есть функция, которая вызывает open() и close() в одной и той же функции, при этом внутри выполняется какое-то действие. 

fun makeCommand(file: File, fileCommand: FileCommand) {
    file.open()
    fileCommand.process(file)
    file.close()
}

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

Чтобы код становился более ожидаемым и side-эффекты не приносили такой боли, существует практика, называемая CQS — command query separation. Она говорит о том, что каждая функция должна быть либо командой, либо запросом. Функции-команды ничего не должны возвращать, а только менять состояние системы. А функции-запросы не должны менять состояние системы, а только возвращать результат. Так мы делаем для читателя более объективный и более понятный код.

В результате вызова функции, если функция ничего не возвращает, мы понимаем, что она имеет side-эффект. Если она что-то возвращает — значит, side-эффекта тут нет. Например, функция setId(), которая возвращает нам boolean.

class SomeClass(var id: Int, var a: Int) {
    /** Неожиданно */
    fun setId(id: Int): Boolean {
        this.id = id
        return true
    }

    /** Неожиданно */
    fun getId(): Int {
        a = 0
        return id
    }
}

Для нас неожиданно, что она возвращает boolean, мы не понимаем — она либо что-то устанавливает, либо что-то возвращает. По setId() очевидно, что она что-то устанавливает. Но, например, функция getId(), является для нас очень неожиданной, так как меняет переменную a, которой кто-то еще может пользоваться (то есть имеет side-эффект). Command Query Separation, позволяет читателю понять — есть у функции side-эффект или нет.

Экстремальный уровень command query separation — дисциплина Tell Don’t Ask. Это дисциплина, при которой мы не спрашиваем состояние объекта, а потом вызываем его же для вызова функции. Мы сразу вызываем функцию и в случае ошибки получаем exception 

Обычно код выглядит следующим образом. Есть класс Payment и этого класса мы можем спросить состояние. В зависимости от этого выполнить какое-то действие. 

val payment = Payment()
if(payment.isStatusOk()) {
    payment.reverse()
}

А почему бы не попросить сам объект выполнить функцию, так как он сам знает свое состояние. А если что-то пошло не так — то обработать это исключение. Дисциплина Tell Don’t Ask говорит, что у payment можно сразу вызвать метод reverse(), внутри которого будет проверяться, в правильном ли он статусе, чтобы вернуться. Потом обработать ошибку, если по какой-то причине статус транзакции оказался неправильным. 

Дисциплина Tell Don’t Ask избавляет нас от нарушения закона Деметры. Закон Деметры выглядит так:  Внутри класса ReceiptReporter можно обращаться только к demetraOk , demetraOk1 ,demetraOk2или к глобальным переменным.

class ReceiptReporter(val demetraOk: SomeClass) {
    val demetraOk1 = SomeClass()

    fun getSellerInn(successPayment: SuccessPayment, demetraOk2: SomeClass) {
        /** Нарушение закона */
        successPayment.getMerchant().getLegalEntity().getInn()
    }
}

Закон Деметры говорит о том, что подобный паровозик successPayment. getMerchant().getLegalEntity().getInn() — это плохо потому что в этом месте создается цепочка общих знаний всей системы, которую потом очень сложно распутать.

Закон Деметры говорит о том, что наш класс должен знать только о своих соседях и не должен знать о всей системе. Такие длинные цепочки означают, что этот класс знает очень много о всей системе. Класс ReceiptReporter знает, что у платежа есть мерчант, у мерчанта есть юрлицо, у юрлица есть ИНН. Таким образом, если, между мерчантом и юрлицом появится дополнительная сущность, то нам нужно будет искать и править абсолютно все места, где существует такая связь, которая знает о всей иерархии системы. 

Tell Don’t Ask позволяет нам сказать объекту, что ему нужно сделать. Он передаст это своему соседу, так как сам знает как выполнить запрос. Сосед же передаст своему соседу. В итоге мы дойдем до класса, который сделает то, что мы хотели.

Ранее у successPayment мы по цепочке запрашивали getMerchant(). getLegalEntity().getInn(). Но можно было бы классу  SuccessPayment добавить  метод getSellerInn().

class ReceiptReporter(val demetraOk: SomeClass) {
    val demetraOk1 = SomeClass()

    fun getSellerInn(successPayment: SuccessPayment, demetraOk2: SomeClass) {
        /** Решение (соблюдать полностью закон почти невозможно - но нарушение - плохой запах) */
        successPayment.getSellerInn()
    }
}

class SuccessPayment {
    fun getMerchant(): Merchant {
        TODO()
    }
    fun getSellerInn(): Long {
        return getMerchant().getLegalEntityInn()
    }
}

class Merchant {
    fun getLegalEntity(): LegalEntity {
        TODO()
    }
    fun getLegalEntityInn(): Long {
        return getLegalEntity().getInn()
    }
}

class LegalEntity {
    fun getInn(): Long {
        TODO()
    }
}

Тогда successPayment мог бы у своего соседа попросить ИНН, а дальше этот сосед  (мерчант) спросил бы этот ИНН у самого класса LegalEntity. В коде было бы написано: если ты не знаешь, что такое ИНН, то спроси у своего соседа, который может знать, где его взять. 

Закон Деметры очень часто называют не законом, а рекомендацией, потому что его очень сложно соблюдать. Но используя Tell Don’t Ask, мы приближаемся к исполнению закона Деметры.

Структурное программирование

Структурное программирование началось с публикации Эдсгера Дейкстры под названием «goto — опасный оператор». Структурное программирование — программирование, в котором есть три базовых блока: 

1. Последовательность, когда один блок следует за другим.

println("ABC")
println("123")

2. Предикат, когда есть булева логика, которая разделяет переменную на две нити, после чего логика соединяется в одну нить.

if(Random().nextBoolean()) {
    println("ABC")
} else {
    println("123")
}

3. Повторение, когда определенный блок кода повторяется несколько раз, пока не удовлетворится какое-то условие.

for(i in 1..5) {
    println(1)
}

Используя эти блоки, мы можем рассуждать о программе последовательно. Эдсгер Дейкстра говорил, что если не использовать оператор goto, то такую программу будет легко читать, потому что имеется один логический вход и один логический выход. Эта публикация говорит о большом вреде нелинейного кода. Нелинейный код - код, в котором непонятно, где вход в программу, а где выход из программы. Ниже пример нелинейного кода:

val needOnlyCaptured = true
val payments = setOf<Payment>()
for(payment in payments) {
    /** Сложные ранние выходы (возвраты) являются нарушениями линейности кода (кстати label@brake еще хуже) */
    if(!payment.isCaptured() && needOnlyCaptured || !payment.getGateway().hasClearingFeature) {
        break
    }
    
    TODO()
}

В цикле есть выход, который сложно прочитать и понять. Он похож на оператор goto потому что непонятно, в какой момент мы выйдем из этого кода. Таким образом код становится нелинейным. 

Соответственно, сложных выходов из цикла надо избегать, но простые выходы можно оставить. То есть, если есть условие payment.needAttention() , которое просто возвращает true илиfalse, то логически это легко прочитать и понятно, когда мы выйдем из этого цикла.

val payments = setOf<Payment>()
for(payment in payments) {
    /** Простые возвраты */
    if(payment.needAttention()) {
        break
    }
    
    TODO()
}

В следующей части мы поговорим о классах и о коде без привязки к функциям или классам.