Я собрал худшее из худшего! Оказалось, что хороших практик — море, и разбираться в них долго, а вот плохих, реально плохих, — считаные единицы.
Понятно, что плохие практики не отвечают на вопрос: «А как делать-то?» — но они помогают быстро разобраться в том, как не делать.
Мы часто спорим про архитектуру и хотим друг от друга знания разных правильных практик проектирования, лучшего мирового опыта и вот этого всего. Понятно, что в реальном мире это совсем-совсем не так. И худших практик часто достаточно, чтобы начать договариваться, как не надо.
Итак, мой любимый антишаблон — поток лавы. Начинается всё просто: в новый проект можно добавить код из старого проекта копипастой. Он там делал что-то полезное, пускай делает почти то же самое полезное в новом проекте. Вот только нужно закомментить один кусок, а в другом месте — чуть дописать. Примерно через три переноса без рефакторинга образуются большие закомментированные участки, функции, которые работают только с частью параметров, сложные обходы вроде «выльем воду из чайника, выключим газ, и это приведёт нас к уже известной задаче кипячения чайника» и так далее.
Это если команда одна. А если разработчики на пятом проекте новые, то начинается самое весёлое — этот сталактит надо ещё прочитать.
Очень часто я вижу лава-код в проектах аутсорсинговых компаний, потому что они используют свою кодовую базу по разным заказчикам как такой своеобразный иннерсорс. А «междисциплинарный» код как раз хорошо обрастает отключаемыми участками и переопределяемыми функциями.
Поток лавы
Итак, дамы и господа, позвольте представить: первый на нашей сцене — поток лавы! В целом про него я уже примерно рассказал, и вы представляете, откуда он такой берётся и что делает. Обратите внимание: паттерн подкупающе хорош в кратковременной перспективе и становится плох только на дальней дистанции. И смертельно плох на очень дальней. Поэтому на всякий случай давайте формально отмечу основное.
Симптомы:
- Непонятно откуда взявшиеся переменные.
- Не относящиеся к задаче фрагменты кода.
- Очень странное покрытие документацией, много выглядящего важным за её пределами.
- Архитектура слеплена из предыдущих трёх архитектур.
- «Да кто её будет смотреть, просто подключайте к проекту!»
- Устаревшие интерфейсы.
Что будет дальше: а дальше либо всё это разбирать, либо разработчики возьмут ветку целиком и унесут дальше, потому что её слишком сложно рефакторить, но она работает. Через три итерации код невозможно будет задокументировать во внятные сроки и в него нельзя будет быстро внести изменения.
В API-driven-подходе + Domain-Driven Development такого в принципе не должно случаться, а мы хотим думать, что придерживаемся этих идей. Точнее, архитекторы уверены, что придерживаемся, а дальше уже как пойдёт в конкретных командах.
Как и в других случаях ниже, тут возможно отдельное развлечение — прогулка по минному полю, когда вы не знаете, какой фрагмент кода как точно работает. И некоторые, казалось бы, очевидные и прямые решения приводят к интересным для отладки последствиям. Если к вам подходит более опытный разработчик и говорит: «Ты так не делай, сейчас расскажу, как обойти, эта функция вообще опасная, с ней лишний раз не связывайся» — это как раз оно.
Blob
Это жирнющая штуковина, которая делает всё. Обычно это класс, который разросся сначала до уровня универсального решателя всего-всего внутри проекта, а потом чуть ли не стал операционной системой. Появиться он может из-за неправильных решений в архитектуре или просто инкапсуляции всего подряд в страшной спешке. Либо из-за последовательного улучшения какого-то класса несколькими поколениями разработчиков, которым просто вбить костыль и пойти дальше.
Эта штука перестаёт быть в полной мере объектно-ориентированной и возвращает нас в мрачные времена расцвета программирования. Собственно, на практике я встречал такие конструкции только в банковском легаси. Всё, что было примерно после нулевых, уже проектировали с головой независимо от того, объектное или функциональное там программирование. Даже старые монолиты могут быть разделены на функции так, что получается модульная структура, легко переживающая изменения (она только релизится вместе, а разрабатывается независимо).
Опять же очень часто блоб выступает просто первым симптомом ещё каких-то проблем. Я видел смелых людей, пытавшихся такое рефакторить (да и сам участвовал пару раз). Так вот, за этим нагромождением обычно бывают сразу десятки частных случаев плохих практик.
Думаю, что несильно ошибусь, если скажу, что у каждого банка из топ-10 в России есть система, которой больше 15 лет и в которой есть штуковина размером с фреймворк для бизнес-логики, вокруг которой аккуратно расставлены обработчики и разные внешние интерфейсы. Сама же система не поддаётся членению и не может быть переписана в ближайшие годы.
Непрерывное устаревание
Здесь всё просто: нужно смотреть, что сейчас новое и правильное из технологий, и внедрять то, что было новым хотя бы пару лет назад. Ну или не внедрять, но осознанно, понимая, что дальше произойдёт с вашим кодом.
Частный случай устаревания — когда какой-то компонент перестаёт поддерживаться, и поддержку фактически оказывает команда разработки. Это даже веселее, чем просто работа на старой технологии.
Заканчивается это либо тем, что система умирает от старости и больше становится не нужна, либо рефакторингом, больше похожим на переписывание с нуля.
Функциональная декомпозиция
Это когда разделение на классы делается не по функциональному уровню, а по частям бизнес-процесса, что в итоге даёт очень дегенеративную архитектуру.
Это когда тащат бэк на фронт и наоборот: фронт запихивают куда-то внутрь классов бэка. Это когда запрос остатков внезапно открывает соединение с каким-то веб-сервисом, это когда в функции сложения двух чисел строится красивый отчёт для оператора и так далее.
Вот простой пример функциональной декомпозиции:
Выяснять плюсы и минусы ООП тут не планируем, это лишь простая демонстрация усложнения понимания в процедурном решении.
Процедурная версия расчёта кредита для клиента.
Объектно-ориентированная версия расчёта кредита для клиента.
Распутывание такого кода, когда он на всём проекте, — отличное приключение. Обычно такая практика случается, когда на проекте — несколько джунов, и нет никого, кто выступил бы архитектором. Или когда разработчики быстро клепают MVP, понимая, что завтра этот код полетит в корзину, а в итоге он используется годами. Иногда я вижу такое в коде аутсорс-компаний, которые берегут время разработчиков.
Итог в том, что система строится так, что провести рефакторинг практически нельзя. Приходится с этим жить и постепенно делать из этого блоб.
Лодочный якорь
Это когда вам досталось много кода, и весь этот код бесполезен в рамках задачи. Я такое видел после слияний-поглощений компаний, когда из двух систем компаний в итоге делают одну. Или когда вам достаётся код без базы данных, часть логики реализована в базе — и удачного вам сбора требований и попыток доработки.
В общем, так бывает, что у вас есть что-то не просто неиспользуемое, а ещё и мешающее. Такой код надо вырезать целиком как можно быстрее.
Золотой молоток
Эту практику я вижу практически постоянно. Когда у вас есть молоток, всё вокруг становится похожим на гвозди.
Когда у вас есть работающий код или технология, обвязанная большим количеством кода, кажется, что ими можно решить любую проблему.
Я встретился с золотым молотком в момент, когда Кафка стала хорошо протестированной шиной. А у нас в проекте везде применялась интеграция через IBM MQ. Пришлось потратить много времени на защиту нового решения интеграции и убедить коллег в её преимуществах для нашей очередной интеграционной задачи.
Смысл в том, что золотой молоток позволяет решать задачу без когнитивной нагрузки. Просто взяли и забили известной технологией. Новая же технология требует изучения, поиска подводных камней, потом непонятно сколько кода переписать, непонятно, что там с ИБ, непонятно, как поддерживать. Но всё это можно оценить. И если преимущества перехода выше, чем все эти затраты, то переходить имеет смысл.
Важно понимать: это обоснованное желание остаться в рамках старой технологии, или «синдром утёнка», когда первая изученная технология кажется лучшей.
Спагетти-код
Вообще-то это группы объектов, написанные для конкретного бизнес-процесса, но не предназначенные для использования в другом. На практике это скопипащенные куски кода, которые в плюс-минус одинаковом виде встречаются раз так 5–10 минимум в разных модулях.
wait_nomsg:
if ((inb(tmport) & 0x04) != 0) {
goto wait_nomsg;
}
outb(1, 0x80);
udelay(100);
for (n = 0; n < 0x30000; n++) {
if ((inb(tmport) & 0x80) != 0) { /* bsy ? */
goto wait_io;
}
}
goto TCM_SYNC;
wait_io:
for (n = 0; n < 0x30000; n++) {
if ((inb(tmport) & 0x81) == 0x0081) {
goto wait_io1;
}
}
goto TCM_SYNC;
wait_io1:
inb(0x80);
val |= 0x8003; /* io,cd,db7 */
outw(val, tmport);
inb(0x80);
val &= 0x00bf; /* no sel */
outw(val, tmport);
outb(2, 0x80);
TCM_SYNC:
/* ... */
small_id:
m = 1;
m <<= k;
if ((m & assignid_map) == 0) {
goto G2Q_QUIN;
}
if (k > 0) {
k--;
goto small_id;
}
G2Q5: /* srch from max acceptable ID# */
k = i; /* max acceptable ID# */
G2Q_LP:
m = 1;
m <<= k;
if ((m & assignid_map) == 0) {
goto G2Q_QUIN;
}
if (k > 0) {
k--;
goto G2Q_LP;
}
G2Q_QUIN: /* k=binID#, */
Что надо знать: хотя бы принципы функционального программирования. А в идеале — ООП и архитектурные принципы типа DDD.
Это тот самый код, который имеет очень яркую региональную принадлежность, и если вы аутсорсите, например, в Индию, то встречаться с ним придётся довольно часто.
Копипаста
Сначала были только книги и журналы. Код из них, конечно, можно было перепечатать, но к катастрофам это не приводило. Появился Google (как первый удобный поисковик), и разработчики стали много гуглить. Непонятные куски кода стали появляться в проектах. «Эта штука делает то-то, и не трогайте внутри» стало нормой. Но такими кусками закрывались какие-то дыры, где компетенций не хватало. Появился StackOverflow, и код часто стал состоять из таких скопированных кусков, а разработчики только криво сшивали их между собой. И, наконец, вершиной копипасты стал ChatGPT4 (и аналоги), который умеет сшивать разные фрагменты кода между собой лучше бездумного копирования.
В результате может получиться или прекрасная экономия времени, если вы понимаете каждую строчку кода, или лютейший отборный г*код, который со временем доставит много проблем всем остальным.
И отдельно — про грибы
Это немного не про разработку, но последствия ощущаются и в коде. Есть такая история — «Mushroom management», когда вместо задачи даются требования. Причём частями. По-русски это называется «Сиди в темноте и ешь навоз», что очень точно описывает основные принципы выращивания грибов. Так вот, если так поступать с разработчиками, то вас ждёт море интересного на приёмочных тестах и при интеграциях. Особенно при интеграциях.
Пока всё
В эту статью не попали, например, такие антипаттерны, как Poltergeists, потому что слабо тянет на антипаттерн, Тупик (Dead End), Кладж ввода (Input Kludge) или Слепая вера (Blind faith), потому что он должен автоматически решаться тестами, и другие, которые не относятся напрямую к разработке (или мало распространены с точки зрения автора), но при желании читателей рассмотрим и остальные.
Такие плохие практики называются «антипаттерны» в книге «Банды четырёх» «Шаблоны проектирования», где как раз описываются практики хорошего программирования. Хорошие практики назвали «паттернами», а противоположные — «антипаттернами». Про антипаттерны можно посмотреть ещё вот здесь:
- www.amazon.com/AntiPatterns-William-J-Brown/dp/0471197130
- antipatterns.com (и antipatterns.com/more.htm)
- sourcemaking.com/antipatterns
Ну а я пошёл дальше собирать хорошие практики, чтобы однажды выложить сборку с ними.
Комментарии (25)
aegoroff
20.06.2023 07:27Статья понравилась, но хотелось поподробнее про Mushroom management, примеры последствия и т.д.
alexundros Автор
20.06.2023 07:27+1В целом, можете спросить любого сотрудника большой компании, услышите много дивных историй. Мы тоже поищем, потому что не боимся про такое рассказывать, хорошая идея, спасибо.
gybson_63
20.06.2023 07:27Ну вот простой пример, который везде наверное можно встретить. Переходим с продукта N на M. Старый продукт был интегрирован с другими системами и прекрасно отдавал данные, но по текущим временам по-старому, файликами. Для нового продукта и интеграция новая пишется, на основе "кролика". Все замечательно, только уже никто не помнит и не знает как данные ходили и трансформировались по пути. Аналитики этого не видят в глубинах кода и задача ставится примерно так : "Сделать обмен данными как было" =)
И вот сидишь, читаешь тонны старого кода просто чтобы понять что от тебя надо.Helldar
20.06.2023 07:27Прилетает одна задача "не работает сортировка. должна работать так", и тут же прилетает другая от другого человека тоже по сортировке, но условия иные.
Смотришь на две задачи и понимаешь, что визуально они дополняют друг друга, но когда приступаешь к корректировке кода, оказывается, что они друг другу очень сильно противоречат по условиям, и оба человека говорят что должно быть именно так, а продукт менеджер на их стороне.
Aquahawk
20.06.2023 07:27+9Каждый раз когда я читаю таки статьи у меня вопрос, все же понимают что производительный код и код соблюдающий кучу принципов удобной архитектуры это почти всегда разный код. И компания должна понимать, за что она готов платить и что она получит. И не устаю приводить шикарный пример файла checker.ts из компилятора Typescript https://github.com/microsoft/TypeScript/blob/main/src/compiler/checker.ts. Это файл на 2.7 Мегабайта кода, в котором 2.5 тысячи функций объявленный словом function и ещё около тысячи стрелочных замыканий. Его даже гихаб отобразить нормально не может. Этому файлу больше 10 лет, он активно развивается. Больше 5 лет назад я предлагал способ его распилить https://github.com/microsoft/TypeScript/issues/17861 Распил приводит к потере производительности. Команда отказалась. Они готовы платить ту цену которую стоит поддержка такого файла для одной цели: скорости работы компилятора.
Typescript великолепный продукт, и люди его делающие точно не идиоты. Но если взять этот файл и дать на ревью любому человеку, который не знаком с внутренностями этого компилятора, он скажет что это полный говнокод который невозможно поддерживать. Кто угодно скажет что это плохо. Но это работает, и те, кто знает и понимает зачем это и почему, продолжают это поддерживать.
И очень тонка разница между безумием и следованием стратегии которая идёт против ветра современных веяний качества программного обеспечения.
ksbes
20.06.2023 07:27+2Этот файл - это пример не говнокода (код-то там как раз нормальный), а говноархитектуры. Перемешивая говно - можно получить только перемешанное говно.
Т.е. в данном конкретном случае надо заново проектировать TypeScript 2, а не кровати переставлять (тоже, кстати, очень распространённый антипаттерн). Но это, как я понимаю, слабоосмыленно.А так принципы как раз и позволяют получать производительный или хорошо поддерживаемый код (не всегда одновременно, к сожалению. C'est la vie)
Aquahawk
20.06.2023 07:27+2Нет, я с вами не согласен. Нет другой такой архитектуры которая бы позволила добиться такой скорости. Это давно измерено и известно, в JS обращение к замкнутой переменной быстрее чем к свойству объекта. И на данном масштабе это становится значимым. Не переписать это лучше с наскока. А Джоэл Спольски уже 23 года назад написал про то, что взять и всё переписать, это худшее что может предпринять любой большой проект https://habr.com/ru/articles/219651/
ksbes
20.06.2023 07:27Вы слово "перепроектировать" от слова "переписать" отличаете? Переделывать плохое - плохое и получишь. Это ж очевидно! Плохое надо понять, отбросить и на основе выработанного понимания сделать новый проект - лучше (если получится). Но это долго и дорого, естественно - потому не всегда целесообразно.
По тем же замыканиям - вопрос не в том, что они быстрее (есть много способов это использовать), а в том что есть тысяча переменных/функций друг с другом связанна так, что их нельзя по разным файлам раскидать. Не проведена банальная декомпозиция. Архитектурная. Не выделены смысловые независимые множества. Проводить её - это не просто что-то там по модулям раскидать. Это переписать вообще всё.
А заниматься улучшайзингом ради улачшайзинга не имея более-менее точно измеримой цели (уменьшить размер исходного файла - это не цель разработки) - вот это антипаттерн.
Вот конкретно в вашем случае - требования к скорости явно были и до того как вы принялись править? Верно? И они были приоритетнее поддерживаемости, так? Так зачем вы вообще этим занялись, если заранее можно было понять, что по цели вы сделаете хуже а не лучше?Aquahawk
20.06.2023 07:27+4Не проведена банальная декомпозиция. Архитектурная. Не выделены смысловые независимые множества
это сделано сознательно. Разделение стейта чекера приведёт к деградации производительности.
Neki
20.06.2023 07:27Почему нельзя какие-то сборщики подключить. Чтобы читать и поддерживать это можно было нормально, а в итоге собирался огромный файл который может быстро работать?
Aquahawk
20.06.2023 07:27Так зачем вы вообще этим занялись, если заранее можно было понять, что по цели вы сделаете хуже а не лучше?
Потому что точной цифры не было. Её узнали.
alexundros Автор
20.06.2023 07:27+8В книге «Основы архитектуры программного обеспечения» Нила Форда ввели «Первый закон архитектуры программного обеспечения», который гласит: «Все в архитектуре программного обеспечения соткано из компромиссов».
Aquahawk
20.06.2023 07:27+2Вот с этим согласен. И круто когда осознаются разные потребности и обоснованно одни приносятся в жертву в пользу других. К сожалению сегодня часто можно видеть фанатичное требование соблюдать какие-то архитектурные паттерны в угоду моде. Например, в какой-то момент все пихали mongo db во всё что можно и нельзя, просто потому что реляционная база это "старое медленное говно", благо сейчас потихоньку отпускать начало.
Helldar
20.06.2023 07:27У нас в проекте один чел так из редиски БД сделал. Причём не просто класть туда данные, а как кэш-репозиторий. То есть запрашиваем, например, массив данных из таблицы - в редиску пойдут все строки из этой таблицы, причём, в один ключ кэша один документ. При повторном запросе идём в кэш и поштучно получаем идентификаторы...
Но и отказ от многих современных методов, которые действительно позволяют значительно сократить время разработки, повысив читаемость кода, тоже плохо и зачастую переходит грань фанатизма. Типа "новое жрёт много памяти" (спойлер: экономия на спичках), или "да 512 метров памяти для пыха на проде это слишком много - 128 метров норм", или "да зачем юзать плагин для IDE с хоткеями и уем - можно же консольную команду написать", и т.д.
Фанатизм в разработке как палка о двух концах: с одной стороны как отказ от нового, так и много этого неоправданного "нового" способно "выстрелить в ногу", так сказать. Здесь действительно главное уметь держать грань разумного.
Helldar
20.06.2023 07:27В сфере разработки огромное количество паттернов и принципов, но предпочитаю руководствоваться всего одним:
Всегда пиши код так, будто поддерживать его будет неуравновешенный и склонный к насилию психопат, который знает где ты живёшь.
perecfx
20.06.2023 07:27+4"Смысл в том, что золотой молоток позволяет решать задачу без когнитивной нагрузки. Просто взяли и забили известной технологией. Новая же технология требует изучения, поиска подводных камней, потом непонятно сколько кода переписать, непонятно, что там с ИБ, непонятно, как поддерживать."
Вот так каждый молодой, активный и прогрессивный приходит в проект с наивным желанием заменить чужой золотой молоток на свой с целомудренным желанием переписать весь код на более прогрессивное и "правильное", осиливает 5%, начинает скучать и терять оптимизм, уходит в другую контору, а потом приходит новый на его место с новыми прогрессивными хотелками... через год новый, а потом... фрагментированный сгнивший монстр празднует свое 15-ти летие еле дыша под матами пользователей.
vanxant
Наброс про скази не понял. Нормальная машина состояния на низком уровне (а никакого другого уровня в драйвера не завезли).
Желаю успехов переписать это без goto (break и continue это тоже goto) с условием не использовать стек значительно больше, чем в приведённом примере (стек в ядре совсем не резиновый).
ksbes
Ну во-первых, есть инлайн-функции, и хотя бы ожидание с таймаутом можно было бы и в такую булеву функцию вывести. Во-вторых - if тоже goto и надо уметь этим пользоваться.
Но конкретно именно к этому коду у меня отношение - "работает - не трогай". Такой код вещь в себе, ему можно. Главное чтобы он сидел где-то под ковром и не отсвечивал (т.е. не было необходимости в него лезть). Что в приведённом отрывке - не соблюдается, хотя бы из-за "магических" времён таймаутов.
vanxant
Проблема с таким двустрочными инлайн-функциями в том, что в них часто приходится слишком много тянуть лишнего (указатели на локальные переменные и т.д.). По факту много потенциально опасной писанины ради непонятно чего.
Если перед глазами есть граф машины состояний железки со всеми переходами и задержками, такой код как в примере из ядра линукса читается вполне себе легко и приятно. Если нет, то лучше в этот код вообще не лезть, в каком бы стиле он не был написан.
aegoroff
плюс в ядре есть ограничения по размеру стека вызовов (это для любителей разбивать код на множество однострочных функций) и его там экономят всеми силами - это вам не юзермод с его мегабайтом стека
vadimr
Ну бесконечный цикл постоянной занятости процессора в первых трёх строчках – это не вершина программистской мысли, с какой стороны ни посмотри. Или чем, к примеру, замечательно число 0x30000?
Очень многие драйверы – чудовищный кошмар внутри, и не только в линуксе.
usrsse2
switch в цикле?
yasha_akimov
Так то goto без проблем заменяется стандартными условиями, циклами да менеджерами контекста, было бы желание.