Привет! Этим постом я завершаю цикл из конспектов видеолекций Дяди Боба про чистый код. Вот ссылки на предыдущие:

Сегодня обсудим исключения, классы и все, что не вошло в прошлые разделы.

Обработка исключений

Не раскрывайте реализацию

Майкл Физерс (Working effectively with legacy code) сказал: «Если обработка ошибок раскрывает реализацию — то это неправильная обработка ошибок». Не раскрывать реализацию можно, если написать исключения перед тем, как написать реализацию функции (привет TDD — по-другому и не получится).

Рассмотрим классCommissionCalculator , который обменивает сумму в разных валютах.

class CommissionCalculator(
    private val currencyCacheService: CurrencyCacheService,
    private val commissionService: CommissionService,
    private val logger: Logger
) {
    fun exchangeCommission(payment: Payment, toCurrency: Int): Double {
        try {
            val commissionInPaymentCurrency = commissionService.getCommission(payment.amount, payment.merchant)
            val exchangeRate = currencyCacheService.getRateForCurrency(payment.currency, toCurrency)
            return exchangeRate * commissionInPaymentCurrency
        } catch (exception: TGetCommission) {
            /** Раскрываем подробности того, что ходим в сервис комиссий */
            throw CommissionServiceTimeout()
        } catch (exception: TCurrencyCacheService) {
            logger.error("Не могу обменять потому что нет такой валютной пары", exception)
            /** Здесь подробностей реализации нет */
            throw ExchangeIsUnavailable()
        }
    }

    class ExchangeIsUnavailable : Exception()
    class CommissionServiceTimeout : Exception()
}

Выбрасываемое исключение CommissionServiceTimeout из метода exchangeCommission() раскрывает то, что в классе сервиса мы ходим по сети в сервис с соответствующим названием. Это является раскрытием реализации. Правильнее было бы выбросить исключение ExchangeIsUnavailable, не раскрывающее подробностей реализации.

Используйте исключения в ожидаемых местах

Например, тут я ожидаю исключение, если что-то пошло не так, а не -1 в ответе.

val testSet = mutableSetOf("value")
/** Еще вспоминаем CQS */
testSet.add("value")

Но вот тут ожидаемо -1 в ответе, а не исключение, потому что вполне ожидаемо, что в списке может не быть значения.

val map = listOf(1)
map.indexOf(10)
Ещё примеры

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

val existingRefund = Refund()
RefundTransactionDao().getParentPayment(existingRefund)

Тут я бы не ожидал ошибку.

val existingRefund = Refund()
RefundTransactionDao().findById(1)

Чтобы понять, что ожидаемо, а что неожиданно, можно спросить у своего соседа — какого поведения он ожидал от конкретной функции.

Не следует выкидывать из функций исключения из стандартной библиотеки

В клиентском коде сложно будет отделить обработку ошибок при работе с вашим кодом. Например, вот тут вместо исключения IllegalStateException надо было выбросить PaymentWrongStatusToReverseException.

Избегайте описания исключений

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

try {
    payment.reverse()
    /** Тут еще какой-то код, что может кинуть исключение */
} catch (exception: IllegalStateException) {
    println(exception)
}

Классы с ошибками очень удобно хранить в самом классе, что их кидает

Так их всегда легко найти и не будет общего пакета exceptions , на который ссылаются все остальные пакеты (пакеты потом сложно развязать).

class ClassThatThrowException {
    class ExceptionOneFromThisClass : Exception()
    class ExceptionTwoFromThisClass : Exception()
}

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

Не наследуйтесь от проверяемых исключений

Этот опыт во всех языках признали неудачным, потому что они создают зависимость, обратную иерархии наследования. Если в отнаследованном классе есть метод и он начинает бросать новое проверяемое исключение — то в базовом методе тоже надо менять сигнатуру (и соответственно в других наследниках), а это нарушает принцип открытости закрытости (O в SOLID).

То есть если в коде ниже мы добавим проверяемое исключение FileNotFoundException в сигнатуру метода read()— придется добавлять это исключение и в интерфейс FileReader, и во все его потомки.

class JpegFileReader : FileReader {
    @Throws(FileNotFoundException::class)
    override fun read() {
        TODO("Not yet implemented")
    }
}

interface FileReader {
    fun read()
}

Пишите обработку исключений в одну функцию.

try {
    authPayment(payment)
} catch (exception: Exception) {
    createTechReverse(payment)
}

Оно вытекает из S в SOLID. Функция должна делать что-то одно. Обработка ошибки — что-то одно, поэтому обработка исключений тоже должна исполняться в одной функции. То есть в нашем примере обработка исключения происходит одним вызовом createTechReverse().

Комментарии

Как можно реже используйте комментарии

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

// [PAYIN-1843] adding some tag with 'D' because something happen
fun addTag45() {
    TODO()
}

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

A comment is a failure to express yourself

Это значит, что вы не смогли написать говорящий код вместо комментария. Например, у нас есть код, где мы создаем токен в определенном статусе, и у нас есть комментарий create token only with status IN_PROGRESS. Этот код комментирует то, что мы могли бы написать в функции. 

// create token only with status IN_PROGRESS
val token = SbpPayinToken(
    status = SbpPayinTokenStatus.IN_PROGRESS
)

Например, мы могли бы создать функцию, которая перенесла бы это описание в само название и называлась бы createSbpPayinInProgressToken().

fun createSbpPayinInProgressToken(): SbpPayinToken {
    TODO()
}

Комментарии врут и дезинформируют

В первую очередь из-за того, что они не локальные. Вначале комментарий мог и не врать, но потом функциональность изменили в другом месте, и комментарий стал дезинформирующим. Давайте смотреть на реальном примере.

fun pay(payment: Payment) {
    try {
        doPayment(payment)
    } catch (exception: Exception) {
        // Делаем тех реверс
        payment.techReverse()
    }
}

