Как стать автором
Обновить

Комментарии 82

Мне кажется или все в итоге все сводится к «работает — не трогай»? Не в каком-то конкретном случае, а абстрактно.
Или все таки стоит делать послабления в сторону «давайте разберем этот окаменелый кал, его уже невозможно ни читать, ни поддерживать»?

В общем-то да. Вы правы. С одной стороны "работает — не трогай". А если трогаешь — осознавай зачем и кому это принесёт пользу.


"Давайте разберёмся" — это конкретная задача с конкретной целью. Ничего против не имею. Но просто так лезть в код, потому что он старый/неподдерживаемый, но работает и последние 3 года его не трогали — это плохая затея.

Не согласен с автором. Проблема в начале статьи про большой пулл-реквест именно в том что пулл-реквест большой. А не в том, что там рефакторинг.
Просто не смешивайте логику и рефакторинг в одном коммите и будет вам счастье. И команду тренируйте чтобы так не делали (методом металлической полметровой линейки).
А то по такой логике получается нафиг его делать не нужно, оно же все ломает.
Просто не смешивайте логику и рефакторинг в одном коммите и будет вам счастье.

Люто плюсую. Если вижу проблему в коде который собираюсь менять — сперва коммит на рефакторинг (с пометкой в сообщении), затем пулл-реквест, и только потом собственно разработка. Пулл-реквесты удваиваются, но сложность каждого падает. Коллеги смирились (читай такой подход — дело вкуса). Обоснование для босса: «трачу полчаса на минимизацию рисков при изменении бизнес-логики».

Да. В идеале так и хочется делать. Но далеко не всегда и не у всех получается. Плюс, многие не понимают зачем так делать. Поэтому и писал, чтобы попробовать растолковать очевидные вещи.

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

Если вижу проблему в коде который собираюсь менять...
делать архитектору больше нечего, чем одобрять получасовые рефакторинги.
А пулл-реквест с одним рефакторингом вполне могут закрыть с вердиктом «никак не относится к задаче».
Команда 5 человек, все рядом, в крайнем случае спросят. Рефакторится только код который реально будет изменяться для решения задачи. Нормальное сообщение со ссылкой на задачу в трекере решает 99% пулл-реквестов. Честно говоря, не помню ни одного реквеста закрытого по «никак не относится к задаче».

Были возражения «на твои реквесты приходится отвлекаться в три раза чаще» (ибо подготовка-задача-подчистка). Потом один коллега выдал реквест на 60 файлов — и возражения в мой адрес резко спали. Контраст-с.

Наконец, в мелких атомарных PR есть своя прелесть. Тех сотрудников, кто подтверждает не глядя, не мучает совесть. LGFM на атомарный PR чисто морально написать проще. Для любителей принципов\паттернов идея «SRP в коде, SRP в коммите, SRP в пулл-реквесте» греет душу. Фанаты чистого кода наслаждаются дистилированным рефакторингом. Иногда выношу в отдельный PR-изменения не затрагивающие функциональность (новые модели, хелперы не используемые пока в коде, и т.д.) — такие реквесты безопасны, ибо «активный» код не меняется вообще, но полезны (ибо фронт-энд девелопер видит ту модель что я ему отошлю и валидирует её).

Хоть техника и заслуживает кое-где права на жизнь, недостатки у неё имеются. Например, трудно связать бизнес-изменения с подготовительным рефакторингом — разделенные по реквестам они теряют общий контекст. Если подготовка прошла, а основное изменение нет — приходится откатываться. Иногда локальный рефакторинг отходит от общего стиля (например, в одном методе меняется подход к валидации); локально код стал лучше, глобально — ХЗ.

Короче, техника на любителя и ситуацию. На некоторых проектах я её даже пробовать не стал. На регулярной основе никто из коллег её не повторяет, обычно бьют подготовку\выполнение\зачистку по коммитам.
Команда 5 человек, все рядом, в крайнем случае спросят. Рефакторится только код который реально будет изменяться для решения задачи. Нормальное сообщение со ссылкой на задачу в трекере решает 99% пулл-реквестов.

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

