There are 999 reasons to become levi niner. Find yours at levi9.com/jobs
×Закрыть

Меньше, но лучше: как повысить качество кода

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

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

Лучше меньше

Возьмем вот такой код из нашего открытого проекта на GitHub, который ребята из R&D Terrasoft развивают в свободное время. Задача этого фрагмента: получив на вход URL и сериализованные данные для запроса, выполнить HTTP-вызов сервиса, скрыв от клиента весь транспортный слой.

Код содержит всего 17 строк. Теперь давайте посмотрим на один из структурных критериев сложности функции — количество отступов. На строке 12 мы видим вложенность 3-го уровня. Если вспомнить, что пишут о таких функциях в книге «Чистый код», можно задаться целью изменить код на более простой. Вот такой, например:

Конечно, приведенный пример — не единственный возможный в данном случае, но и он показывает одну интересную особенность: без изменения логики работы кода теперь вместо 17 строк кода мы имеем 26.

Как же так получается, что мы используем практику упрощения кода, а в итоге получаем его структурное усложнение?

Давайте разберемся с этим явлением подробнее и вспомним правило KISS: в коде мы использовали конструкцию языка C# - Extensions, добавили отдельный класс и ввели не общеизвестную функцию в наш проект. Согласитесь, это явно немного сложнее, чем просто написать вызов нескольких framework native методов, и это можно считать не очень хорошим решением. Но с другой стороны, мы упростили чтение изначальной функции, уменьшив её вложенность, что можно засчитать в плюс.

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

Как получилось, что пример на 17 строк лучше примера на 26, несмотря на то, что он содержит «запахи»? Теперь давайте посмотрим не только на функцию ExecutePostRequest, но и на дополняющую её функцию ExecuteGetRequest. Её синтаксис такой:

Как видно, в ней такая же операция обработки ответа от сервера. А теперь давайте вспомним ещё одно правило — YAGNI, или «Выделяйте функцию тогда, когда она вам понадобилась второй раз», и ещё вспомним известную аббревиатуру DRY. Теперь, если мы выделим код обработки ответа в отдельную функцию, получим вот такой результат:

Я выделил одинаковыми цветами логически идентичные блоки до и после рефакторинга. Если внимательнее посмотреть на пример «после», мы увидим, что дублирование кода уменьшилось, синтаксис стал более коротким.

Также заметно, что код вспомогательной функции отличается от первого примера и все еще содержит тройной отступ. Я бы хотел акцентировать внимание на том, что при выборе того, какой именно код выделять отдельно, учитывайте, что итоговый вариант дает более стройную форму кода, с одной стороны. С другой же — не вносит значительной сложности в понимание кода, поскольку «проблемный» участок занимает всего 9 строк и локализован в рамках одной функции. Ну и, конечно, хочу отметить, что количество строк кода в ходе изменений уменьшилось.

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

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

  • создание HTTP-запроса;
  • добавление к запросу данных;
  • получение ответа и запись его в файл.

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

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

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

Поэтому логику записи данных в запрос сразу вынесем в функцию создания запроса:

Теперь у нас в функции уменьшилось количество действий:

  • создание HTTP-запроса с данными;
  • получение ответа и запись его в файл.

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

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

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

Однако давайте вернемся к нашему примеру и посмотрим, во что превратилась исходная функция:

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

Если до объединения формирования запроса и записи в него данных мы видим структурную разницу методов Get и Post, то уже после мы видим одинаковую структурную сложность этих функций, и это хорошо. После этих операций общий объем файла уменьшился на 17 строк, чем подтверждается принцип «улучшения кода уменьшают его объем». После более детального рассмотрения полученного кода заметна алгоритмическая схожесть, что уже наталкивает на мысль о применении DRY для дальнейшего улучшения. Подробнее о том, как развивать это код, и о других критериях оценки кода мы поговорим в рамках следующей статьи из этого цикла.

Выводы

Рассмотренные критерии по минимальности строк общего кода и одинаковой сложности внутренних структур не являются «научными», с одной стороны. Но, с другой — они позволяют легко найти места для оптимизаций.

Вот несколько моментов, на которые стоит обратить внимание в процессе написания кода:

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

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

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

LinkedIn

54 комментария

Подписаться на комментарииОтписаться от комментариев Комментарии могут оставлять только пользователи с подтвержденными аккаунтами.

