CI, кодстайл и TDD: обзор практик для повышения качества кода12.01.2023 12:01

Я видел не во сне, а наяву атакующие корабли, пылающие под четырьмя вложенными if-else, и лучи CI с кучей сканирований у ворот Тангейзера, вызывающие лютую боль разработчиков. Меня зовут Максим, и я техлид в Газпромбанке.

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

Моё понимание чистого кода: это исходники, каждый файл которых, например, связан с одной зоной ответственности — доменом; методы и классы в нём лаконичны, не расползаются на пять вложенных циклов с ветвлениями. Качественный же код вбирает в себя свойства чистого кода, дополняет их вниманием к техническому исполнению и тестируемостью классов по отдельности.

В примерах будет код на Kotlin, однако это не означает, что практики нельзя применить к другим языкам. Руководство подойдёт даже для YAML-программистов. Попробуйте после прочтения материала посмотреть на какой-нибудь код ролей Ansible, и наверняка обнаружите простор для улучшения.

Код для людей

1. Пишите код и тесты так, чтобы они были главной документацией разработчика

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

Как достичь такого «самодокументированного» кода? Начните с постепенного введения Type-Driven Development и тестов бизнес-логики. В случае с Kotlin: делайте sealed class и в нём описывайте все возможные результаты выполнения метода через классы и object«ы.

Вот пример:

data class SubscriberDataUpdate private constructor(
   private val dataUpdate: DataUpdate,
   private val subscriber: Subscriber
) {
 
   val subscriberId: SubscriberId = subscriber.subscriberId
   val dataUpdateId: DataUpdateId = dataUpdate.dataUpdateId
 
   fun prepareUpdateRequest(): Result =
       when (isUpdateRequired()) {
           true -> createSubscriberUpdateRequest()
           else -> failNoUpdateRequired()
       }
 
   private fun failNoUpdateRequired(): Result =
       Result.failure(RuntimeException("No Update Required"))
 
   private fun createSubscriberUpdateRequest()
           : Result =
       Result.success(
           SubscriberUpdateRequest(
               subscriberId.value,
               dataUpdate.msisdn.value,
               dataUpdate.mobileRegionId.value,
               this
           )
       )
 
   private fun isUpdateRequired(): Boolean =
       subscriber.mobileRegionId != dataUpdate.mobileRegionId

    companion object {
       fun emerge(
           dataUpdate: DataUpdate,
           subscriberResult: Result
       ): Result =
           subscriberResult.map {
               SubscriberDataUpdate(dataUpdate, it)
           }
   }
 
}

Type Driven в лучах ValueObject

data class SubscriberId
private constructor(
   override val value: String
) : ValueObject {
   companion object {
       fun emerge(subscriberId: String)
               : Result =
           when (isStringConsists9Digits(subscriberId)) {
               true -> Result.success(SubscriberId(subscriberId))
               else -> Result.failure(IllegalArgumentException("Subscriber Id consists of numbers maximum length 9"))
           }

       private val isStringConsist9Digits = "^\\d{1,6}\$".toRegex()
       private fun isStringConsists9Digits(value: String) =
           isStringConsist9Digits.matches(value)
   }
}

TESTS

Суть в этом — класс сам себя тестирует:

nternal class SubscriberIdTest {
   @Test
   fun success() {
       val sut = SubscriberId.emerge("888")
       assertThat(sut.isSuccess).isTrue
       assertThat(sut.getOrThrow().value).isEqualTo("888")
   }

   @Test
   fun failWithMoreThen9Digits() {
       val sut = SubscriberId.emerge("1234567890")
       assertThat(sut.isFailure).isTrue
       assertThat(sut.exceptionOrNull()!!.message).isEqualTo("Subscriber Id consists of numbers maximum length 9")
   }

   @Test
   fun failWithWrongFormat() {
       val sut = SubscriberId.emerge("L124S")
       assertThat(sut.isFailure).isTrue
       assertThat(sut.exceptionOrNull()!!.message).isEqualTo("Subscriber Id consists of numbers maximum length 9")
   }

   @Test
   fun failWithWrongFormat2() {
       val sut = SubscriberId.emerge("11aa")
       assertThat(sut.isFailure).isTrue
       assertThat(sut.exceptionOrNull()!!.message).isEqualTo("Subscriber Id consists of numbers maximum length 9")
   }

   @Test
   fun failWithEmpty() {
       val sut = SubscriberId.emerge("")
       assertThat(sut.isFailure).isTrue
       assertThat(sut.exceptionOrNull()!!.message).isEqualTo("Subscriber Id consists of numbers maximum length 9")
   }

}