Я где-то после 35 резко понял, что код пишется не для машин, а для людей. Соответственно, рефакторинг часто призван сделать код читаемым, поддерживаемым, а всю архитектуру более логичной и эффективной. Ну, например, валидации у нас производились в трёх местах, причём в одном из них ещё и неправильно. Мы выделили всё в отдельный модуль. И т.д.

В примере выше слова вроде все понятные, но расставленные «не по гайдлайнам». Собственно, любой язык программирования — почти то же, что и любой другой язык общения. И да, мы используем правильный порядок слов и знаки препинания. Так быстрее и проще читать. Причём в некоторых языках (в китайском, например) порядок слов задан ещё жёстче.

Для бизнеса в целом вовремя сделанный рефакторинг означает меньшие расходы на поддержку и дальнейшую разработку. Массовая доля костылей в коде тупо повышает степень функции скорости разработки от времени жизни IT-компании
Вы в целом правы, но, к сожалению, за вашими словами скрывается одно «но»:
вовремя сделанный рефакторинг
Осталось только угадать своевременность и согласовать с боссом.
Эта проблема не имеет простого решения. Босс либо понимает, либо… эффективный менеджер
Босс может видеть то, что не видете вы. Например что он продаст компанию через 2 года и на вопросы лонг терм суппорта ему вообще покласть. И да, он примет эффективное решение дать по рукам программисту и запретит заниматься перфекционизмом.

Эта проблема называется "недостаток коммуникации" и "микроменджмент".


Вместо того, чтобы объяснить суть поставленных задач босс пытается выполнять работу программиста.


Естественно, что:
1) босс будет принимать технические решения хуже, чем программист
2) програмист будет считать босса некомпетным самоуверенным дебилом, потому что не знает обстоятельств
3) даже если программист будет выполнять требования, то это будут не те требования, которые изначально нужны

Вы уверены, что босс обязан рассказывать программисту о том, что он собирается продать компанию через 2 года?

А вы уверены, что программист захочет тратить 2 года своей жизни на проект, который даже в резюме будет записать стыдно? Давайте всё же признаем, что у "босса" и работника разные приоритеты. "босс" не в рабство берёт, а нанимает специалиста с конкретными навыками, подходом к работе, стремлениями и тараканами. И всё это надо учитывать. Особенно в условиях дефицита кадров. Мой опыт говорит, что говнокодить — себе дороже. Когда твой код будут оценивать, никто не будет уточнять в каких условиях и почему написано именно так. А программисту, который пусть и быстро, но говнокодит, никогда не доверят действительно крупный проект.

Босс не имеет гарантий, что программисты не расскажут коммерческие планы компании всему миру. Конечно, можно юридически оформить режим коммерческой тайны, но тут программисты скорее выберут оставаться без информации, чем получить её вместе с возможной уголовной ответственностью за разглашение.
Вы уверены, что босс обязан рассказывать программисту о том, что он собирается продать компанию через 2 года?

Если босс хочет, чтобы программист работал эффективно — не то чтобы обязан, просто других вариантов нет.

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


Ну, что ж. Наверное в список проблем стоит добавить "босс — гондон". Честно, я совсем не имею понятия, что советовать гондонам. Я абсолютно некомптентен в вашем вопросе

Почему не этично? Возьмём, к примеру, Minecraft, который Нотч продал в Microsoft за $2.5 млрд.

Он, как получил деньги, сразу умыл руки с проекта. Команде, конечно, неприятно. Но покупателю важна только юзер-база и бренд. Им не нужен ни этот программист, ни код, они сами всё допишут.

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

У вас содержится неявная предпосылка "если Нотч свалил, то значит Нотч говнокодил специально". Иначе пример Нотча бессмысленнен в контексте разговора.


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