Вопрос можно? Что за язык показывается в примерах. Как-то непонятен немного контекст.

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

Именно этим разработчики и занимаются большую часть своего времени :)

Мне кажется, проблема подобных статей про «сложность», в том, что примеры описываемые, они не сложные, ни были и после статьи проще не становятся. Как борьба с мельницами.

Но реальная сложность существует в природе, особенно когда к тебе приходит старый код, который писал не ты. И как раз в этот момент, появляется интерес узнать, а были ли тесты? Или скорее «А как оно вообще @#$^ работает?». И начинаешь сам тесты писать, чтоб покрыть все кейсы и тем самым понять, как именно (если это хоть как-то возможно).
Правда, если тесты были, обычно код еще и напичкан всякими premature generalization вместе с банальным вездесущим boilerplate и в целом такой код раза в 3 больше по количеству строк, что переварить возможно и «сложнее», чем тот что без тестов.

Объективности и научности пока в данном деле, не много, хотя Visual Studio Ultimate, имеет крутую tool «Analyze > Calculate Code Metrics», позволяющая охладить твой пыл по переписыванию всего проекта с нуля и указыванию горячих мест.

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

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

Однозначно процесс непростой и научится ему за один вечер вряд ли получится. Однако если подойти к этому вопросу системно то можно достичь очень хороших результатов и файлы даже на 2000 строк не будут вызывать тоску. Вы начнете обращать внимание на то что написано внутри этих строк и иногда задача на 100 строк будет более сложной чем на 1000. Например изначальный фрагмент кода который лег в основу статьи был немногим больше 700 строк, к окончанию строки код стал занимать меньше 200 строк. Чтобы этот процесс обучения упростить существует довольно четкие алгоритмы действий которые не зависят от того что и как написал код который вы собрались менять, своим коллегам рекомендую ознакомится с материалами на refactoring guru. Рефакторинг очень похож на футбол — со стороны все просто берешь мяч и пробуешь его забить в ворота ногой, но если начать тренироваться с професианальной командой и тренером, вы посмотрите на него абсолютно с другой стороны — в нем будет теория, домашние заготовки стандартных ситуаций, стратегия на одну игру или серию игр, много практики, спортивные доктора, правильное питание и много того что не бросается в глаза.

Если проблема со вложенностью при использовании using, у StreamWriter есть параметр leaveOpen, т.е. писатель может сам закрыть стрим.

А еще можно не делать отспупы для вложенных using:

using(var response = ...)
using(var dataStream = ...)
using(var writer = ...) {
    writer.Write(...);
}

C# 8. Прикольно, еще не перешли на него.

Так и есть) По крайней мере из тех у кого ci есть

Игорь, вы затронули интересную тему. В последнее время синтаксический сахар очень благоприятно влияет на структуру кода и этого нельзя не признать. Мы видим в новых редакциях C# регулярное его появление. Мне очень интересно, что будет вместе с dotnet 5, появятся ли кардинально новые структуры в платформе или нет. Например, на прошлой конференции IT Arena вскользь поднимался вопрос о поколении языков C# и Java. Я бы очень хотел увидеть level up в этом вопросе.

Именно, если посмотреть на скорости развития и востребованности low code платформ в бизнесе, то можно увидеть как меняются требования рынка. Вспомните сайты визитки которые раньше писали студенты, и сравните их с современными конструкторами или площадками продаж/коммуникаций и др. Для использования которых уже не надо решать тривиальные задачи через код. При этом если посмотреть на сложность задач которые решаются в разработке систем для AI, СУБД, ML и других — то она растёт с каждым годом и предьявляет новые требования к инструментарию. А код это как раз и есть наш инструмент для решения задач.

Андрей, привет. Рад тебя видеть 🙂 Да, такой вариант дальнейшего развития возможен.
В каждом ответе я стараюсь ответить немного более развернуто на особо интересные моменты. Давайте посмотрим на использование конструкции using. Если поискать статьи на эту тему, мы увидим очень много интересного и глубоко системного (с точки зрения .net-а) материала, понимание которого на нашем уровне абстракции коде вроде-как не нужно. Но нам приходится об этом знать и, используя выручалочку using-a, влиять тем самым на структуру кода
по одной простой причине — проблема освобождения unmanaged ресурсов не решена на уровне платформы полностью и часть решение перекладывается на конечный код.

Если на метод сложно написать тест — код не очень.