У нас есть функция pay(), которая делает doPayment(), иначе делает techReverse(). Мы добавили комментарий, что делаем тех реверс. Этот комментарий не локальный, потому что внутри функции techReverse() может что-то поменяться. 

Мы можем добавить дополнительный код, в результате чего этот комментарий станет дезинформирующим. Например, в функцию techReverse() нам понадобилось добавить возможность сделать refund() (отличается тем, что он с комиссией и не моментальный), если reverse() не удался.

class Payment() {
    /** Мы возможно даже переименовали бы это, но комментарий забыли скорее всего */
    fun techReverse() {
        if(reverse()) {
            true
        } else {
            /** Эта ветка добавилась позже */
            refund()
        }
    }

    private fun reverse(): Boolean {
      TODO()
    }
    private fun refund(): Boolean {
       TODO()
    }
}

Сначала у нас был код без ветвления — просто вызываем reverse() и все.  Потом мы добавили еще один if else, в результате этого стал выполняться либо reverse(), либо refund(). Мы можем даже не забыть переименовать метод techReverse() , но, скорее всего, забудем переименовать комментарий. Мы добавили изменение в методе techTeverse() класса Payment и не имеем понятия, что есть еще какой-то комментарий где-то в другом месте, который в результате нашего изменений станет дезинформировать.

Другие плохие случаи использования комментариев. Это комментарии вида ворчания или рассуждения, так как это размывает важность комментариев. Вот пример такого комментария:

// Меня заставили так написать, я не хотел, но продакт, будь он неладен, настоял

Комментарии очевидного (DRY)

// Payment Id
val paymentId = "124bdc"

Что делать, когда вы видите неправильный комментарий?

Соблюдайте правило бойскаута

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

Комментарий можно хранить в Git’е.

Такой комментарий  можно убрать в систему контроля версий и не засорять код.

// [PAYIN-1864] add this because of this ticket

Еще пример. Есть аннотация @Disabled в тестах, и в нее принято писать причину отключения теста. Здесь следовало бы добавить причину отключения в систему контроля версий.

@Disabled("Disabled until CustomerService is up!")
@Test
fun testCustomerServiceGet() {
assertEquals(2, 1 + 1);
}

Не пишите комментарий, который может стать не локальным.

Вот пример не локального комментария. Есть функция setPort, а port указан в application YAML. Добавили комментарий, что устанавливаем порт 10000. Естественно тот, кто будет править порт в application YAML, понятия не имеет, что где-то есть комментарий, в котором указано, что порт у нас — 10000.

// Set port 10000 to run web application
fun setPort(port: Int) {
TODO()
}

Остальные быстрые правила, как не надо использовать комментарии.

  • Не кладите в код HTML.

  • Не оставляйте в коде - «Я автор этого кода».

  • Не храните в комментариях changeLog.

  • Удаляйте закомментированный код и пометки, что этот код надо будет удалить.

  • Не храните TODO — вместо этого заводите в трекере техдолг.

Когда можно использовать комментарии?

  • Если это комментарии об использовании лицензии.

  • Если эти комментарии информативные. Например, вы пишете какое-то регулярное выражение, и по нему сложно понять, что там делается.

  • Если это какой-то SQL-код, потому что SQL-код сложно сделать выразительным, поэтому к нему можно написать какой-то комментарий.

  • Если это комментарий к публичным API.

Форматирование — важно

Оно является частицей информации. Например, в Python компилятор смотрит код и обращает внимание на форматирование. С форматированием нужно обращаться заботливо. 

fun doSome() {
    println("Start process")
    doSome1(param1 = "param1")

    /** Перенос это частица информации - что закончился один логический блок и начался другой */
    println("End process")
    doSome1(param1 = "param1")
}

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

Например, такой код тоже смотрится неряшливо, его сложно читать, он выглядит так, что форматирование для того, кто написал этот код, ничего не значит (здесь перенос сделали просто по достижению строки в x символов). 

fun doSome1(
    param1: Any,
) {
    logger().info("asdawf ewfqewfeqwfq ewfqewfeqfe qwfeqwfeqwfqewfq wefqwefqwe fqewfeqf ewdqwedqwed {}, {}", "dasc",
        "ascdasdadsawf")
}

Не нужно делать такие табуляции (раньше модно было так их делать, кажется, в C++), потому что такая табуляция заставляет читателя читать либо список переменных, либо их типы, либо их уровень доступа.

class SomeClass {
    private     val socket:                     Socket? = null
    protected   var requestParsingTimeLimit:    Long = 0
    private     val requestProgress:            Long = 0
}

Размеры файлов

Дядя Боб говорит, что размеры файлов должны быть от 30 до 100 строк. Доказательство этому следующее. Он рассматривает проекты Junit, fitnesse, testNG, tam, jdepend, ant и tomcat, которые характерны такими свойствами: долгим сроком жизни, достаточно постоянной скоростью развития и добавления новых фич. Я в первой статье говорил , что чем больше становится проект, тем сложнее в него добавлять фичи и тем большее время на это затрачивается, то есть продуктивность падает со временем. Дядя Боб отмечает, что эти проекты достаточно качественные. 

Все эти проекты разных размеров, и он делает исследование по тому, сколько в среднем составляет размер строк в файлах. Максимальный размер файлов у каждого проекта — 500 строк, средний размер класса — 50-60 строк, а большинство файлов от 30 до 100 строк кода. На этой основе он рекомендует придерживаться такого размера, потому что эти проекты долго живут и развиваются. PS в проектах, в которых используется TDD, обычно размеры файлов меньше.

Вертикальное форматирование

Разделяйте переводом строки разные логические блоки

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

class SomeClass(private val value1: String) {
    private val logger = logger()

    /** Закончился блок с переменными - начались функции */
    fun someFunction() {
        logger.info("Start process")
        doFirst()

        /** Переход строки разделяет логические части */
        logger.info("On process finish called doSecond")
        doSecond()
    }

