Комментарии 34
Это не новелла, это хоррор! Спать 3 дня теперь не смогу!
Это шедевр! Пишите еще.
Хехе, у этого хоррора есть еще сиквел про маленький переиспользуемый пакет, в который вынесены несколько полезных функций, используемых в нескольких проектах. И теперь мало того что изменения логики могут аукнуться в каком-то далеком месте, и тесты этого не покажут; мало того что любые изменения, ломающие API, становятся дикой головной болью... Так еще у этого пакета образовалось несколько внешних зависимостей, и все проекты его использующие обязаны выравнивать версии своих зависимостей, попутно исправляя ломающие изменения, привнесенные их обновлением.
Мне это всё напомнило вот такой хороший сайт. Периодически делюсь этим с младшими коллегами: https://refactoring.guru/ru/smells/long-method
Например, в истории по ссылке попытка избавиться от слишком длинного метода простым извлечением метода чревато тем, что просто расплодятся вспомогательные методы, которые не имеют смысла без основного и сильно связаны с ним и друг с другом. В результате «слишком длинный» (якобы) метод, который занимал целую страницу (но всего одну, которую можно окинуть одним взглядом, разползется на кучу, занимающую, из-за бойлерплейта, далеко не одну страницу, которую уже не окинуть одним взглядом. А вот сложность при этом никуда не денется.
Нужно как-то все же обосабливать хоть какие-нибудь блоки, даже если это приводит к некоторому увеличению кода, и если обособления в них даже не видишь.
Просто потому, что когда читаешь код, его пытаешься располжить в свою быструю память, а она ограничена. Пыешься охватывать взглядом блоки, объемы которых могут быть схвачены за один взгляд. И т.д.
И вот нужно как-нибудь разбивать на подфункции, или как-либо по другому делать разметку, что бы было удобно читать такой код. Однозначных рецептов нет, но если не пытаться, то вообще не будет разбивки.
Бывает следующая стадия. Когда эта функция начинет уменьшаться и даже иногда исчезат вовсе.
Почему-то в рабочих проектах происходит именно так, да. А вот в собственных у меня уже несколько раз случалась следующая красота: Некоторая логика несколько раз используется в разных местах. По идее всю эту логику можно было бы инкапсулировать в отдельный класс. Причем простой и наивный класс. И вот в один день я сажусь этот класс писать. Это ему знать не нужно, это тоже. И вообще, вот у меня тесты, как я буду его использовать.. И вдруг он уже готов. 100 строк тестового кода, который полностью проверяет поведение класса. Сам класс от силы строчек 30. Прохожусь по основному коду и дублирую существующую логику с использованием нового класса и проверками, что результат одинаковый. Нахожу ошибки в старой логике, которые с новым классом оказались на поверхности. Выпиливаю старый код. 100 строк удалено, 30 новых вставлено.
Так вы описали стадию
Рекурсия блестит и сверкает своей стройностью и логичностью. В ней ничего лишнего, к ней ничего не нужно добавлять. Пока...
Просто собственные проекты изменяются реже. Расширяются, растут и дт так же, а вот вернуться к старому функционалу и сказать: "а теперь ты будешь работать иначе" случается реже, так как заказчик — вы. А когда заказчик это новый человек на старой должности и он хочет новые свистелки, но только в одном месте из трех, откуда выносили класс — вот тут и начинается хоррор из статьи.
По моему опыту уже только наличие тестов почти всегда предотвращает дальнейшие негативные этапы.
Да и вообще, я если пишу функцию длиной более 20 строк, это уже звоночек, что что-то не так.
Объясните начинающему, как написать, например, маппер на овермного полей в 20 строк кода? Или вот ещё, работа с экселем, когда нужно вставить много полей. Наверняка есть ещё куча примеров, но я не припомню
Если у Вас есть структура на 20+ полей, то это уже само по себе обычно означает необходимость рефакторинга и совершенные ранее ошибки.
Если полей менее 20, но у маппинга сложная логика, то выносим всю логику в отдельные малые методы и вспомогательные мапперы.
Ну и наконец 20 строк - это звоночек, а не жесткое правило. Просто в 9 случаях из 10 метод на 20+ строк уже необходимо рефакторить.
Как наличие тестов предотвратит изменения в требованиях?
Вот был у вас какой-то вынесенный кусочек с проверкой, допустим неких данных. Ок — ок, не ок — выдать сообщение пользователю (одно и то же). И был он такой одинаковый в пяти местах.
Потом оказалось, что нужно в одном из мест сделать сообщение синеньким. Потом в другом — что на иврите. Потом, в третьем — выдать пользователю "ок" и тихонько сообщить в СБ.
Да в первом случае можно добавить цвет со значением по умолчанию в параметры. Потом встроить функцию перевода, третий случай наверное все-таки нужно менять на вызов нового блока, но 90% кода — проверка собственно — будет совпадать. Ясно, что проверка скорее всего вынесена в отдельную функцию, но все равно будет блок-обертка над куском в целом. и про эту обертку надо будет думать, или делать 5 (или 3) штуки, отличающиеся одной строкой, или копить условия в одной обертке.
Тест в какой-то момент станет очень лсожно доработать и проще окажется задублировать метод. Тест в данном случае просто заставляет лишний раз подумать.
Тесты никак не связаны с изменением требований. Собственно если меняются требования, то надо идти и менять тесты. А если тесты нормально не меняются, то, как уже было сказано, это повод задуматься, нужно ли все это пихать в ту же самую функцию.
а еще бывает забавно, когда в самом языке есть баг, который много раз отлавливали в реальных программах и научились обходить. но когда встал вопрос исправить этот баг в самом языке, оказалось, что теперь это может повлиять на тысячи работающих программ, у которых вдруг сломается логика, основанная на этом баге. причём не понятно где, когда и как. и этот баг оставляют в самом языке.
Причём в первом же абзаце обозначена проблема. Недавно сознательно написал такой код:
res = sync_and_do_something(data)
if (is_err(res)) {
close_and_release(data);
return 1;
}
close_and_release(data);
return 0;
Потому, что совпадение низкоуровневого поведения при успехе и неуспехе - это так получилось. При любом техническом или функциональном изменении они разойдутся и появится описанная выше проблема, когда содержательно разный код "продолжить работу vs закрыть работу" описывается одной функцией со 100500 параметров.
ПС
К стати мне кажется, что функциональное программирование (при существующих его недостатках) частично решает описанную проблему, т.к. там принято в более явном виде разделять "основную логику" от "побочной, передаваемой как лямбды".
Мне сложно это описать более явно, но вот есть ощущение, что в ФП такой стиль \ привычки, что этой проблемы скорее не возникнет.
res = sync_and_do_something(data) if (is_err(res)) { close_and_release(data); return 1; } close_and_release(data); return 0;
А почему не что-то вроде:
res = sync_and_do_something(data);
close_and_release(data);
return iserr(res);там обычно есть конструкция try… finally, которая позволяет вызвать код очистки вне зависимости ни от чего. Или — ее фунциональный заменитель, типа автоматического вызова деструктора локальной переменной при ее выходе из области видимости в C++.
Но вот в старых, процедурных языках, типа C, оригнинального Паскаля (не Delphi) и т.п. это действительно проблема.
PS Я бы все-таки, если расхождение веток пока что не просматривается на горизонте, слил бы пока эти две ветки в одну — но с прицелом на разделение, как только они разойдутся (для этого и комментарий можно оставить).
PS: Прототипом к описанию была функция, которая занимает ~3 страницы, с кучей вспомогательных функций, которые все мельче, но суммарно их много.
Когда я пришел в программирование в первые полгода мне надо было пофиксить один мелкий баг. Баг казался несложным, со стэктрейсом с nullpointer в определенной строке. Я не видел подвоха пока не открыл исходники-там вызывалась функция на 3000 строк с десятками входных параметров и обращений к базе с ифами! И ошибка была где-то в середине...
В общем поиска причины мне хватило чтобы усвоить на практике зачем нужны правила типа SOLID, KISS и как ненадо писать код.
3 страницы - это ещё не так много. Как насчёт функции, которая занимает 12 страниц? При этом она рекурсивно вызывает себя и напрямую, и через другие функции. Человек, который её написал, говорил, что должен "помедитировать" прежде чем что-то в этой функции поменять. Он уволился 2 года назад, а тот код до сих пор никто не трогает, потому что его даже протестировать толком нельзя.
Не потому, что логика обосабливаемая, а просто потому, что оно повторяется.
Вот она причина, повторяется она по чистому совпадению. Если нельзя разбить на логические утилитнные функции, я всегда против переиспользования
Ну понятно, в программировании вообще нет абсолютных правил, только гайдлайны.
Когда мы выносим в одну функцию код, который не связан логикой мы нарушаем Single Responsibility Principle: каждый класс должен иметь только одну причину для изменения. Все дальнейшие проблемы от этого
каждый класс должен иметь только одну причину для изменения
Смысл этого предложения несколько кривой, наверное ты подразумевал «иметь одну ответственность». И дальше следует категоричное «Все дальнейшие проблемы от этого».
Согласно этому принципу, если мне в функцию размером в страницу кода нужно добавить новое условие, то я должен сделать копию этого кода и менять в копию. Ведь это уже другой смысл — «А» это не «А+Б». Когда понадобиться восемь доп.условий, то это уже 256 потенциальных страниц копий кода. И потом еще на каждый вызов иметь switch/case то же на 256 позиций — это что бы не заморачиваться, которые варианты из этих 256 все же нужны для текущего случая.
Смысл этого предложения несколько кривой, наверное ты подразумевал «иметь одну ответственность».
Uncle Bob настаивает именно на этом определении: https://blog.cleancoder.com/uncle-bob/2014/05/08/SingleReponsibilityPrinciple.html
Если функция выполняет «А», то причина изменения может быть, то что бизнес требование «А» поменялось. Если есть другая бизнес задача «Б» , которая по совпадению имеет такой же код как и «А», то, согласно принципу они должны лежать в разных местах, и если требование к «А» (но не «Б») поменялись, то можно без проблем поменять код в одном месте
https://blog.cleancoder.com/uncle-bob/2014/05/08/SingleReponsibilityPrinciple.html
Новелла про одну функцию. Основано на реальных событиях