Андрей, это высказывание прямо в точку. Я, правда, немного по-другому его формулирую для себя — использование API должно быть простым (без уточнения места использования из теста или другого кода). О конкретных примерах и техниках достижения такого результата будет в следующей статье.

Тут нюанс. Вызов метода может быть весьма простым. И это может быть ок для внешних consumer-ов, пока не придётся править его бизнес-логику.

В одном докладов по построению микросервисной архитектуры (ссылку могу попробовать поискать, если нужно) частично был дан совет, как сделать просто — пишите файлы по 40-50 срок и тогда вы в них очень легко будете менять логику :)

Не всегда.
Классический пример. Простой и понятный код, только нетестируемый и поэтому с багом.

public void congrats() {
    Date today = new Date();
    if (today.getMonth() == 1 && today.getDay() == 1) {
        System.out.println("Happy New Year!");
     }
}

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

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

Странное утверждение. Как можно «легко менять и поддерживать» без тестов?!
Пример выше — простой, но плохой код.

инструменты мутационного тестирования

Статья же про качественный код? Имхо, если мутационное тестирование нужно, вряд ли это хороший код.

Постараюсь немного детализировать свои мысли:
1. «Hello world!» на любом языке пишется без теста, при этом код будет работать отлично и правильно. А если возникнет задача заменить текст то с ней без труда справится junior — наглядная иллюстрация предположения что простой код легко менять и поддерживать. В данном случае речь о качестве кода не идёт. Основной акцент именно на простоте.
2. Мутационное тестирование ничего не говорит о качестве кода, оно показывает качество тестов. Я сделал акцент не на самом тестировании а именно на инструментах этого подхода. Потому что код использующий статику или переменные среды покрывается не самыми простыми приемами.

Метод отсылает сообщение в RabbitMQ. Протестировали, все работает, дальше что? Как протестировать воркер, который берет сообщение из RabbitMQ и делает фоновую работу? Причем протестировать что он в проде будет получать сообщения от кролика и их обрабатывать, а не что теоретически оно работает с мок-апами.

Это уже уровень не unit, а интеграционного тестирования, который не покажет качество кода/дизайна системы.

Ок, но юнит-тест ведь тоже покажет только некоторые аспекты качества. Может метод тестироваться хорошо, а внутри 1000 строк кода

Про 1000 строк очень уместное замечание. Используемые критерии должны дополнять друг друга, главное — заранее обозначить цели, которые эти критерии помогают достичь и тогда получится действительно хороший Quality Gate, который будет помогать разработчикам, а не вносить дополнительную сложность в процесс.

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

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

а с каких пор юнит тесты показывают качество?
юнит тесты только показывают как код работает в контролируемумом окружении

в контролируемумом окружении

вот возможность добиться «контролируемого окружения» (подмены зависимостей) и есть одна из характеристик качества и достаточно важная.

может раньше так и было, а сейчас есть куча фреймворков которые позволяют замокать любую зависимость, а если это managed язык так даже ничего в коде не нужно менять, в языках которые компилируется в native возможность подмены зависимостей приводит к увеличению сущностей в системе, блоатед коду, и к субоптимальной производительности

Дмитрий, вот не поверю что не ясно о чём речь.

Если в избитой теории, то Dependency Inversion Principle.

А на практике не очень гуд код для тестировния:

public boolean recalcDepositPercentage(User user)
{
    Logger logger = new Logger;
    NotifyManager notify = new NotifyManager;
    ....
    какая-то логика
}
где logger куда-то в базу логирует, а NotifyManager в RabbitMQ пишет для дальнейшей отправки нотифов пользователю.

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

да и от того что сделаете
public boolean recalcDepositPercentage(User user, ILogger logger, INotifyManager notify)
код не станет лучше, станет меньше инвариантов, нужно будет как то жить с тем что вместо logger могу и ссылку на ноль передать.

про native я вообще молчу, ведь там кроме как «объекты по ссылке» могут быть «объекты по значению», поскольку интерфейс нельзя передать по значению то использование Dependency Inversion приводит к отказу от объектов по значению

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

ИМХО, здесь какое-то недопонимание.

В чём противоречие? Фреймворком так фреймворком (точнее, даже в подавляющем большинстве случаев фреймворком). Легко замокать — хорошее качество (с этой точки зрения)/слабая связность (скорее всего), сложно — плохое качество (с этой точки зрения)/сильная связность (скорее всего). Всё ж просто.