    private fun doFirst() {
        TODO()
    }

    private fun doSecond() {
        TODO()
    }
}

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

@Test
fun test() {
    /** Arrange */
    val someClass = SomeClass("abc")

    /** Act */
    val result = someClass.someFunction()

    /** Assert */
    assertEquals(Unit, result)
}

Я чуть выше говорил, что перевод строки — часть информации, поэтому, например, в тестах есть правило или практика Act-Arrange-Assert  В таких функциях нужно разделять переводом строки логические блоки Arrange, Act, Assert

Группируй вместе все, что используется вместе

Например, есть класс Car. Внутри него вместе используются переменные тип топлива и максимальная дистанция, которую он может проехать на этом топливе. Также вместе используются переменная комплектации автомобиля и дополнительные пакеты. Эти переменные нужно группировать вместе так как они используются вместе.

class Car {
    val gasolineType = "Diesel"
    val maxDistance = 100

    val equipment = "Super"
    val additionalPackages = listOf("Winter package")
}

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

fun displayWarning(car: Car, distance: Int) {
    /** Должно быть инициализировано в месте использования */
    val isEnoughGasoline = car.maxDistance > distance
    val hasConditioner = car.equipment == "Super"
    val isLongDistance = distance > 250
    if(hasConditioner && isLongDistance) {
        println("Turn off the conditioner to eco mode")
    }
    if(!isEnoughGasoline) {
        println("Find the gasoline station on the route")
    }
}

Пример нарушения этого правила — метод displayWarning(). Он отображает ошибки на дисплее водителя. В нем инициируются переменные — достаточно ли топлива, есть ли кондиционер и длинная ли дистанция. 

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

Нарушение тут состоит в том, что переменная isEnoughGasoline инициализируется не в том месте, в котором она используется. Правильно было бы перенести ниже, то есть там, где она используется.

Скролла вправо не должно быть

Дядя Боб аналогично использует анализ тех же самых проектов и говорит, что на их основе максимальное количество символов в строке для всех них — 80, среднее количество символов в строке — от 30 до 50. Поэтому он рекомендует, чтобы для большинства строк у нас было максимальное количество символов 40, а 80 — это уже много. 

Это правило сформировалось достаточно давно, потому что скролл вправо заставляет перечитывать информацию по несколько раз. Когда вы доходите до аргумента номер 8, вы уже забываете, что было в аргументе 1. Когда вы идете направо, то ты забываете, что было слева. Но так как экраны стали широкими после введения этого правила, его нарушение перестало быть такой большой проблемой. Но все равно, желательно этого правила придерживаться, потому что человеческая память — штука прихотливая

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

Связанные классы

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

class ScreenPrinter(private val user: User, private val device: Device) {
    fun printDayReport() {
        device.print(user.toString(), 90)
    }

    fun printNightReport() {
        device.print(user.toString(), 30)
    }
}

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

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

class TruckHelper(private val capacity: Int, private val fuelDistance: Int, private val orderDocuments: String) {
    fun loadCargo() {
        capacity
        TODO("Check capacity / set current weight")
    }

    fun buildRoute() {
        fuelDistance
        TODO("Check fuel distance and find route with gasoline stations")
    }

    fun unload() {
        orderDocuments
        TODO("")
    }
}

P.S. При добавлении сеттеров в сильно связанные классы вы делаете класс менее связанным. Это делать нежелательно. Чем меньше сеттеров, тем лучше.

Абстрактные данные — путь к полиморфизму

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

class Car {
    /**
     * Раскрываем что наша машина ездит на бензине.
     * Кроме того для дизельной машины это неправильно, а для электрической вообще нерелевантно
     */
    fun getGallonsOfGas(): Double {
        TODO()
    }
}

У класса Car есть функция getGallonsOfGas(). Получается, что мы раскрываем информацию о том, что этот автомобиль ездит на бензине. Таким образом мы теряем возможность сделать наследование от этого класса, например, для электромобиля, или дизельного автомобиля. 

class Car {
    fun getPercentOfFuel(): Double {
        TODO()
    }
}

Что можно было бы с этим сделать? Можно переделать класс Car так, чтобы у нас появился метод getPencentOfFuel()— сколько у нас осталось топлива в процентах. Это не противоречит ни электромобилю, ни машине на дизельном топливе.  

P.S. Здесь надо быть аккуратными: на мой взгляд, делать вещи слишком абстрактными опасно, потому что сложно становится понимать, что происходит в коде.

Дата-классы

Говоря о swith case , мы обещали вернуться к обсуждения. Время пришло. Дата-классы, они же DTO, структуры данных — это антиклассы. У дата-классов не должно быть или почти не должно быть функций и у них нет связности. Они раскрывают реализацию, и они антиполиморфны. Это значит, что switch-case нормально с ними использовать именно с дата классами. 

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

Например, есть класс CarManagementConsole (панель управление автомобилем), у которого есть метод accelerate()— двигаться. Этот метод ожидает на вход любую реализацию интерфейса Transmission

class CarManagementConsole {
    fun accelerate(transmission: Transmission) {
        transmission.drive()
    }
}

class DieselTransmission : Transmission {
    override fun drive() {
        TODO("Not yet implemented")
    }
}
class GasolineTransmission: Transmission {
    override fun drive() {
        TODO("Not yet implemented")
    }
}
/** Добавляем новый тип - ничего не меняется в CarManagementConsole */
class ElectricTransmission: Transmission {
    override fun drive() {
        TODO("Not yet implemented")
    }
}
interface Transmission {
    fun drive()
    /** Но если нам нужно функцию stop в клиентский код - нужно добавлять везде и перекомпилировать и пакет Transmission*/
}

