Чистый код. Часть 3
Привет! Этим постом я завершаю цикл из конспектов видеолекций Дяди Боба про чистый код. Вот ссылки на предыдущие:
Сегодня обсудим исключения, классы и все, что не вошло в прошлые разделы.
Обработка исключений
Не раскрывайте реализацию
Майкл Физерс (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. Когда меня спрашивают — можешь в двух словах объяснить зачем нужен чистый код — я обычно скидываю этот мем