Это пример того, что покупателю иногда не важно качество кода от слова «совсем» (они всё равно планировали переписать с java на c#, не следил, чем это закончилось).

Акцент на Нотче в том, что он показал неэтичное поведение, свалив с проекта. Покупатель думал, что купил бренд и его лицо в виде разработчика, а лицо свалило (и качество кода тут не при чём). Там ещё были скандальные новости, как Нотч обещал членам команды по сколько-там сотен тысяч, если они останутся на полгода, что на фоне его миллиардов просто смешно.

Лучше так спросить: если визионер придумал идею сервиса и хочет быстренько напилить прототип, предпочитая концентрироваться на функциональности (пусть глючной, зато полной в части, которая ключевая для проекта), а не вылизанном отрефакторенном коде (но без полной ключевой функциональности), чтобы потом продать прототип новому владельцу, в этом есть какая-то этическая проблема?
> Покупатель думал, что купил бренд и его лицо в виде разработчика, а лицо свалило

Договор же, наверное, был на бренд, а не на разработчика?

> в этом есть какая-то этическая проблема?

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

Теперь ясен пример с Нотчем. Если было действительно так, как вы описывали, это действительно вопрос этики. Хотя странно, что не было договоренностей на этот счёт.


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

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

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


Например, почти во всех production проектах есть участки скопированного кода. Где-то кто-то поленился один раз, потом два раза, потом три. В итоге, у нас дюжина участков, где находится copy-paste код. При каждом изменении надо не забыть пройтись по всей дюжине и внести идентичные правки. Если где-то забыть, то в лучшем случае функциональность сломатеся сразу. Но так не бывает. Обычно где-то потом спустя несколько дней-недель-месяцев начинают случаться необъяснимые баги. На отладку которых может уйти от часа до недели.


Что лучше: каждый раз править дюжину скопированных кусков кода, или все-таки один раз сесть, отрефакторить, вынести общий код в одно место (модуль, класс, утилиту), и потом не терять время во время последующих изменений и отладки?


Однозначного ответа дать нельзя.


К тому же, у всех разные требования, разный ритм работы, и разная восприимчивость. Кто-то может работать с плохим кодом спокойно, а кому-то видеть что-то вроде:


var priceStr = priceInput.value;
var newPriceStr = priceStr.replace(/\./g, '');
var newPriceStr1 = newPriceStr.replace(/\,/g, '');

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

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

Новые фичи — это уже не поддержка, а полноценная разработка, развитие

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

Изменения могут быть на разных уровнях: и на уровне архитектуры и на уровне какой-то одной мелкой функции. Поэтому и рефакторинг может быть на разных уровнях.

Например, программист Вася планирует реализовать алгоритм и вводит в свою мелкую функцию переменную Counter. Он называет так ее потому, что на этом этапе он так думает. Но по мере разработки он понимает, что название было неудачным и начинает мешать ему лично лучше понимать код. Т.е. условия изменились и примененное техническое решение (переменная и ее название) уже не соответствует текущему состоянию кода. Он делает рефакторинг на своем уровне, переименовывает эту переменную. Теперь новое название Item больше соответствует текущему состоянию кода.

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

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

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

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

Обычно программист не доводит себя до катастрофы, пытаясь переписать код или ища возможность все-то прикручивать сваи не изолентой, а стальным тросом. И мучается, постоянно борясь со сложностями и неудобствами. В итоге его производительность падает, код становится ненадежным, изменение становится настолько дорогим, что программист всячески избегает изменений. Тупик. И бизнес за это платит! Незаметно из кармана владельца уходят суммы, сравнимые с зарплатой программистов. А программисты работают в более сложных условиях, мучая себя и желая избегать работу. Или и вовсе, начинают подумывать об уходе.

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

PS. У меня есть проекты, пережившие 12 мажорных версий. Если бы не рефакторинг, я бы уже давно разорился.

В сказанном вы правы. Но я ведь про другой аспект говорил. Рефакторить надо осознанно и в согласии с бизнесом, а не самому, потому что так решил. Сказать бизнесу, что "вот тут и тут — фигня; она нам помешает вот в таких-то случаях; сделать хорошо займёт столько-то дней". Обсудить детали, запланировать и честно сделать хорошо. А не отрывать кусок времени от основной задачи и вязнуть в переписывании того, что и так хорошо работает, но не удовлетворяет лично разработчика.

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

Еще немного дополнений с практической стороны:

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

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

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

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

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

Это можно ускорить, если начать читать книги:
1. Классиков Agile. Из Agile пришел рефакторинг таким, каким мы его сейчас знаем. Там это — основной инструмент, потому что приниципы разработки предполагались как постоянное изменение кода в условиях неизвестности. Даже если разработка не ведется по идеологии Agile, можно перенести параллели на свои проекты и смотреть, как что меняется и почему.
2. По системному анализу и теории систем. Сейчас это преподают в ВУЗах как стандартный предмет. И это правильно. Сейчас программирование настолько сложное, что нельзя ухватить все в уме. Системный взгляд позволяет успешно проектировать и менять код в таких условиях.

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

Касаемо бизнеса. Чтобы лучше понимать бизнес, нужно самому иметь свой бизнес. Для программиста возможность лучше понимать бизнес — это подработки или shareware. Или совсем иные сферы, но касаемо своих хобби. Пока нет своего бизнеса, пока сам не принимаешь бизнес-решения и не видишь последствия, взгляды на бизнес в целом могут быть верными, но нюансы решают все.
Со своей стороны хочу сузить границы и определить рефакторинг (в худшем смысле это слова) как любые изменения, которые непосредственно не связаны с решаемой задачей и не изменяют внешнее поведение системы, но выполняются в рамках исходной задачи.

Ну то есть вы ввели определение, которое не вытекает из оригинального, и теперь рассказываете нам, как это плохо?


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

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

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

Рулетка же.

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


бизнесу нужна фича и не интересуют детали

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

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

Если сами пытаетесь героически спасать код, но бизнесу это не интересно, то ничего не получится

Но получается же.


Лучше начать с обсуждения целей и причин

"бизнесу не интересны детали".

Детали в виде новых багов там, где их годы не было, будут очень даже интересны.

… а при разработке новых фич или багфиксинге такого, конечно, никогда не бывает?

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

Я и говорю: у вас никогда так не бывает, что идет работа над одной функциональностью, а баги внезапно лезут в другой, где их годы не было?


А вот если втихую начать шлифовать что-то старое, никак не связанное с актуальным беклогом

А этого, заметим, я делать не предлагал.

в Ruby amount == 0 смело заменялся на amount.zero? без учёта того, что для случая nil в amount эти конструкции не эквивалентны

Поведение программы стало более однозначным, потому это и прописано в стандарте. А ловить nil сравнивая с нулём — это хак на грани говнокода.
Большинство проблем с рефакторингом из-за того, что его начинают делать без самого важного этапа перед ним — написание тестов для того кода, который планируется менять. И именно тесты — залог что ничего не сломается. Это, как минимум, текущие тесты проекта, но лучше всего делать отдельный набор тестов именно под планируемый рефакторинг. Небольшой набор тестов, акцентирующийся на наиболее важных моментах в плане стоимости потенциальных ошибок. И делать рефакторинг надо небольшими шагами, коммит за коммитом в отдельной ветке и соответствующим тестированием. Далее по завершении — тестировать всю ветку и весь набор тестов, далее запуск тестов проекта и проверка уже в режиме обычного тестового пайплайна проекта с выкатыванием в изолированное «рабочее» окружение и пр.
Если не лениться и следовать правильным подходам при рефакторинге по TDD (сначала тест, потом код) — то и проблем особых не будет.

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

Прошу прощения. Мне это показалось само собой разумеющимся и поэтому никак не упоминал. Без покрытия тестами бессмысленно говорить о каком-либо изменении. Ведь, в таком случае, критерием приёмки может быть только полное регрессионное тестирование всей системы.

НЛО прилетело и опубликовало эту надпись здесь

Ой да ладно. Так ли много реальных случав, когда тесты невозможно написать? Ну может прямая работа с железом.
Как-то же проверяется работоспособность кода человеком? Обычно это как-то всегда в той или иной степени может быть автоматизировано.

НЛО прилетело и опубликовало эту надпись здесь
И проверка работоспособности всего продукта человеком скорее эквивалентна интеграционным тестам, а тут про юнит-тестирование.

Так и писать интеграционные. Их то уж точно всегда можно. Они и полезнее, с-но, юнит-тесты в контексте рефакторинга вообще не помогают.

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

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

Что значит рефакторинг за быстродействие?
Для быстродействия занимаются оптимизацией, а никак не рефакторингом. Часто бывает наоборот, что рефакторинг замедляет программу, а оптимизация усложняет исходный код.

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

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

Ну, я имел ввиду то, что как при рефакторинге может произойти оптимизация, так и оптимизация может вызвать рефакторинг.
Лично я считаю что оптимизация не может производиться без рефакторинга (ну, в 99% случаев ИМХО).
Иногда даже бывают случаи что код качественный, рефакторить нечего, но надо оптимизировать, а без переписывания энного колва строк невозможно произвести оптимизацию.
Да и оптимизации оптимизациям рознь… В каких то проектах заменить пузырьковую сортировку является оптимизацией, а в каких-то добавить \ при вызове стандартных функций PHP…
Я не скажу про других, скажу про себя: если я что-то рефакторю, я пытаюсь выбрать какую-то середину между машинами и людьми, ибо если рефакторить для людей — будет медленно, если оптимизировать для машин — будет непонятно людям, я пытаюсь найти золотую середину, когда и быстро, и понятно.

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

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

Пользователю реально пофиг на рефакторинг, его интересует стабильность и функциональность, а рефакторинг не об этом.
Ну если у вас рефакторинг не об этом, то вопросов более не имею.

Я становлюсь владельцем отрефакторенного кода и все вопросы по нему, в первую очередь, будут направлены ко мне
«Чужой» код никогда не рефакторю, если за что-то берусь, то и всю ответственность на себя беру и делаю с этим кодом всё что хочу (под мою персональную ответственнось), слава б-гу на текущем месте у нас практика такая, что у каждого проекта или функциональной части проекта есть одно конкретное ответственное лицо/автор и ответственность переходит к кому-то только в случаях форс-мажора с предыдущим автором.
«Чужой» код никогда не рефакторю

И что вы совсем с легаси не работаете?!
Или работая с легаси оставляете всё то говно, что в нём есть?!
Нафига вам тогда нужен рефакторинг, если чужой говно-код не трогаете совсем?!
Чтобы похвастаться как у вас лично красиво, и какой говно код написали другие?
Рефакторинг — нужен, чтобы из говна сделать конфетку!
А если чужой говнокод не подвергать рефакторингу, то толку от рефакторинга в проекте — не будет никакого.
Чужой в смысле авторства и ответственности в данный конкретный момент, могу только аккуратно указать автору на недостатки, к сожалению не получится весь отдел уволить и меня клонировать на все рабочие места.
Естественно на мне сейчас есть и куча легаси от предыдущих деятелей, кою рефакторю по полной программе, покрываю тестами, заменяю пробелы на табы и прочие непотребства какие в голову придут.
Согласен с тезисом, что рефакторинг — признак юности, но не согласен с тем, что это плохо.
По коллегам замечаю, что с возрастом сильно падает энтузиазм. Человек привык к продукту (если не меняет место), привык к его «особенностям» — читай странностям архитектуры, и уже устал что-либо менять. В результате новичок, попавший на проект, обречён на разбирание килотонн накопившегося архитектурного бреда.
Поэтому считаю, что стремление к рефакторингу — не проблема человека, а лишь свидетельство присутствия энтузиазма.
Субъективно: рефакторинг — это юношеское "заболевание". По личным наблюдениям, где-то после 26 лет начинает отпускать.

Смотря как смотреть на рефакторинг, если на него смотреть "Ух… сейчас все перепишу", то да, это юношеское.


В моем понимании рефакторинг это постоянный процесс. Если при разработке фичи постоянно делать рефакторинг, то не придется делать это в тихую от заказчика, причины просты, при написании фичи рефакторинг займет 5-15 минут, завтра 30 минут, через месяц 5ч-1день. То есть код надо постоянно поддерживать в нормальном состоянии.


Для легаси кода есть правило 3.


Напомню, что говорю о спонтанном изменении в ходе реализации фичи, а не о запланированных изменениях.

Рефакторинг должен происходить постоянно, в том числе и при разработке фичи, а также по несколько раз при разработке большой фичи. Что такое запланированные изменения? это что то в духе пишем абы как месяц, а потом все переписываем? если так, то такая практика как рефакторинг не работает и вряд-ли это можно назвать рефакторингом(нормальным рефакторингом).


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

У рефакторинга простые цели… что бы код был поддерживаемым и расширяемым.

Представьте, что вы воспользовались сервисом по уборке квартиры. Они пришли, начали уборку, но, по ходу дела, всю мебель в квартире переставили и шторы из гостинной в ванну перевесили с аргументацией "в таких условиях уборщице было приятнее выполнять свою работу". Картинку с "WTF?!" можете домыслить сами.

Для программиста код — это как раз та квартира, в которой он живет 5 дней в неделю по 8 часов. И жить в ней должно быть комфортно. Программист в данном случае — это не уборщица. Бизнесу же плевать на этот комфорт. В данном примере скорее бизнес — это уборщица, которая зачем-то запрещает мне переставлять мебель в моей же квартире.


И просто хорошая цитата про рефакторинг:


for each desired change, make the change easy (warning: this may be hard), then make the easy change
— Kent Beck (@KentBeck) 25 сентября 2012 г.


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


Чтобы сократить эстетического приведения кода в порядок — линтеры. Их можно настроить как хуки, перед комитом/сейвом будет все форматироваться или ругаться. Для похапе (мб и для других) есть кодсниффер, который переоткрывает таск разрабу, который закомитил код не по стилю. Так что некоторые вещи при рефакторинге можно автоматизировать, а другие спасут вашу попу в случае "Мэй Дэй". Но по кодстайлу надо договориться на берегу, чтобы потом не было "так положено".

скобки, расставленные по фен шую, никак не влияют на баги в коде
Они влияют на читабельность кода. А читабельность кода влияет на скорость правок, дополнений и тд. Если в команде единый кодстайл, то каждому будет легче читать чужой код. В общем-то рефакторинг не только (и не столько) про «скобки по феншую».
«Они не влияют на баги в коде» потому что подгонка кодстайла никак не поменяет поведение кода, как наблюдаемое/тестируемое, так и внутреннее. Я не считаю это полноценным рефакторингом, в отличие от переработки логики или архитектуры приложения.
Ну так я и написал, что рефакторинг не столько про код-стайл. Однако автор поста кодстайл приравнял к рефаторингу, поэтому я и обратил внимание.

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

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

Рефакторинг — это уже терапия, плата за скорость разработки в прошлых периодах. Нужно выбрасывать старый код, нужно модифицировать под текущие реальности если в этом ощущается экономическая целесообразность. Понять это можно только экспертно. Все рефакторить — глупо и дорого.
Loriowar ну серьезно, неужели ты всерьез считаешь, что упомянутая проблема с sidekiq'ом появилась из-за того, что кто-то улучшил быстродействие? Так вы хотя бы о проблеме узнали.
По теме, рефакторинг это не заболевание, а постоянный и крайне необходимый процесс. А задача разработчика обосновать эту деятельность (независимо от того, в рамках отдельной задачи ведется работа или нет).

Почему такая проблема появилась — это отдельный вопрос. Но проявилась она именно в ходе "улучшения" того, что и так хорошо работало. Таки да, баг на лицо и архитектура кривая, но цель-то в работе приложения, а не в допиливании того, что уже работает.


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

«повышение читаемости и поддерживаемости» с одной стороны, достаточно субъективные метрики, а, с другой, метрики типа цикломатической сложности в целом аргументом не являются.

А вообще появление багов при рефакторинге часто говорит о том, что его провести было нужно давно. Особенно в случае, если баг прямо связан с не самым очевидным поведением типа amount == 0: с очень большой вероятностью тот, кто делал рефакторинг, не понял, что тут не только сравнение числа с нулём происходит, но и с nil. То есть код был написан нечитаемо, читатель кода не смог понять, что в коде происходит, не заметил неэквивалентности старого и нового кода.
Проявиться она могла в абсолютно любой момент, чуть более «тяжелая» транзакция, чуть более «шустрое» железо с sidekiq'ом и бага проявляется снова, только уже не постоянно, а один раз на миллион. И найти концы в этом случае было бы значительно сложнее. Вобщем, ИМХО, улучшение производительности вследствие банального рефакторинга вам только на руку сыграло.
суть статьи и всего обсуждения, я понял такая, не умеешь делать рефакторинг — не делай

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

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

Зарегистрируйтесь на Хабре, чтобы оставить комментарий

Публикации