Обычный класс позволяет нам подменять поведение и делать новые реализации Transmission (то есть у нас может быть реализация DieselTransmission, GasolineTransmission и ElectricTransmission), не меняя при этом вызывающий код в CarManagementConsole. При этом добавлю — если нам надо будет реализовать в CarManagementConsole метод stop() , нам придется править его самого и каждую реализацию Transmission — то есть очень больно.

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

Например, у нас есть интерфейс ControlPanel — панель управления автомобилем (всякие рычажки, которые вы можете понажимать в автомобиле). Есть реализации TruckControlPanel, CarControlPanel и BikeControlPanel , которые говорят, что у разных типов транспортных средств есть разный набор органов управления. 

Также есть класс-хелпер TrafficRulesHelper , который говорит, что надо делать в разных ситуациях на дороге по ПДД для разных типов транспортных средств. Например, у нас есть метод в случае аварийной остановки emergencyStop(): для грузовика — включить аварийку, сообщить что-то по рации и выставить emergency-знак; для автомобиля — что-то другое; для мотоцикла — что-то еще, например, просто включить аварийный сигнал. 

interface ControlPanel
data class TruckControlPanel(val turnSignal: Any, val backCamera: Any, val transmitter: Any, val emergencySign: Any) : ControlPanel
data class CarControlPanel(val turnSignal: Any, val emergencySign: Any) : ControlPanel
data class BikeControlPanel(val turnSignal: Any) : ControlPanel

class TrafficRulesHelper {
    fun emergencyStop(controlPanel: ControlPanel) {
        when(controlPanel) {
            is TruckControlPanel -> TODO("Turn all signals / call transmitter / set emergency sign")
            is CarControlPanel -> TODO("Turn all signals / set emergency sign")
            is BikeControlPanel -> TODO("Turn all signals")
        }
    }

    fun startDrive(controlPanel: ControlPanel) {
        when(controlPanel) {
            is TruckControlPanel -> TODO("Turn left signals / call transmitter / see in back camera")
            is CarControlPanel -> TODO("Turn left signals")
            is BikeControlPanel -> TODO("Turn left signals and raise left hand")
        }
    }

    /** Добавляем еще много функций */
}

Дата-классы позволяют нам добавлять таких клиентских функций бесчисленное количество, то есть мы можем добавить безболезненно метод startDrive() и нам не нужно будет ничего править, кроме места в вызывающем коде. Но если у нас появится какая-то новая реализация, например, BusControlPanel , то нам придется полностью переделывать каждый switch case тут и смотреть везде, где он используется. Это будет достаточно больно. 

Итого: используйте классы там, где надо предохранить от добавления новых реализаций. Используйте дата-классы, где надо предохранить от добавления новых клиентских функций (switch case)

Еще один пример, где какой тип классов использовать. При работе с базой данных. База данных очень конкретна (не полиморфна). Она полностью раскрывает детали данных, которые в ней лежат. А это значит, в ней лежат дата-классы. Но в нашем приложении мы хотим пользоваться гибкостью полиморфизма и независимостью от деталей в БД. Это значит, что в слое с работы с конкретной БД мы используем дата-классы, а при пересечении границы с приложением — уже работаем с доменными объектами (использование инструментов типа hibernate полезно, но надо понимать, что они просто загружают в память в DTO строку из БД).

interface RefundDao {
    fun getById(id: String) : Refund
}
class Refund {
    fun recalculateCommission() {
        TODO()
    }
}

class OracleRefundDao : RefundDao {
    override fun getById(id: String): Refund {
        val refund = RefundDataClass("1", 10.00, 643) /** Достали из бд */
        return Refund()
    }
}

Как видно, в коде реализация работает с дата-классом (например, это могло быть через hibernate). А когда нужно вернуть приложению объект — уже возвращается обычный класс. 

Послесловие

В завершение хотел сказать, что написание чистого кода не зависит от вашего продакта и от ваших коллег. Написание чистого кода — проявление вашего профессионализма. Написать код, понятный для компьютера — полдела, настоящая задача — написать код, понятный для человека. У меня на этом все.

P.S. Добавлю кое-что про тестовый код. Я очень часто замечаю, что к тестовому коду относятся как к побочному коду, как к коду, к которому можно не применять правила для чистого кода, писать его как попало. Но к тестовому коду надо относиться так же, как и к продакшн-коду, потому что он предохраняет от каких-то ошибок и позволяет нам делать рефакторинг, на основе которого мы можем оставлять код чистым и защищать его от гниения.

P.P.S. Недавно на хабре была очень популярная статья по тому, что чистый код плох в производительности (не буду давать на нее ссылку, чтобы не популяризировать). На мой взгляд, читабельность и поддерживаемость кода стоит куда больше, чем какое-то количество «лишне созданных» объектов. Кроме того, в большинстве случаев проблема производительности кроется либо в хранилище и сети, либо в каком-то малом количестве мест и выявляется трассировкой  

P.P.P.S. Когда меня спрашивают - можешь в двух словах объяснить зачем нужен чистый код - я обычно скидываю этот мем