Нюансы

• Внедрение Type-Driven Development сопряжено с пусть и небольшими, но затратами.

• Требует перемен в складе мышления.

• Не совсем понятно, что в Type-Driven Development делать с исключениями. Но мы к этому ещё вернёмся.

1. Логи и мониторинг в помощь инженерам

Что лучше — писать логи в какой-то файл где-то на файловой системе или отправлять их в stdout? Отправлять логи строками в Elasticsearch или потратить время на изучение distributed tracing и структурированных логов? Спроектированный с умом мониторинг сокращает время обратной связи и упрощает поиск причины неполадок там, где нет дебаггера под рукой.

Как можно внедрить логи с метриками на примере незамысловатого класса-прослойки для чтения файла. Подключить сбор метрики: alphaMetrica (productId, route).

В коде это будет выглядеть так:

fun dataUpdateProcess(unvalidatedUpdateRequest: UnvalidatedDataUpdateRequest)
       : Mono> =
   Mono.just(ValidatedDataUpdateRequest.emerge(unvalidatedUpdateRequest))
       .flatMap { findDataWithUpdates(it) }
       .flatMap { findSubscriberForUpdate(it) }
       .flatMap { prepareSubscriberUpdateRequest(it) }
       .flatMap { updateSubscriber(it) }
       .flatMap { sendMetrica(it) }

Всё должно быть прозрачно, очевидно и настраиваемо.

Особенности:

• Делать логи надо такими же читаемыми, как и код, т. е. избегать println (»123»).

• Метрики лучше готовить вместе с инженерами эксплуатации, иначе можно по незнанию включить в них персональные данные и просто чувствительную информацию пользователей, например токены или пароли.

• Или включить в label-метрики URL запроса и таким образом размножить их в Prometheus. В любом из двух случаев вы получите по шапке от соседних команд или руководства.
  

1. DSL в бизнес-логике: нееш подумой

Доменно-специфичные языки появились сравнительно давно, один из первых популяризаторов DSL — это язык Groovy. С помощью DSL-конструкций можно элегантно и декларативно решать разные задачи, но есть и подводные камни в виде оверхеда лямбд или сложностей проектирования. И да, если вы пытаетесь лечить бойлерплейт в бизнес-логике приложения с помощью DSL или иных декларативных решений, например data-классами в Python (которые с type hints), дважды подумайте. 

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

1. Синтаксические возможности языка и их польза для code quality

Как использовать фичи ЯП и не нарываться на ошибки или code smell? Говоря опять про Kotlin, функции-расширения — это очень здорово, мы в Газпромбанке ими активно пользуемся. И идея с композицией вместо наследования в data-классах мне нравится — всё как Джошуа Блох советует.

Но у каждого языка есть и свои сложные синтаксические решения. Inline-методы с noinline- и crossinline-аргументами тому пример. Или ООП в Python, свойства и геттеры-сеттеры. Как и с DSL, если дело дошло до синтаксических приёмов в бизнес-логике, то остановитесь и задумайтесь, как вы оказались в таком положении.

Как можно делать:

fun  Result.Companion.zip(a: Result, b: Result, c: Result)
       : Result> =
   if (sequenceOf(a, b, c).none { it.isFailure })
       Result.success(Tuples.of(a.getOrThrow(), b.getOrThrow(), c.getOrThrow()))
   else
       Result.failure(sequenceOf(a, b, c).first { it.isFailure }.exceptionOrNull()!!)

fun dataUpdateProcess(unvalidatedUpdateRequest: UnvalidatedDataUpdateRequest)
       : Mono> =
   Mono.just(ValidatedDataUpdateRequest.emerge(unvalidatedUpdateRequest))
       .flatMap { findDataWithUpdates(it) }
       .flatMap { findSubscriberForUpdate(it) }
       .flatMap { prepareSubscriberUpdateRequest(it) }
       .flatMap { updateSubscriber(it) }

rest:
 services:
   subscribers:
     url: http://localhost:8080 #wireMock me local
     updates: /api/v1/subscribers/updates/{dataUpdateId}
     subscribers: /api/v1/subscribers/{subscriberId}

rest:
 services:
   subscribers:
     url: http://localhost:${wiremock.server.port} #wireMock me local

wiremock:
 server:
   port: 4567