От переводчика: Опубликовали для вас статью Камила Лелонека о значении читабельности кода и «эмпатии программистов».
Вы когда-либо задумывались, кто будет просматривать ваш код? Насколько сложным он может оказаться для других? Пытались определить его читабельность??
«Любой дурак может написать код, который будет понятен машине. Но вот код, который понятен еще и людям, пишут лишь хорошие программисты», — Мартин Фаулер.Время от времени, когда я вижу некоторые сниппеты кода, я теряю веру в существование эмпатии среди программистов. Вы должны понимать, о чем я говорю, — ведь каждый из нас сталкивался с кодом, который был написан просто ужасно и являлся практически нечитаемым.
Skillbox рекомендует: Двухлетний практический курс «Я – Веб-разработчик PRO».Недавно я увидел нечто вроде этого:
Напоминаем: для всех читателей «Хабра» — скидка 10 000 рублей при записи на любой курс Skillbox по промокоду «Хабр».
defmodule Util.Combinators do
def then(a, b) do
fn data -> b.(a.(data)) end
end
def a ~> b, do: a |> then(b)
end
В принципе, здесь все хорошо: возможно, у кого-то просто работает фантазия или у автора кода солидное математическое образование. Я не хотел переписывать этот код, но подсознательно мне казалось, что здесь что-то не так. «Должен быть способ сделать его лучше, сформулировать по-другому. Посмотрю, как все устроено», — так я думал. Довольно быстро я нашел вот что:
import Util.{Reset, Combinators}
# ...
conn = conn!()
Benchee.run(
# ...
time: 40,
warmup: 10,
inputs: inputs,
before_scenario: do_reset!(conn) ~> init,
formatter_options: %{console: %{extended_statistics: true}}
)
Хммм, похоже, что не только ~> импортируется, но также функции conn!/0 и do_reset!/1. Ок, давайте взглянем на модуль theReset:
defmodule Util.Reset do
alias EventStore.{Config, Storage.Initializer}
def conn! do
{:ok, conn} = Config.parsed() |> Config.default_postgrex_opts() |> Postgrex.start_link()
conn
end
def do_reset!(conn) do
fn data ->
Initializer.reset!(conn)
data
end
end
end
Что касается conn!, то есть парочка способов сделать этот участок проще. Тем не менее останавливаться на этом моменте нет смысла. Я лучше сосредоточусь на do_reset!/1. Эта функция возвращает функцию, которая возвращает аргумент и выполняет сброс для Initializer; да и само имя у нее довольно сложное.
Я решил выполнить реверс-инжиниринг кода. Согласно документации benchee, before_scenario принимает ввод сценария в качестве аргумента. Возвратное значение становится входным для следующих шагов. Вот что автор, вероятно, имел в виду:
- Инициализация соединения Postgrex.
- Reset EventStore.
- Использование входных значений в качестве элементов конфигурации (речь о количестве аккаунтов).
- Подготовка данных для тестов (т.е. создание пользователей и вход в приложение).
- Использование бенчмарков.
В общем-то, все понятно, такой код легко написать. Отмечу, что в рефакторинге я не буду показывать или изменять функцию init, здесь это не очень важно.
Первый шаг — явное использование алиасинга вместо имплицированного импорта. Мне никогда не нравились «магические» функции, появляющиеся в моем коде, даже при условии, что Ecto.Query делает запросы элегантными. Теперь наш модуль Connection выглядит следующим образом:
defmodule Benchmarks.Util.Connection do
alias EventStore.{Config, Storage.Initializer}
def init! do
with {:ok, conn} =
Config.parsed()
|> Config.default_postgrex_opts()
|> Postgrex.start_link() do
conn
end
end
def reset!(conn),
do: Initializer.reset!(conn)
end
Далее я решил написать «крючок», как это предлагается в документации:
before_scenario: fn inputs -> inputs end
Все, что осталось сделать, — подготовить данные. Финальный результат таков:
alias Benchmarks.Util.Connection
conn = Connection.init!()
# ...
Benchee.run(
inputs: inputs,
before_scenario: fn inputs ->
Connection.reset!(conn)
init.(inputs)
end,
formatter_options: %{console: %{extended_statistics: true}}
)
Connection.reset!(conn)
Идеален ли этот код? Наверное, еще нет. Но проще ли он для понимания? Я надеюсь на это. Можно ли было так сделать сразу? Определенно да.
В чем проблема?
Когда предложил решение автору, я услышал: «Круто». Впрочем, на большее я и не рассчитывал.
Проблема в том, что первичный код работал. Единственное, что привело меня к мысли о необходимости рефакторинга, — слишком сложная структура кода и его низкая читабельность.
Чтобы убедить других разработчиков делать свой код читабельным, нужно нечто убедительное. И аргумент «я решил переделать твой код, поскольку он малопонятен», не будет воспринят, ответом будет «значит, ты просто плохой разработчик, что я могу поделать ?\_(?)_/?».
Это (не) проблема менеджмента
Ни у кого не вызывает удивления, что бизнес ожидает от сотрудников результатов. И чем быстрее они будут получены, тем лучше. Менеджеры обычно оценивают программное обеспечение и его написание с точки зрения дедлайнов, бюджета, скорости. Я не говорю, что это плохо, просто стараюсь объяснить, почему появляется не слишком качественный код, который «просто работает». Дело в том, что менеджеров мало интересуют красота и читабельность, им необходимы продажи, низкие затраты и быстрые результаты.
Когда на программистов давят, они ищут выход. Чаще всего решением становится создание «рабочего кода», в котором может быть куча «костылей». Он создается без мысли о том, что код необходимо обслуживать в будущем. А элегантный код очень сложно писать быстро. Не имеет значения, насколько вы опытный программист, — когда работа ведется в условиях цейтнота, о красоте не думает никто.
Решить эту ситуацию можно, лишь убедив менеджеров, что плохой (хотя и рабочий) код грозит увеличением затрат на его обслуживание уже в ближайшем будущем. Исправление багов и добавление новых функций, рецензирование, написание технической документации — в случае некачественного кода на все это нужно гораздо больше времени, чем в ситуации, когда он элегантен и рационален.
Важная роль эмпатии
Если вы разрабатываете программное обеспечение, то понимаете, что оно предназначено для других людей, а разработка ведется в команде. И эмпатия здесь очень важна.
Ваш код — это своеобразная форма коммуникации. В процессе разработки архитектуры будущего ПО нужно думать и о тех, кто будет взаимодействовать с вашим кодом.
«Эмпатия программистов» помогает создать более чистый и рациональный код, даже когда сроки поджимают, а менеджер постоянно «давит». Она помогает понять, каково это — разбирать чужой нечитаемый код, который чрезвычайно сложен для понимания.
В качестве вывода
Недавно я написал код на Elixir:
result = calculate_results()
Connection.close(conn)
result
Затем я подумал о методе Ruby tap, который позволяет переписать этот код:
calculate_result().tap do
Connection.close(conn)
end
ИМХО, было бы лучше сделать это без промежуточной переменной result. Я обдумал, как это можно сделать, и пришел к следующему выводу:
with result = calculate_results(),
Connection.close(conn),
do: result
Результат будет таким же. Но вот использование with может вызвать у кого-то, изучающего этот код, затруднения, поскольку в обычном случае with используется иначе.
Поэтому я решил оставить все как было, чтобы не делать лучше себе в ущерб другим. Я прошу и вас поступать таким же образом, не усложняя код сверх меры. Прежде чем вводить некую экзотичную функцию или переменную, вспомните, что ваши коллеги могут просто не понять их при ревью.
В целом, рекомендую использовать следующий принцип: «Когда вы пишете ВАШ код, думайте о ДРУГИХ».
- Онлайн-курс «Профессия frontend-разработчик»
- Практический курс «Мобильный разработчик PRO».
- Практический годовой курс «PHP-разработчик с 0 до PRO».
Комментарии (13)
nckma
20.11.2018 14:47Решить эту ситуацию можно, лишь убедив менеджеров, что плохой (хотя и рабочий) код грозит увеличением затрат на его обслуживание уже в ближайшем будущем.
Если Вы скажете менеджеру, что код который пишет команда плохой, то вся команда проклянет Вас. А так и код плохой и вся команда убеждает менеджера, что код идеален, просто сложный, но ведь современный продукт всегда сложный, поэтому трудно обслуживать и т.д…
BalinTomsk
20.11.2018 20:43Мне очень сильно помогло, то что в проекте практиковалось полное покрытие юнит тестами. То есть 2-5 тестов на каждую строчку кода.
Очень дисциплинирует и заставляет писать тестируемый код.
Pre-review не позволит попать коду в репозиторий, а post-review в рабочий проект.
moonster
21.11.2018 02:43Вот список практик моего текущего проекта, которые в разной степени направлены на поддержание понятности кода.
- Придерживаться общего для проекта стиля. Да, его нужно выработать, задокументировать и автоматизировать контроль.
- Давать осмысленные названия переменным, классам, функциям, пакетам.
- Выровнять уровни абстракции. В том числе в рамках одного файла.
- Стремиться сохранить линейный формат кода на каждом уровне абстракции. Считайте, что вам удалось, если возможно сделать близкое к реальности предположение о том, что делает функция, прочитав её всего лишь раз сверху вниз.
- Избегать повторяющихся проверок. В идеале — вытеснить неопределенность в данных на максимально высокий уровень абстракции и сделать некорректное состояние невыразимым на нижележащих.
- Написать тесты, которые как минимум демонстрируют замысел.
- Зачищать ненужный код, комментарии, и т.д..
- Хороший комментарий в коде — это ссылка на документацию или объяснение, почему сделано именно так, а не иначе.
- Снабжать commit'ы внятными комментариями.
- Не объединять в одном commit'е разрозненные по смыслу вещи.
- Делать обзор собственного кода перед созданием pull-request'а.
- Не создавать слишком большие pull-request'ы. Если изменений слишком много — лучше создать несколько pull-request'ов.
- Делать code review. Никакого merge до консенсуального исправления всех значимых замечаний.
Я, наверное, что-то упустил, но это не важно. Важно то, что всех этих практик все равно оказывается недостаточно. )))
vyatsek
21.11.2018 16:08Странная статья без каких-то общих подходов.
Код должен быть такой, чтобы было понятно за чем (по какой причине) так написано, а не что код делает.Nondv
23.11.2018 12:26Истинно так.
Главное соблюдать уровни абстракции. А конкретную низкоуровневую реализацию всегда можно понять, немного углубившись.
HerrDirektor
Знавал я в конце прошлого века одного дядечку, который специально запутывал свой код так, что без бутылки не разберешься. Эдакая ручная обфускация.
На вопрос — «а зачем?» был шикарный ответ: «Чтобы меньше криворуких смогло разобраться в моих идеях и скопировать их».
Хотя дяденьке на тот момент было за 40, подростковая глупость и гордыня так и не покинули его.
Не будьте такими дядечками.
Igor_Shumilov
На прошлой работе у нас была интересная задача. Я был сотрудником ЦНИИ в Санкт-Петербурге, который писал прошивку для микроконтроллера. Само изделие должен быть изготавливать завод в Нижнем Новгороде. Соответственно всю документацию (включая исходный код) надо было передать ему.
На заводе было своё КБ, которое всё чаще задавалось вопросом «Зачем нам ЦНИИ? Мы же и сами может разрабатывать изделия!». И вот тут то и возникла задача, как бы им передать исходный код, который работает, но который они не смогут понять и использовать в качестве отправной точки своих разработок. И скилы такого дядечки нам бы пригодились.
roscomtheend
В этом случае нет. Разобраться как работает — однократно и профит высок, можно сделать. Поддержавать постоянно — профита мало, а сил заметно больше (если не свести к пункту 1).
a-tk
Такие вещи решаются другим путём: если не хотите, чтобы кто-то обогнал вас на ваших разработках — двигайтесь вперёд сами.
Igor_Shumilov
В данном случае не получилось бы. У нас не было производственных мощностей чтобы изготавливать серийное изделие. А у заводчан они были.
На мой взгляд нанять дополнительных инженеров всё же проще, чем организовать промышленное производство. Поэтому в этой «гонке» преимущества были не на нашей стороне.
a-tk
Тогда бабло побеждает зло: продали и забыли.