Комментарии (47)


  1. AlexXYZ
    16.05.2023 06:59
    +2

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

    Прям выглядит как шутка. У меня раз в полгода-год просят добавить какие-то параметры в форме из 150 значений (проектирование в строительстве). И везде в коде стоят комментарии что и куда прописать. Читаю комментарии, делаю по алгоритму, может даже что-то добавляю в комментарии и забываю на следующие полгода-год. )))

    Во всей этой чехарде с исключениями, особенно когда их много больше всего меня напрягали формулировки, потому что даже при логировании достаточно трудно провести поверхностный анализ того, что происходит, т.к. формулировки в логах могут совпадать. Потом я попробовал применить уникальную нумерацию сообщений в исключениях. И это дало приличные результаты: пользователь звонит и говорит номер ошибки, а что написано в сообщении уже почти не важно. Теперь легко сопоставить ошибку с кодом, а в логах найти историю этой ошибки по другим номерам от других сообщений. Своеобразный stackTrace по бизнеслогике. И пользователю не надо сильно распинаться и разработчику и саппорту проще понять, что происходит и как начать разговор. Например, вместо долгих объяснение что делалось, что ожидалось и т.д. просто говоришь номер ошибки, а т.к. они все уникальны, то выспрашивание контекста можно делать на самой последней стадии общения. И, кстати, нумерация исключений вполне может скрывать реализацию исключений. От пользователя иногда достаточно получить номер ошибки, чтобы понять контекст. Кстати, введение нумерации добавляет уверенности пользователю, что это не он дурак, а в системе предусмотрен обработчик и пользователю нужно запомнить, что сделано не так, чтобы избежать такой ошибки в будущем.

    Честно говоря, примеры исключений, вписанных в статье, лично мне кажутся не очень хорошей практикой. Нет четкой связи с кодом, где возникло исключение. Там, где хоть какое-то описание контекста попадает в исключение ещё можно связать с местом в коде, а там где исключение просто бросается, например, что «нет такой валютной пары», так и потом даже с логами не понять, что произошло: в логах нет намёка, что исключение брошено, а брошенное исключение никак не связано с записью в журнале логов. Может добавить сообщение в конструктор исключения при вызове исключения + нумерация? (Это как предложение) Например, «5843. Нет такой валютной пары» и «6432. Нет такой валютной пары» могут быть в разных местах кода и по номеру проще понять где они брошены.


    1. lexus1990 Автор
      16.05.2023 06:59
      +1

      Используйте Mapped Diagnostic Context (MDC) для логирования. Пишите в логи stackTrace тогда понять где была ошибка будет не проблемой. Пользователю как раз надо отдавать traceId из этого MDC.

      Название ошибок существует в основном (но не только) для чтения кода. Одно исключение должно кидаться как можно в меньшем количестве мест и должно быть как можно более уникальным - тогда не будет проблем с поиском по коду - откуда вылетеле именно это исключение.


      1. mixsture
        16.05.2023 06:59
        +1

        И в результате мы исключаем возможности пользователя (и любого ИТ-стафа вокруг него) по диагностике и исправлению ошибки. Хотя ошибки бывают тривиальные и могут быть исправлены самим пользователем, но сообщения об ошибках вида "что-то пошло не так" (как раз идеально не раскрывающие сути) не оставляют ему шансов.
        Но и это не единственное. Ваш подход делит сообщение об ошибке и теперь оно вместо 1 места показывается/хранится по частям в нескольких: одна часть у пользователя, одна часть в логе. И потеря любой части теперь критична. Пользователь вспомнил об ошибке, с которой столкнулся полтора месяца назад и прислал скрин? А у нас логи ошибок обрезаются по истечении 30 дней и поэтому больше никакой инфы уже не осталось — до свидания.


        Я бы отнес это скорее к современным отвратительным практикам, которые сильно уменьшают и первичную диагностику на стороне пользователя, и объем обратной связи.


        1. lexus1990 Автор
          16.05.2023 06:59
          +1

          Использование MDC относится к практике observability.

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

          То есть если карта пользователя истекла - надо отобразить ему какую-то ошибку - попробуйте другой картой. Это вполне ожидаемый сценарий.

          Если у нас где-то произошла сетевая проблема между сервисами - то пользователь ничего с этим не сделает. Отображать ему что-то нет смысла.


      1. AlexXYZ
        16.05.2023 06:59
        +1

        Одно исключение должно кидаться как можно в меньшем количестве мест и должно быть как можно более уникальным - тогда не будет проблем с поиском по коду

        Честно говоря я не согласен с этим потому что в моём примере в проекте последний номер ошибки на сегодняшний день 2051. Как в этом случае вы предлагаете мне поступить - создать примерно такое количество классов исключений? (Я не думаю, что вы хотели, чтобы я сделал именно так, но исходя из вашего предложения я не могу представить себе чего-то другого).

        P.S.

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

        P.P.S.

        Тему вы подняли очень важную - исключения - это только только верширна айсберга при обработке ошибок. )))


        1. lexus1990 Автор
          16.05.2023 06:59

          Вы храните где-то отношение что код 2051 = неправильный параметр номер телефона (к примеру). Вот если бы вместо него было UserPhoneValidationException - это был бы перенос документации в код. Пользователю в этом случае наверно надо было бы отобразить - введите друго телефон, а не код ошибки 2051 - смотрите по доке что это значит


        1. lexus1990 Автор
          16.05.2023 06:59

          Кстати, может быть вместо очень большого описания проблемы посмотрим на коде? Вы можете примеры привести на псевдокоде?


          1. AlexXYZ
            16.05.2023 06:59

            Вы можете примеры привести на псевдокоде?

            Да, конечно. Можно прямо в коде:

            Пример предупреждения пользователю (сорри если некоторые тексты сообщений не очень понятны - уж что попросили конструкторы, то я и написал. На мои вопросы они отвечают - "мы знаем что это значит, оставьте так". Ок):

            Пример в логах:

            Пример исключения для логов (в данном случае исключение 172 чуть позже бросается):

            Пользователю в этом случае наверно надо было бы отобразить - введите другой телефон, а не код ошибки 2051 - смотрите по доке что это значит

            Так я же не требую выводить только код ошибки. Добавьте после кода ошибки сопроводительный текст и информация будет предоставлена в том же виде, что и раньше. 3-5 символов на числовой код ошибки не сильно что-то испортят в сообщение, но зато у пользователя и разработчика/оператора программы будет предметный разговор. Например, мне звонит сотрудник и говорит, что у меня на экране сообщение: "...", и сообщает оператору первое же число - код ошибки: "172" А у оператора перед глазами список действий, которые он может предложить пользователю по этому номеру. Согласитель, общение же более конкретное, чем спрашивать, как он к этому сообщению попал? Примерно всегда можно установить даже ФИО разработчика, который этот генератор/обработчик ошибки написал. Известное выражение: "У каждой ошибки есть имя и фамилия". /s

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

            Да, и напомню, что мне всё ещё интересно как вы предлоагаете выйти из ситуации с количеством классов исключений для большого количества ошибок? Писать же отдельный класс на каждую ошибку очень накладно.


            1. lexus1990 Автор
              16.05.2023 06:59

              А зачем пользователю отдавать номер ошибки если есть описание и то, что он должен сделать? Это не трата денег - звонить оператору? Разьве оператору надо звонить не только если произошло что-то неожиданное?

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

              P.S. Я не понял чем код ошибки лучше чем traceId по которому поддержка может посмотреть что не так (если учесть что пользователь звонит только в случае ошибки которая требует оператора)


              1. AlexXYZ
                16.05.2023 06:59

                Я не понял чем код ошибки лучше чем traceId

                В принципе, если traceid детерминантный (т.е. на одну и ту же ошибку в одном и том же месте программы выдаётся один и тот же код), то разницы нет. А вот если разный, то это плохо. Но я работаю на C# и у меня нет MDC. Кстати, при изменении исходного кода изменяется traceid?

                Но вы ещё так и не ответили на мой вопрос. )


                1. lexus1990 Автор
                  16.05.2023 06:59

                  Для C# есть библиотека с реализацией MDC https://nlog-project.org/documentation/v4.3.0/html/T_NLog_MappedDiagnosticsLogicalContext.htm - не знаю на сколько она является стандартом. Но скорее всего есть аналоги которые являются стандартом.

                  Код ошибки и traceId совершенно для разных целей. Первое для наблюдаемости. Второе для того чтобы пользователь мог сам решить свою проблему.


                  1. AlexXYZ
                    16.05.2023 06:59

                    Код ошибки и traceId совершенно для разных целей

                    Моё личное мнение - не стоит плодить разные контексты для чисел, которые показываются пользователю. Можно придумать ещё какой-нибудь контекст, например, pidId и другие разные Id. Но пользователю, который столкнулся с исключением в конкретный момент прохождения алгоритма своего решения от этого не легче.

                    Я не понимаю, почему по traceId можно решить свою ошибку, а по коду ошибки нет? (В случае проблем при взаимодействии с вашей системой пользователь видит два числа Код Ошибки и TraceId или что-то одно?)

                    P.S.

                    Спасибо за подсказку с библиотекой, посмотрю.


                    1. lexus1990 Автор
                      16.05.2023 06:59

                      Ошибок которые может решить сам пользователь обычно не очень много, если только в приложении нет бизнес логики как таковой. Чаще всего именно исключения (не ошибки) возникают в бизнес логике. Надо разбираться со стектрейсом - что произошло. Исправлять, заводить техдолги и тд


                1. lexus1990 Автор
                  16.05.2023 06:59

                  Еще не понял про того - кто добавил этот обработчик ошибки. Несколько людей могут изменить - что-то добавить в любой обработчик. Как вы определяете ответственного за весь класс / обработчик?

                  P.S. Вы не пользуетесь подходом общего владения кода? Это очень важный подход в крупных компаниях.


                  1. AlexXYZ
                    16.05.2023 06:59

                    У нас ответственность на уровне модуля. Ставится задача отдельному сотруднику, он пишет небольшой модуль. Из него делается nuget-пакет и он попадает в проект.

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

                    Ок, но пусть даже и большая компания, но взять тот же синий экран "смерти" Windows. У него есть число и, может быть, файл с трассировкой стека. Но ведь число? О нём речь и идёт, о числе.


                    1. lexus1990 Автор
                      16.05.2023 06:59

                      Отличный пример про PSOD. Такого не должно быть впринципе. Если такая ошибка случилась - это должно быть что-то неожиданное и серьезное. Вариантов того, почему это произошло может быть огромное количество. И понимание даже конкретного эксепшна не дает полной картины - что произошло. Поможет только traceId (или логи ошибки со стектрейсом, которые и просят отправить когда падает ПО запущенное на личном компе).


                      1. AlexXYZ
                        16.05.2023 06:59

                        Но разбор таких ошибок - это уже немного другая область. Но опять же, в инете можно поискать что означает ошибка синего экрана с номером x80005142563 и т.д. Т.е. поиск можно начать с конкретного числа. Сам stacktrace уже вторичен. Но опять же - всё сходится на определённом числе независимо от типа исключения. Это же хорошо?


                      1. lexus1990 Автор
                        16.05.2023 06:59

                        Получив такой код начинаешь рыться по форумам поддержки - документации этого ПО. В итоге это обычно приводит к тому, что ошибка в каком-нибудь драйвере видеокарты. Примерно то же самое (только чуть быстрее) было бы если бы я получил ошибку GriphicsDriverException


                1. lexus1990 Автор
                  16.05.2023 06:59

                  Я вроде ответил на ваш вопрос по количеству exception. Для отображения пользователю я вообще не стал бы их использовать.

                  Для кода приложения можно использовать сколько угодно исключений и фраза "накладно" совсем мне не понятна. Возможно вы пишете код для микроконтроллеров, но тогда непонятно почему вы это делаете на C#


                  1. AlexXYZ
                    16.05.2023 06:59

                    Для отображения пользователю я вообще не стал бы их использовать.

                    Теперь я вас понял. Но, как тогда общаться с пользователем, у которого возникло исключение (за исключением ситуации, когда исключения можно обработать автоматически)

                    Возможно вы пишете код для микроконтроллеров

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


                  1. AlexXYZ
                    16.05.2023 06:59

                    Для отображения пользователю я вообще не стал бы их использовать.

                    Подождите, я переосмыслил ваш ответ, но речь шла не об неиспользовании, а как раз о том, что на каждое исключение делать уникальный класс исключений, а то что вы советуете не использовать для отображения пользователю не означает, что их не надо делать? Поэтому я и сказал, что это накладно, т.к. на каждое уникальное исключение, которых могут быть тысячи, вы предлагали делать уникальный класс исключения? Так ли это или не так?


                    1. lexus1990 Автор
                      16.05.2023 06:59

                      Мой ответ такой: для каждой неожидаемой ситуации в системе использовать исключение (нормально если их сотня и больше). Это исключение не должно показываться пользователю. Если исключение случилось - значит проблема какая-то которую пользователь не может решить - для общения по этой проблеме используем traceId

                      Пользователю должны показываться только те ошибки (не исключения), с которыми он может сам что-то сделать (недостаточно денег - используйте другую карту / неправильные реквизиты карты - попробуйте еще раз и тд). Их не может быть 2000 (если так - то вообще что это за софт)


                      1. AlexXYZ
                        16.05.2023 06:59

                        недостаточно денег - используйте другую карту

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

                        Мне не очень понятно, почему вы считаете, что исключение, которое нельзя обработать автоматически, не должно показываться пользователю? Он же всё равно не может выполнить какую-то операцию? Что ему тогда показывать? (или я чего-то не понимаю?)


                      1. lexus1990 Автор
                        16.05.2023 06:59

                        А что вы ему покажете если один микросервис системы не смог по сети сходить в другой микросевис?

                        В лучшем случае просто техническую ошибку. И начать разбираться даже еще до звонка пользователя (триггер на ошибки если это серьезная ошибка).

                        В худшем случае дать ему стектрейс и скомпрометировать код системы.


                      1. AlexXYZ
                        16.05.2023 06:59

                        Мой ответ такой: для каждой неожидаемой ситуации в системе использовать исключение (нормально если их сотня и больше). Это исключение не должно показываться пользователю.

                        Но вот я и спрашиваю - а писать новый класс для каждого из 100-200 исключений нужно ли?


                      1. lexus1990 Автор
                        16.05.2023 06:59

                        Да


                      1. AlexXYZ
                        16.05.2023 06:59

                        Хорошо, я вас понял. Но предлагаете ли вы таким образом использовать один класс для одного случая? (В идеале).

                        Простите мне мою дотошность, я хочу понять принципиальное отличие от нумерации. Я рассматривал ваш вариант, но пришёл к выводу, что мне проще нумеровать исключения, чем строить иерархию как раз потому, что не хотел писать 100-200 и более классов исключений и столько же обработчиков. При том, что у меня сквозная нумерация и по исключениям и по предупреждающим сообщениям. Т.е. у меня уникальное число независимо от того, кто его сгенерировал - исключение или просто предупреждение в интерфейсе или вывод в лог. Соответственно формат всех чисел в исходном коде у меня начинается с символа подчёркивания и вид числа _1234 никогда не встречается в коде. Поэтому я всегда однозначно могу найти место в программе, которое генерирует сообщение вообще без контекста - исключение ли это, предупреждение или запись в логи.


                      1. AlexXYZ
                        16.05.2023 06:59

                        вид числа _1234 никогда не встречается в коде

                        Я имел в виду повторно не встречается в коде.


  1. GoodGod
    16.05.2023 06:59

    Первый пример:

    } catch (exception: TGetCommission) {

    /** Раскрываем подробности того, что ходим в сервис комиссий */

    throw CommissionServiceTimeout()

    }

    Вы можете пояснить как из TGetCommission вы получили таймаут? А может там не таймаут, может там 500 из-за бага в коде? Как вы так "кастите" TGetCommission именно в CommissionServiceTimeout?


    1. lexus1990 Автор
      16.05.2023 06:59

      Вполне вероятно. Надо смотреть как устроен трифт и когда он кидает эту ошибку. Это мешает понять мысль, которую описывает этот код?


      1. GoodGod
        16.05.2023 06:59
        -1

        Согласно чистому коду - код как раз должен быть таким чтобы не надо было смотреть как устроен трифт. А код который надо смотреть как устроен - и я могу написать.


        1. lexus1990 Автор
          16.05.2023 06:59

          Трифт - это сторонняя библиотека бинарных сообщений отправляемых по сети https://thrift.apache.org/ (аналог protobuf)


  1. mixsture
    16.05.2023 06:59
    +1

    Не следует выкидывать из функций исключения из стандартной библиотеки

    И эта практика отвратительно себя показывает в языках, где в сигнатуры функций не включается описание исключений — например, в python. Потому что теперь документация становится архикритичной.
    Я с этим сталкивался в следующем варианте: имеем функцию, которая по описанию явно использует http запрос, но в документации нет ничего похожего на характерные исключения. Функция использует пакет requests, значит будет логично предположить, что его исключения и надо перехватывать. Но… нет, потому что автор этой функции решил перехватить и вместо них бросать свои кастомные и теперь я должен изучить все возможные ветвления и вызовы его функции, чтобы найти эти кастомные — "очень занимательно".


    1. lexus1990 Автор
      16.05.2023 06:59

      Что значит "использует пакет requests, значит будет логично предположить, что его исключения и надо перехватывать"? Искать все exceptions этого пакета?

      Как по мне очень удобно если вы хотите обработать исключения - чтобы класс кидал исключения которые определены только в этом классе.


      1. mixsture
        16.05.2023 06:59

        Что значит "использует пакет requests, значит будет логично предположить, что его исключения и надо перехватывать"? Искать все exceptions этого пакета?

        Да, но их искать не надо. Этот пакет (requests) распространенный и к нему хорошая документация плюсом.


    1. Rukis
      16.05.2023 06:59

      Это вы просто знаете что функция использует http и какой пакет в добавок. А могли и не знать, так что проблема не в кастомных исключениях. Были бы исключения в каком то виде перечислены и всё было бы ок независимо от их кастомности или наличия у вас доп знаний.


      1. lexus1990 Автор
        16.05.2023 06:59

        Вы пишите на языке с проверяемыми исключениями? Если да - то вам знакомо когда весь код выглядит как сплошной catch() (слава богу еще что в новых версиях java и в kotlin можно указывать исключения в catch через | - или)


    1. GoodGod
      16.05.2023 06:59
      +1

      Понимать Дадюшку Боба очень сложно. Первый пример из статьи:

      Не раскрывай детали реализации используя исключения.

      Если обработка ошибок раскрывает реализацию — то это неправильная обработка ошибок

      Не цитата Дяди Боба, но на этом принципе построен первый пример.

      Представьте, если ваша упоминаемая http библиотека будет бросать исключение - UnableToDoNeededHttpRequest, что переводится как "НеУдалосьВыполнитьHttpЗапрос". И ты хрен поймешь в чем там проблема - DNS не нашел имя, connect на порт не удался, сервер ответил 500. Детали реализации то мы не раскрываем.

      Хорошо, но для решения проблемы есть пункт

      Название исключения надо давать настолько подробным, чтобы описание было вообще не нужно.

      Т.е. проблема решается такими исключениями, которые говорят сами за себя.

      Попробуй теперь выполнить сразу 2 правила.

      1. И не раскрыть что мы делаем DNS запрос.

      2. И сообщить, что проблема в шаге с DNS запросом.

      Сразу и не придумаешь как это реализовать.

      Это не шаблонный и понятный DnsErrorException (который нарушает оба правила, зато придумывается за 1 секунду и получается в итоге достаточно "шаблонный" квадратно-ездовой код).

      Hidden text

      Будет UnableToFindServerByName исключение.

      И не раскрывает детали реализации (только совсем верхнеуровнево), и говорящее само за себя имя исключения.

      Еще надо подумать как дать красивое название исключению.

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

      Т.е. без правил Дядюшки Боба - получится средний, шаблонный, стандартный код. Но если ты освоишь все правила Дядюшки Боба, то получится шедевр. Но если не освоишь - получится наоборот - говнокод.


      1. mixsture
        16.05.2023 06:59

        Но смысловая нагрузка UnableToFindServerByName ровно такая же, как и DnsErrorException. Более того, конечный пользователь уже знаком с термином DNS, а вы вместо него вставляете обобщенную конструкцию, теперь пользователю нужно догадаться, что эта конструкция развернется в реализацию (DNS). Что уже сложнее и часть пользователей отвалится на этом, что сократит обратную связь.
        Более того, подозреваю, что обобщенная конструкция и не предусматривает какой-либо другой реализации, кроме DNS и скорее всего получилась лишним усложнением, от которого вы не получите никаких выгод.


        Т.е. без правил Дядюшки Боба — получится средний, шаблонный, стандартный код. Но если ты освоишь все правила Дядюшки Боба, то получится шедевр. Но если не освоишь — получится наоборот — говнокод.

        И тогда, я бы сказал, что лучше пользоваться средним шаблонным кодом ровно во всех случаях, когда это возможно. А методы дядюшки Боба с хорошим шансом выстрелить себе в ногу оставить лишь для случаев, когда средний шаблонный код ну совсем не подходит.


        1. GoodGod
          16.05.2023 06:59

          Т.е. без правил Дядюшки Боба — получится средний, шаблонный, стандартный код. Но если ты освоишь все правила Дядюшки Боба, то получится шедевр. Но если не освоишь — получится наоборот — говнокод.

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


      1. lexus1990 Автор
        16.05.2023 06:59

        Представьте, если ваша упоминаемая http библиотека будет бросать исключение - UnableToDoNeededHttpRequest, что переводится как "НеУдалосьВыполнитьHttpЗапрос". И ты хрен поймешь в чем там проблема - DNS не нашел имя, connect на порт не удался, сервер ответил 500. Детали реализации то мы не раскрываем.

        Пойму если положить изначальный exception в cause своего exception


      1. lexus1990 Автор
        16.05.2023 06:59

        Сразу и не придумаешь как это реализовать.

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

        Если это клиент который отправляет запрос и его вызывает сервис - то можно подробно описать вид ошибки ошибка DNS или что-то подобное.

        Если это сервис, который вызывает другой сервис в пайплайне - то подробность о том, что мы не смогли рассчитать комиссию по платежу из-за DNS не важна для читающего код (для разбора проблем это важно - логируем cause)


  1. mixsture
    16.05.2023 06:59
    +2

    Дядя Боб говорит, что размеры файлов должны быть от 30 до 100 строк.

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


  1. ReadOnlySadUser
    16.05.2023 06:59

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

    Ну, будут комментарии с пояснениями пропускать и что с того? Я их писал не для чтения каждый день, а для разбирательств в тяжёлые времена.


    1. lexus1990 Автор
      16.05.2023 06:59

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


      1. ReadOnlySadUser
        16.05.2023 06:59

        Так не надо писать комментарии к реализации и уж тем более как пояснения к строчкам кода.

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

        Или, например, если код содержит неочевидные расчёты, оставить ссылку на статью/статьи откуда это взято или описать всю логику этих расчетов.

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

        Бывают места, которые очень хочется переписать, но это мягко говоря невозможно. Поэтому, если оставить комментарий вида "я знаю, что тебе очень хочется это переписать, но имей ввиду, что это сломает <список вещей >", я думаю это может сэкономить кучу времени следующему желающему.

        В общем, понятное дело, что комментарии вида "запрашиваю список товаров из БД" бессмысленны) Это очевидно из кода)


        1. lexus1990 Автор
          16.05.2023 06:59

          Вы не обратили внимание что я написал выше про то, что устаревание важнее (ИМХО)?

          Про термины бизнес задачи - все в названия и в код (тут DDD еще проклевывается - общий язык бизнеса и разработки)

          Ссылку на статью / новость - кладем в тикет систему и пишем в описание коммита (коммит могут перетереть а коммент оставить)