код не станет лучше, станет меньше инвариантов, нужно будет как то жить с тем что вместо logger могу и ссылку на ноль передать.

Так с DI фреймворком Вы туда руками ничего и не передаёте.

Легко замокать — хорошее качество (с этой точки зрения)/слабая связность (скорее всего), сложно — плохое качество (с этой точки зрения)/сильная связность (скорее всего)

на современном уровне развития легко замокать почти всегда. следуя этой логике почти всегда хорошее качество

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

Просто не согласен). Смотря, что означает

на современном уровне развития

Т.к. это больше звучит как «если писать хороший код, то будет хороший код»). Но далеко не везде он хороший, и ещё реже используются DI фреймворки (а без них замокать не то что даже тяжело, а показатель, что нужно сначала отрефакторить этот самый код).

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

Поднять оба сервиса и RabbitMQ в докере, например. Но это уже другой уровень тестов. Подозреваю, Андрей говорил про Unit тесты.

Геннадий, вопрос тестирования и уверенности, что всё будет работать у пользователя терзает всех, кто в IT-мире релизит продукты. 🙂
Единого ответа тут достичь тяжело, но для себя, в данный момент, выбрал вот-такой подход ->
Каждый слой ваших тестов и метрик служит для конкретной цели:
— unit тесты проверяют работу подготовки и трансформации данных/состояния
— integration тесты проверяют коммуникацию между физически разными компонентами (сервисами, СУБД, API и др)
— метрики production-a показывают вам, что приложение действительно работает у конечного пользователя
Поэтому стараюсь не смешивать эти вопросы на одном слое.

А что за метрики production-а такие?
Интересно как вы меряете что приложение действительно работает у пользователя.

Мы собираем с наших сервисов технические (RAM\CPU\интенсивность запросов etc) и бизнес метрики (скорость бизнес операций, их количество, размер необработанных объектов и т.д в зависимости от назначения сервиса), и сравниваем их текущие показатели со средним значением. Для каждого сервиса вырабатываем порог на который реагируем и внедряем процедуры контроля и реакции в процессы управления и поддержки сервисами. Очень похожий подход был представлен на highload конференции в Москве ребятами из Авито, часть доклада была посвящена именно мониторингу (по моему это за 2018 год, запись доклада есть в online доступе). Основной принцип — реакция на отклонения от средних величин по критичным параметрам.

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

На этот код легко написать тест? На любой из них?

Если рассматривать уровень интеграционных тестов, ответ будет — да. Код теста будет очень простым. Создание клиента, вызов метода по url и проверка ответа. Главное правильно определить назначении и форму самого теста.

Стоило назвать — «Как из очень плохого кода — получить просто хуже среднего».
Просто мнение)

Валерий, я с вами полностью согласен, в этом как раз и цель рефакторинга — цепочка итеративных изменений, которые делают код лучше. И статья направлена не на разбор конкретного кода, а на приёмы, которые помогают двигаться по этому пути. Для примера, возьмите листинг книги «Чистый код», где разбирается принцип SRP для функции. Вы увидите схожую ситуацию. Мы живем не в идеальном мире и в процессе работы сталкиваемся с различным кодом: и хорошим, и плохим. Наша задача как разработчиков уметь с ним работать так, чтобы он становился лучше и удобнее с каждой заливкой.

Каждый, кто стремится к совершенству

Такой каждый, мог бы опечатки в коде не публиковать.
prntscr.com/nrbdf2

Игорь, спасибо за замечание. Действительно в первой реализации была опечатка, я специально проверил наш репозиторий, там нет. Чтобы быть последовательным в улучшении не только на бумаге но и в делах скрин заменил 🙂 В DDD есть такой приём — вы создаете словарь терминов вашего домена и потом его можно использовать для проверки полноты реализации требований и Name convetion-a. Я правда этот подход не практикую, но если вы пользуетесь инструментами которые помогают не держать в голове лексические проверки буду признателен за ссылочку на описание.

IDE от JetBrains умеют показывать опечатки. Правда задалбывает, протому что например knex — это название библиотеки, а никакая не опечатка. Но можно добавить в словарь (уровня проекта)

Чувствуемую что появился ещё один повод познакомится с Rider-ом поближе, спасибо за наводку.

Подписаться на комментарии