Вдохновители
Писать начал после очередного просмотра пулл-реквеста на 150+ файлов, где было люто замешана новая функциональность и рефакторинг существующей. Рефакторинг был не только косметическим, но и логическим, который вызывал наибольшую боль. Например, в Ruby amount == 0 смело заменялся на amount.zero? без учёта того, что для случая nil в amount эти конструкции не эквивалентны. Объяснялось же всё это примерно так: "но по код стандарту так положено!" На логичный вопрос "какая цель следования код стандарту и вообще, какая цель у тебя как разработчика?" человек замыкался в себе и немного виновато повторял "но по код-стандарту же вот так писать надо..." и выглядел как блондинка в магазине одежды, которая не в силах совладать с желанием отрефакторитькупить всё.
Определение
Тема щекотливая, поэтому нужно особое внимание уделить вопросу "кто есть кто?" Так вот, согласно вики, рефакторинг — это процесс изменения внутренней структуры программы, не затрагивающий её внешнего поведения и имеющий целью облегчить понимание её работы.
Со своей стороны хочу сузить границы и определить рефакторинг (в худшем смысле это слова) как любые изменения, которые непосредственно не связаны с решаемой задачей и не изменяют внешнее поведение системы, но выполняются в рамках исходной задачи.
То есть поговорить хочу не о запланированном изменении кодовой базы, для которого очерчена область работ и поставлены конкретные цели, а о спонтанных модификациях, случающихся в ходе разработки.
Ценность продукта
Теперь начну издалека. Исходный код — не цель и не ценность. Конечно, эстетически или художественно он может представлять некоторый интерес, но это исключения. В общем случае, код является инструментом для создания программного продукта, которым кто-либо пользуется. Поэтому, для начала, хорошо было бы определить какие есть ценности в продукте.
Прямые ценности продукта
Тут всё просто. Продуктом пользуются, поэтому прямые ценности это то, что явно щупает/видит/чувствует пользователь. А именно:
- всевозможная функциональность продукта;
- удобство использования (UI/UX, быстродействие, отказоустойчивость и тд).
Второй пункт может вызвать некоторую дискуссию. Ведь многие считают, что это не главное. Так как если функциональность хороша, то не важно во что она завёрнута. Хорошие примеры функциональности без вменяемого UI/UX: Redmine и SAP. Однако я с таким взглядом не согласен и ближе по взглядам к товарищу Алану Куперу и его "Психбольнице в руках пациентов".
Косвенные ценности продукта
Это те ценности, которые сами по себе на пользователя не влияют. Но могут "выстрелить" или "накопиться" и оказать влияние (разной степени тяжести) на продукт или его функциональность.
- Баги. Пограничный случай ценностей. Вторичны они потому, что сами ценности не несут, а забирают её у соседних фичей.
- Чистота. Это про читабельность, модульность, минимизацию входящих компонентов, стандартизацию и унификацию интерфейсов и тд.
- Документированность. Это про код и пояснения для разработчиков, не про бизнес описание или пользовательские юзкейсы. Хорошо иллюстрируется фразой весёлого мужика с одного из собеседований: "Логика в БД написана как книга".
- Масштабируемость/защищённость/безопасность. Пользователь их не видит, пока не случится что-нибудь нехорошее.
Ценности для разработчика
Очень важная категория, которую многие упускают из виду, но которая всегда влияет на результат.
- Код по стандарту.
- Отступы по фен-шую.
- Иные сделки с совестью. Ведь код писал, значит результат за день есть, а значит и польза есть.
- Согласие кода с внутренним миром.
Но давайте быть честными. Всё это не про ценности, связанные с продуктом и конечным пользователем. Это про психологию и личных тараканов.
Взгляд со стороны бизнеса
Для полноты картины надо бы посмотреть на всё это со стороны бизнеса, а не программного продукта. В этом случае, разделение на прямые и косвенные ценности становится довольно банальным: прямые — явно приносят деньги и можно дать однозначную количественную оценку; косвенные — денег не приносят и/или количественную оценку дать очень сложно; ценности для разработчика денег не приносят ни в каком виде (возможно, даже отнимают их).
Например.
- Новая фича с прикручиванием OAuth увеличила количество регистраций на 10% и мы получили +1к$.
- Разделили модуль биллинга на несколько независимых частей, основываясь на паттерне single-responsibility. Вроде как поддерживать проще стало, но измерить никак не получилось.
- Мы привели кодовую базу к соответствию с код-стандартом и… "ничего" не получили.
Стоит отметить, что из вышеозначенных оценок растут ноги у любимого бизнесом ускорения работы, всеобщего поторапливания и нежелания выделять время на что-либо кроме функциональности и, быть может, багфикса. Ведь прямые ценности можно оценить и "продать", а косвенные — очень сложно, либо невозможно. И получается, что косвенные ценности осознаются либо в силу инженерного бэкграунда, либо понимаются на интуитивном уровне, либо выбрасываются как "бесполезные".
Справедливости ради, тут нужно вспомнить понятие enabler'а, который "открывает дорогу" к реализации желаемой фичи, но явного профита пользователю не несёт. Но это уже другая история.
Причём тут рефакторинг?
Как минимум при том, что он может влиять исключительно на косвенные ценности продукта. Поэтому ни от одного рефакторинга пользователю лучше не станет.
Так же, важно помнить про энтропию. Рефакторинг всегда неуменьшает её. Для уменьшения энтропии, в идеале, нужна команда архитекторов, минимизирующих энтропию на этапе проектирования. Процитирую кусочек из Мифического человекомесяца:
Системное программирование является процессом, уменьшающим энтропию, а потому ему внутренне присуща метастабильность. Сопровождение программ есть процесс, увеличивающий энтропию, и даже самое умелое его ведение лишь отдаляет впадение системы в безнадежное устаревание.
Посему, если считать рефакторинг частью процесса сопровождения программы, то чем меньше рефакторить, тем дольше код проживёт без переписывания.
Какие цели могут быть у рефакторинга?
Напомню, что говорю о спонтанном изменении в ходе реализации фичи, а не о запланированных изменениях. В этом случае определение цели полностью лежит на плечах разработчика. Он должен задать себе главный вопрос "зачем?", ответить на него и после этого, двигаться к намеченной цели.
За энтропию!
Такое самому провернуть сложно. А протащить через ревью так и вообще нереально. Цель конечно благая: расчистить плацдарм для новых свершений, а так же вычистить шлаки и токсины, накопленные за время поддержки продукта. Но перелопатить с нуля пару модулей не потеряв ничего по пути — это не самая простая задача. Её надо бы решать совместно с бизнес-аналитиками и архитекторами, если таковые есть. Так что таким мало кто рискует заниматься на добровольных началах.
За документацию!
Это уже проще. Переименовать переменные/функции по принципу "что на коробке, то и в коробке", упростить конструкции за счёт возможностей языка и библиотек, добавить комментариев в неочевидных местах и т.д. Такое можно сделать в одиночку и, при правильном подходе, можно сделать хорошо как себе будущему, так и коллегам соседним.
За быстродействие!
По-честному, такие работы должны быть выделены в отдельную задачу. Так как либо вам достаточно текущих возможностей системы и делать ничего не надо, либо вы знаете что конкретно и на сколько именно нужно ускорить.
В эту категорию входит всё: от безобидного исправления N+1 до серьёзного ускорения за счёт изменения алгоритмов работы. Вся проблема в том, что на скорости всегда сидит "чётность" багов. Вот, к случаю, пример из жизни: внутри одной транзакции пушатся данные в БД и в этой же транзакции ставится задача в Sidekiq; очередь Sidekiq'а в Redis'е и транзакция на него не распространяется; при увеличении скорости работы очереди таска иногда берётся на выполнение раньше, чем закоммитятся данные; последствия и процесс дебага можете представить сами.
За рефакторинг!
Представьте, что вы воспользовались сервисом по уборке квартиры. Они пришли, начали уборку, но, по ходу дела, всю мебель в квартире переставили и шторы из гостинной в ванну перевесили с аргументацией "в таких условиях уборщице было приятнее выполнять свою работу". Картинку с "WTF?!" можете домыслить сами.
Надеюсь, вы поняли, что думать нужно не о себе, а о том, для кого вы это делаете.
Смирение и принятие
В завершении, обязательно нужно дать "мануал" о том, что делать перед рефакторингом. Правда это не TODO-лист, а, скорее, список фактов, с которыми нужно смириться и, либо принять, либо не приступать к действу.
- Я увеличиваю количество багов в проекте и могу за это получить в бубен.
- Я становлюсь владельцем отрефакторенного кода и все вопросы по нему, в первую очередь, будут направлены ко мне.
- Своими действиями я не произвожу ничего ценного для конечного пользователя.
Немного пояснений.
- Любое изменение кода имеет ненулевой шанс породить баг. А раз это действие не связано с конечной функциональностью, то вы просто плодите баги без генерации основных ценностей. Хорошо это осознавать и не прикидываться шлангами, когда к вам придут с вопросами.
- Отмазки вида "anotate previous" и подобные очень убоги, ибо в том же гитхабе/гитлабе нет такой фичи. Плюс, предыдущий автор подтвердил, что именно в его конфигурации всё работает, а за ваши изменения он ответственности не несёт и теряет часть картины происходящего. Точнее, вы её забираете у него вместе с ответственностью.
- Пользователю реально пофиг на рефакторинг, его интересует стабильность и функциональность, а рефакторинг не об этом.
И ещё раз: если не согласны с хотя бы одним из пунктов — не начинайте рефакторинг. Лучше почитайте хабр, лурк, чаю выпейте или, на худой конец, запилите следующую фичу с дашборда.
Конец
Не судите строго. По возможности, критикуйте конструктивно. И всегда задумывайтесь над целью своих действий. Спасибо.
Комментарии (67)
DigitalSmile
28.06.2018 10:21+3Не согласен с автором. Проблема в начале статьи про большой пулл-реквест именно в том что пулл-реквест большой. А не в том, что там рефакторинг.
Просто не смешивайте логику и рефакторинг в одном коммите и будет вам счастье. И команду тренируйте чтобы так не делали (методом металлической полметровой линейки).
А то по такой логике получается нафиг его делать не нужно, оно же все ломает.Oxoron
28.06.2018 10:39+2Просто не смешивайте логику и рефакторинг в одном коммите и будет вам счастье.
Люто плюсую. Если вижу проблему в коде который собираюсь менять — сперва коммит на рефакторинг (с пометкой в сообщении), затем пулл-реквест, и только потом собственно разработка. Пулл-реквесты удваиваются, но сложность каждого падает. Коллеги смирились (читай такой подход — дело вкуса). Обоснование для босса: «трачу полчаса на минимизацию рисков при изменении бизнес-логики».Loriowar Автор
28.06.2018 11:10+1Да. В идеале так и хочется делать. Но далеко не всегда и не у всех получается. Плюс, многие не понимают зачем так делать. Поэтому и писал, чтобы попробовать растолковать очевидные вещи.
VolCh
29.06.2018 21:50+1А пулл-реквест с одним рефакторингом вполне могут закрыть с вердиктом «никак не относится к задаче».
Oxoron
30.06.2018 00:41Команда 5 человек, все рядом, в крайнем случае спросят. Рефакторится только код который реально будет изменяться для решения задачи. Нормальное сообщение со ссылкой на задачу в трекере решает 99% пулл-реквестов. Честно говоря, не помню ни одного реквеста закрытого по «никак не относится к задаче».
Были возражения «на твои реквесты приходится отвлекаться в три раза чаще» (ибо подготовка-задача-подчистка). Потом один коллега выдал реквест на 60 файлов — и возражения в мой адрес резко спали. Контраст-с.
Наконец, вмелкихатомарных PR есть своя прелесть. Тех сотрудников, кто подтверждает не глядя, не мучает совесть. LGFM на атомарный PR чисто морально написать проще. Для любителей принципов\паттернов идея «SRP в коде, SRP в коммите, SRP в пулл-реквесте» греет душу. Фанаты чистого кода наслаждаются дистилированным рефакторингом. Иногда выношу в отдельный PR-изменения не затрагивающие функциональность (новые модели, хелперы не используемые пока в коде, и т.д.) — такие реквесты безопасны, ибо «активный» код не меняется вообще, но полезны (ибо фронт-энд девелопер видит ту модель что я ему отошлю и валидирует её).
Хоть техника и заслуживает кое-где права на жизнь, недостатки у неё имеются. Например, трудно связать бизнес-изменения с подготовительным рефакторингом — разделенные по реквестам они теряют общий контекст. Если подготовка прошла, а основное изменение нет — приходится откатываться. Иногда локальный рефакторинг отходит от общего стиля (например, в одном методе меняется подход к валидации); локально код стал лучше, глобально — ХЗ.
Короче, техника на любителя и ситуацию. На некоторых проектах я её даже пробовать не стал. На регулярной основе никто из коллег её не повторяет, обычно бьют подготовку\выполнение\зачистку по коммитам.
botyaslonim
28.06.2018 10:37+1Задачи отвечай смысл? же понятен тебе
Я где-то после 35 резко понял, что код пишется не для машин, а для людей. Соответственно, рефакторинг часто призван сделать код читаемым, поддерживаемым, а всю архитектуру более логичной и эффективной. Ну, например, валидации у нас производились в трёх местах, причём в одном из них ещё и неправильно. Мы выделили всё в отдельный модуль. И т.д.
В примере выше слова вроде все понятные, но расставленные «не по гайдлайнам». Собственно, любой язык программирования — почти то же, что и любой другой язык общения. И да, мы используем правильный порядок слов и знаки препинания. Так быстрее и проще читать. Причём в некоторых языках (в китайском, например) порядок слов задан ещё жёстче.
Для бизнеса в целом вовремя сделанный рефакторинг означает меньшие расходы на поддержку и дальнейшую разработку. Массовая доля костылей в коде тупо повышает степень функции скорости разработки от времени жизни IT-компанииOxoron
28.06.2018 10:44+1Вы в целом правы, но, к сожалению, за вашими словами скрывается одно «но»:
вовремя сделанный рефакторинг
Осталось только угадать своевременность и согласовать с боссом.botyaslonim
28.06.2018 10:45+2Эта проблема не имеет простого решения. Босс либо понимает, либо… эффективный менеджер
usego
28.06.2018 16:42Босс может видеть то, что не видете вы. Например что он продаст компанию через 2 года и на вопросы лонг терм суппорта ему вообще покласть. И да, он примет эффективное решение дать по рукам программисту и запретит заниматься перфекционизмом.
nsinreal
28.06.2018 20:55+2Эта проблема называется "недостаток коммуникации" и "микроменджмент".
Вместо того, чтобы объяснить суть поставленных задач босс пытается выполнять работу программиста.
Естественно, что:
1) босс будет принимать технические решения хуже, чем программист
2) програмист будет считать босса некомпетным самоуверенным дебилом, потому что не знает обстоятельств
3) даже если программист будет выполнять требования, то это будут не те требования, которые изначально нужныufm
30.06.2018 05:56Вы уверены, что босс обязан рассказывать программисту о том, что он собирается продать компанию через 2 года?
vintage
30.06.2018 11:04А вы уверены, что программист захочет тратить 2 года своей жизни на проект, который даже в резюме будет записать стыдно? Давайте всё же признаем, что у "босса" и работника разные приоритеты. "босс" не в рабство берёт, а нанимает специалиста с конкретными навыками, подходом к работе, стремлениями и тараканами. И всё это надо учитывать. Особенно в условиях дефицита кадров. Мой опыт говорит, что говнокодить — себе дороже. Когда твой код будут оценивать, никто не будет уточнять в каких условиях и почему написано именно так. А программисту, который пусть и быстро, но говнокодит, никогда не доверят действительно крупный проект.
qw1
30.06.2018 12:09Босс не имеет гарантий, что программисты не расскажут коммерческие планы компании всему миру. Конечно, можно юридически оформить режим коммерческой тайны, но тут программисты скорее выберут оставаться без информации, чем получить её вместе с возможной уголовной ответственностью за разглашение.
Druu
30.06.2018 12:17Вы уверены, что босс обязан рассказывать программисту о том, что он собирается продать компанию через 2 года?
Если босс хочет, чтобы программист работал эффективно — не то чтобы обязан, просто других вариантов нет.
nsinreal
01.07.2018 00:20Я только сейчас понял, что рассказывать не даёт гарантии.
Босс пытается поступать неэтично по отношению к будущему покупателю. Программист может обоснованно посчитать босса гондоном. Тем более, что при продаже компании сотрудники останутся.
Ну, что ж. Наверное в список проблем стоит добавить "босс — гондон". Честно, я совсем не имею понятия, что советовать гондонам. Я абсолютно некомптентен в вашем вопросе
qw1
01.07.2018 16:10Почему не этично? Возьмём, к примеру, Minecraft, который Нотч продал в Microsoft за $2.5 млрд.
Он, как получил деньги, сразу умыл руки с проекта. Команде, конечно, неприятно. Но покупателю важна только юзер-база и бренд. Им не нужен ни этот программист, ни код, они сами всё допишут.
В любом другом стартапе босс так же может считать — сейчас главное нафигачить всё начерновую, лишь бы реализовать свои гениальные идеи (пусть и с глюками), занять рынок, доказать, что идеи верные и продать тому, кто может вести проект технически грамотно, но что-то новое придумать не способен.
rjhdby
28.06.2018 10:51+1Мне кажется, что цитата старины Мартина будет кстати.
Чтобы написать чистый код, мы сначала пишем грязный код, а затем рефакторим его.
avraam_linkoln
28.06.2018 13:37+1Сначала мы пишем так, чтоб работало, потом рефакторим и после этого убеждаемся что всё работатет
OldMaster
28.06.2018 10:57Хорошая статья. В некоторых местах узнал самого себя.
На мой взгляд, есть ряд ситуатций где с автором можно поспорить:
- Разработка библиотеки для ее использования в конечных продуктах. Тут косвенные ценность становятся прямыми, т.к. «пользователи» — это сами разработчики.
- Разработка кода в коллективе более трех человек. В этом случае рефакторинг часто возникает стихийно в местах «столкновения». Но тут все сильно зависит от возраста и темперамента разработчиков.
immaculate
28.06.2018 11:10+1Я думаю, нельзя однозначно сказать. Иногда код очень плохой. Не дышать на него и заниматься исключительно разработкой новых фич для бизнеса — значит десятки раз наступать на одни и те же грабли.
Например, почти во всех production проектах есть участки скопированного кода. Где-то кто-то поленился один раз, потом два раза, потом три. В итоге, у нас дюжина участков, где находится copy-paste код. При каждом изменении надо не забыть пройтись по всей дюжине и внести идентичные правки. Если где-то забыть, то в лучшем случае функциональность сломатеся сразу. Но так не бывает. Обычно где-то потом спустя несколько дней-недель-месяцев начинают случаться необъяснимые баги. На отладку которых может уйти от часа до недели.
Что лучше: каждый раз править дюжину скопированных кусков кода, или все-таки один раз сесть, отрефакторить, вынести общий код в одно место (модуль, класс, утилиту), и потом не терять время во время последующих изменений и отладки?
Однозначного ответа дать нельзя.
К тому же, у всех разные требования, разный ритм работы, и разная восприимчивость. Кто-то может работать с плохим кодом спокойно, а кому-то видеть что-то вроде:
var priceStr = priceInput.value; var newPriceStr = priceStr.replace(/\./g, ''); var newPriceStr1 = newPriceStr.replace(/\,/g, '');
все равно что ногтем по доске. И когда каждый день видишь это снова и снова, то становится нехорошо, пропадает настроение и мотивация
avraam_linkoln
28.06.2018 11:11Автор забыл в ценностях продукта написать что в идеале поддержка продукта должна длиться вечно.
sojuznik
28.06.2018 11:16Рефакторинг нужен для одной вещи — обеспечение устойчивости к изменениям. Когда проект разрабатывается или модифицируется, разница между планируемой архитектурой и действительным положением вещей постоянно увеличивается. Это происходит потому, что в самом начале неизвестно, как на самом деле все будет работать в релизе, как этот код придется изменять. Постепенно первоначальные представления о продукте меняются и отлично-спроектированный код постепенно перестает быть отлично-спроектированным для изменившихся условий. Вот тогда и осуществляется рефакторинг.
Изменения могут быть на разных уровнях: и на уровне архитектуры и на уровне какой-то одной мелкой функции. Поэтому и рефакторинг может быть на разных уровнях.
Например, программист Вася планирует реализовать алгоритм и вводит в свою мелкую функцию переменную Counter. Он называет так ее потому, что на этом этапе он так думает. Но по мере разработки он понимает, что название было неудачным и начинает мешать ему лично лучше понимать код. Т.е. условия изменились и примененное техническое решение (переменная и ее название) уже не соответствует текущему состоянию кода. Он делает рефакторинг на своем уровне, переименовывает эту переменную. Теперь новое название Item больше соответствует текущему состоянию кода.
Усложните этот пример, увеличив уровень абстракции, и вы увидите, что подобные вещи постоянно происходят и там. Неудачно названа функция, неудачно выбраны параметры и их типы, структуры данных хранят много всего, что нужно переносить в другое место, некоторый код слишком сложно написан, некоторая часть кода вообще не ясно как работает, а эта подсистема имеет слишком много лишних связей с другой подсистемой. Все это мешает дорабатывать и менять код на каждом из уровней.
И рефакторинг имеет ценность для бизнеса. Если программист или менеджер проекта не видят этой ценности, то это означает лишь одно: эти лица не понимают, как себя ведет продукт в изменениях.
Рассматривайте рефакторинг как заточку инструмента. Можно работать тупым ножом, но лучше его заточить. Работать станет быстрее и приятнее.
Или рассматривайте рефакторинг как приваривание ушек для крепления одной тяжелой конструкции к другой. Можно без этих мест крепления привязать веревочками или изолентой прикрутить, поначалу все будет работать, но потом нас ждет катастрофа.
Обычно программист не доводит себя до катастрофы, пытаясь переписать код или ища возможность все-то прикручивать сваи не изолентой, а стальным тросом. И мучается, постоянно борясь со сложностями и неудобствами. В итоге его производительность падает, код становится ненадежным, изменение становится настолько дорогим, что программист всячески избегает изменений. Тупик. И бизнес за это платит! Незаметно из кармана владельца уходят суммы, сравнимые с зарплатой программистов. А программисты работают в более сложных условиях, мучая себя и желая избегать работу. Или и вовсе, начинают подумывать об уходе.
Так что, если хочется мазохизма, если хочется побольше вбухать бабла в программирование, не делайте рефакторинг, спихивайте это на тех, кто придет на ваше место после вашего увольнения, спихивайте на архитекторов, на коллег.
PS. У меня есть проекты, пережившие 12 мажорных версий. Если бы не рефакторинг, я бы уже давно разорился.Loriowar Автор
28.06.2018 11:19В сказанном вы правы. Но я ведь про другой аспект говорил. Рефакторить надо осознанно и в согласии с бизнесом, а не самому, потому что так решил. Сказать бизнесу, что "вот тут и тут — фигня; она нам помешает вот в таких-то случаях; сделать хорошо займёт столько-то дней". Обсудить детали, запланировать и честно сделать хорошо. А не отрывать кусок времени от основной задачи и вязнуть в переписывании того, что и так хорошо работает, но не удовлетворяет лично разработчика.
sojuznik
28.06.2018 12:05Совершенно верно. У меня нет серьезных замечаний. Я добавил те аспекты, какие балансируют понимание рефакторинга.
Еще немного дополнений с практической стороны:
Наилучшая стратегия для рефакторинга — делать его тогда, когда код меняется, когда старое состояние кода начинает мешать дальнейшей разработке или планируемым позже изменениям. «Мешать» — это когда хочется обойти код заплатками, надстройками и прочей «изолентой». Или когда спотыкаешься о код и видишь, что это тормозит работе и даже мешает. Но не когда испытываешь эстетическое неприятие кода. Даже идеально написанный код может быть эстетически неприятным. Не все алгоритмы красивы, некоторые уродливы. И никаким рефакторингом это уродство не убрать.
Рефакторить код там, где его не видно лично программисту, не надо. Не надо специально выискивать плохие места и специально уделять часть времени на такой поиск и такое «сервисное обслуживание». Рефакторинг должен быть частью изменения кода.
Даже если в программе есть места, какие «воняют» очень сильно, но их не видно за интерфейсами или приватным кодом, не надо ничего там менять. Это место работает и не мешает всему остальному. Вот когда начнет мешать, тогда и надо будет рефакторить. Только надо понимать, что мешать приватный код может только тогда, когда на уровне абстракции выше от него требуется решение задачи с иными требованиями. И тут, возможно, не рефакторинг понадобится, а разработка более соответствующего требованиям модуля.
Если делать рефакторинг вовремя, то его сложность и цена изменений становятся небольшими. Если делать раньше или делать «от нечего делать» или «для красоты» — это пустая работа. У меня было много опыта наведения лоска на код, какой потом приходилось сносить полностью. Когда видишь, сколько впустую потратил на это времени, начинает доходить.
Баланса достичь трудно, если не смотреть на код системно. К сожалению, системный взгляд приходит только с опытом.
Это можно ускорить, если начать читать книги:
1. Классиков Agile. Из Agile пришел рефакторинг таким, каким мы его сейчас знаем. Там это — основной инструмент, потому что приниципы разработки предполагались как постоянное изменение кода в условиях неизвестности. Даже если разработка не ведется по идеологии Agile, можно перенести параллели на свои проекты и смотреть, как что меняется и почему.
2. По системному анализу и теории систем. Сейчас это преподают в ВУЗах как стандартный предмет. И это правильно. Сейчас программирование настолько сложное, что нельзя ухватить все в уме. Системный взгляд позволяет успешно проектировать и менять код в таких условиях.
Также рекомендую вести свой личный проект помимо основной работы, как тренировочный инструмент. Меняйте проект после каждой версии так, чтобы не просто добавлять какие-то функции, а менять что-то серьезное (мажорные изменения версии). Как упражнение, можно сделать плавную переделку в совсем иное. Но не переписывать с нуля! Это поможет лучше понимать, как проектировать устойчиво к изменениям, когда и как делать рефакторинг.
Касаемо бизнеса. Чтобы лучше понимать бизнес, нужно самому иметь свой бизнес. Для программиста возможность лучше понимать бизнес — это подработки или shareware. Или совсем иные сферы, но касаемо своих хобби. Пока нет своего бизнеса, пока сам не принимаешь бизнес-решения и не видишь последствия, взгляды на бизнес в целом могут быть верными, но нюансы решают все.
lair
28.06.2018 11:37Со своей стороны хочу сузить границы и определить рефакторинг (в худшем смысле это слова) как любые изменения, которые непосредственно не связаны с решаемой задачей и не изменяют внешнее поведение системы, но выполняются в рамках исходной задачи.
Ну то есть вы ввели определение, которое не вытекает из оригинального, и теперь рассказываете нам, как это плохо?
Своими действиями я произвожу ничего ценного для конечного пользователя.
Вы забыли один важный нюанс: ничего ценного сейчас. Однако завтра этот же пользователь получит свою фичу раньше, потому что рефакторинг упростил внесение изменений в систему.
Loriowar Автор
28.06.2018 12:33Или завтра пользователь получит баг, который не даст пользоваться любимой фичёй. Рулетка же. Поэтому хотелось бы согласия между бизнесом и разработкой. А не как в большинстве случаев: бизнесу нужна фича и не интересуют детали, а разработчики партизански меняет код без явного описания потребности этого действа.
lair
28.06.2018 12:35Рулетка же.
Не больше чем при создании новой фичи без рефакторинга или починке другого бага.
бизнесу нужна фича и не интересуют детали
Если бизнес не интересуют детали, то явное описание никому не поможет.
Loriowar Автор
28.06.2018 12:41Так об этом и речь. Не надо заниматься самодеятельностью. Если сами пытаетесь героически спасать код, но бизнесу это не интересно, то ничего не получится. Лучше начать с обсуждения целей и причин, нежели бежать и код писать.
lair
28.06.2018 12:45Если сами пытаетесь героически спасать код, но бизнесу это не интересно, то ничего не получится
Но получается же.
Лучше начать с обсуждения целей и причин
"бизнесу не интересны детали".
usego
28.06.2018 16:45Детали в виде новых багов там, где их годы не было, будут очень даже интересны.
lair
28.06.2018 16:49… а при разработке новых фич или багфиксинге такого, конечно, никогда не бывает?
usego
28.06.2018 16:51конечно, но бизнес в курсе, что над этим функционалом идёт работа и ожидать надо всякое. А вот если втихую начать шлифовать что-то старое, никак не связанное с актуальным беклогом — тут жди вопросов чё ты туда вообще полез.
lair
28.06.2018 16:53конечно, но бизнес в курсе, что над этим функционалом идёт работа и ожидать надо всякое.
Я и говорю: у вас никогда так не бывает, что идет работа над одной функциональностью, а баги внезапно лезут в другой, где их годы не было?
А вот если втихую начать шлифовать что-то старое, никак не связанное с актуальным беклогом
А этого, заметим, я делать не предлагал.
Busla
28.06.2018 11:42в Ruby amount == 0 смело заменялся на amount.zero? без учёта того, что для случая nil в amount эти конструкции не эквивалентны
Поведение программы стало более однозначным, потому это и прописано в стандарте. А ловить nil сравнивая с нулём — это хак на грани говнокода.
slava_k
28.06.2018 12:31+1Большинство проблем с рефакторингом из-за того, что его начинают делать без самого важного этапа перед ним — написание тестов для того кода, который планируется менять. И именно тесты — залог что ничего не сломается. Это, как минимум, текущие тесты проекта, но лучше всего делать отдельный набор тестов именно под планируемый рефакторинг. Небольшой набор тестов, акцентирующийся на наиболее важных моментах в плане стоимости потенциальных ошибок. И делать рефакторинг надо небольшими шагами, коммит за коммитом в отдельной ветке и соответствующим тестированием. Далее по завершении — тестировать всю ветку и весь набор тестов, далее запуск тестов проекта и проверка уже в режиме обычного тестового пайплайна проекта с выкатыванием в изолированное «рабочее» окружение и пр.
Если не лениться и следовать правильным подходам при рефакторинге по TDD (сначала тест, потом код) — то и проблем особых не будет.
wert_lex
28.06.2018 13:26+1В статье ни разу не упомянули про тесты. Первое правило рефакторинга — сначала тест и только потом код.
Loriowar Автор
28.06.2018 15:07Прошу прощения. Мне это показалось само собой разумеющимся и поэтому никак не упоминал. Без покрытия тестами бессмысленно говорить о каком-либо изменении. Ведь, в таком случае, критерием приёмки может быть только полное регрессионное тестирование всей системы.
0xd34df00d
29.06.2018 23:17Вам повезло, если для предыдущей версии кода вообще можно написать тесты.
wert_lex
30.06.2018 17:06Ой да ладно. Так ли много реальных случав, когда тесты невозможно написать? Ну может прямая работа с железом.
Как-то же проверяется работоспособность кода человеком? Обычно это как-то всегда в той или иной степени может быть автоматизировано.0xd34df00d
30.06.2018 18:07Я про качество кода, а не про предметную область.
И проверка работоспособности всего продукта человеком скорее эквивалентна интеграционным тестам, а тут про юнит-тестирование.Druu
01.07.2018 04:54И проверка работоспособности всего продукта человеком скорее эквивалентна интеграционным тестам, а тут про юнит-тестирование.
Так и писать интеграционные. Их то уж точно всегда можно. Они и полезнее, с-но, юнит-тесты в контексте рефакторинга вообще не помогают.
stu5002
28.06.2018 15:08Очень интересное мнение на самом деле, что рефакторинг приводит к увеличению багов.
И что разработчику нужно любой рефакторинг согласовывать с бизнесом. То есть, безусловно, если бизнес — это компания из 5 человек и владелец бизнеса — сам принимает участие в разработке, то это разумно. Иначе — это как если бы повара в ресторане согласовывали уборку своего рабочего места каждый вечер с владельцем.
Поэтому ни от одного рефакторинга пользователю лучше не станет.
Ну раз такое дело, то заодно можно распустить и весь обслуживающий персонал в IT-компаниях, а так же большую часть менеджмента, hr-ов и вот это вот все. Не просто так, а потому, что они не занимаются непосредственно разработкой фич, которые должны сделать лучше пользователям.
За быстродействие!
Что значит рефакторинг за быстродействие?
Для быстродействия занимаются оптимизацией, а никак не рефакторингом. Часто бывает наоборот, что рефакторинг замедляет программу, а оптимизация усложняет исходный код.POPSuL
29.06.2018 05:37Ну почему же? Открываешь какой-нибудь метод, видишь там непонятную кашу которую кто-то когда-то написал, переписываешь, сокращаешь метод на пол сотни строк без потери функциональности, и в итоге и отрефакторил, и ускорил.
kester
29.06.2018 16:33Быстродействие не может быть целью рефакторинга.
Оно может быть целью оптимизации, а она противоположная рефакторингу.
То, что рефакторинг может увеличить быстродействие, это только приятный бонус, но никак не цель. Большая вероятность, что случиться наоборот.
Простое правило: рефакторинг для людей, оптимизация для машины.POPSuL
29.06.2018 17:28Ну, я имел ввиду то, что как при рефакторинге может произойти оптимизация, так и оптимизация может вызвать рефакторинг.
Лично я считаю что оптимизация не может производиться без рефакторинга (ну, в 99% случаев ИМХО).
Иногда даже бывают случаи что код качественный, рефакторить нечего, но надо оптимизировать, а без переписывания энного колва строк невозможно произвести оптимизацию.
Да и оптимизации оптимизациям рознь… В каких то проектах заменить пузырьковую сортировку является оптимизацией, а в каких-то добавить\при вызове стандартных функций PHP…
Я не скажу про других, скажу про себя: если я что-то рефакторю, я пытаюсь выбрать какую-то середину между машинами и людьми, ибо если рефакторить для людей — будет медленно, если оптимизировать для машин — будет непонятно людям, я пытаюсь найти золотую середину, когда и быстро, и понятно.
dipsy
28.06.2018 17:39Любое изменение кода имеет ненулевой шанс породить баг.
Рефакторинг преследует цель в т.ч. и уменьшить количество «неуловимых» багов за счет более SOLID-архитектуры нового кода, и уменьшение дальнейших проблем при изменениях, меньше шансов появления новых неуловимых багов.
когда к вам придут с вопросами
По проблемам с legacy-кодом прибегают гораздо чаще, причем проблемы из серии «зависает ваша система, второй раз уже за месяц, срочно устраните!» и делай с этим что хочешь.
Пользователю реально пофиг на рефакторинг, его интересует стабильность и функциональность, а рефакторинг не об этом.
Ну если у вас рефакторинг не об этом, то вопросов более не имею.
Я становлюсь владельцем отрефакторенного кода и все вопросы по нему, в первую очередь, будут направлены ко мне
«Чужой» код никогда не рефакторю, если за что-то берусь, то и всю ответственность на себя беру и делаю с этим кодом всё что хочу (под мою персональную ответственнось), слава б-гу на текущем месте у нас практика такая, что у каждого проекта или функциональной части проекта есть одно конкретное ответственное лицо/автор и ответственность переходит к кому-то только в случаях форс-мажора с предыдущим автором.GeekberryFinn
28.06.2018 17:48«Чужой» код никогда не рефакторю
И что вы совсем с легаси не работаете?!
Или работая с легаси оставляете всё то говно, что в нём есть?!
Нафига вам тогда нужен рефакторинг, если чужой говно-код не трогаете совсем?!
Чтобы похвастаться как у вас лично красиво, и какой говно код написали другие?
Рефакторинг — нужен, чтобы из говна сделать конфетку!
А если чужой говнокод не подвергать рефакторингу, то толку от рефакторинга в проекте — не будет никакого.dipsy
28.06.2018 17:52Чужой в смысле авторства и ответственности в данный конкретный момент, могу только аккуратно указать автору на недостатки, к сожалению не получится весь отдел уволить и меня клонировать на все рабочие места.
Естественно на мне сейчас есть и куча легаси от предыдущих деятелей, кою рефакторю по полной программе, покрываю тестами, заменяю пробелы на табы и прочие непотребства какие в голову придут.
ICELedyanoj
28.06.2018 18:38+1Согласен с тезисом, что рефакторинг — признак юности, но не согласен с тем, что это плохо.
По коллегам замечаю, что с возрастом сильно падает энтузиазм. Человек привык к продукту (если не меняет место), привык к его «особенностям» — читай странностям архитектуры, и уже устал что-либо менять. В результате новичок, попавший на проект, обречён на разбирание килотонн накопившегося архитектурного бреда.
Поэтому считаю, что стремление к рефакторингу — не проблема человека, а лишь свидетельство присутствия энтузиазма.
inew-iborn
28.06.2018 20:44Субъективно: рефакторинг — это юношеское "заболевание". По личным наблюдениям, где-то после 26 лет начинает отпускать.
Смотря как смотреть на рефакторинг, если на него смотреть "Ух… сейчас все перепишу", то да, это юношеское.
В моем понимании рефакторинг это постоянный процесс. Если при разработке фичи постоянно делать рефакторинг, то не придется делать это в тихую от заказчика, причины просты, при написании фичи рефакторинг займет 5-15 минут, завтра 30 минут, через месяц 5ч-1день. То есть код надо постоянно поддерживать в нормальном состоянии.
Для легаси кода есть правило 3.
Напомню, что говорю о спонтанном изменении в ходе реализации фичи, а не о запланированных изменениях.
Рефакторинг должен происходить постоянно, в том числе и при разработке фичи, а также по несколько раз при разработке большой фичи. Что такое запланированные изменения? это что то в духе пишем абы как месяц, а потом все переписываем? если так, то такая практика как рефакторинг не работает и вряд-ли это можно назвать рефакторингом(нормальным рефакторингом).
В этом случае определение цели полностью лежит на плечах разработчика. Он должен задать себе главный вопрос "зачем?", ответить на него и после этого, двигаться к намеченной цели.
У рефакторинга простые цели… что бы код был поддерживаемым и расширяемым.
Telichkin
28.06.2018 22:10+1Представьте, что вы воспользовались сервисом по уборке квартиры. Они пришли, начали уборку, но, по ходу дела, всю мебель в квартире переставили и шторы из гостинной в ванну перевесили с аргументацией "в таких условиях уборщице было приятнее выполнять свою работу". Картинку с "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 г.
chelovekkakvse
28.06.2018 22:55Осознание цели, в любой ситуации, это необходимость. Однако не согласен, что рефакторинг нельзя оценить в попугаях. Банально — нечитаемый и неправильно спроектированный код увеличивает время разработки новых фич и модули и экспоненциально возрастает риск, что в один момент упадет все. Кроме того, экстренная реанимация также увеличивается во времени, т.к. надо сидеть и разбираться, чё тут написали.
Чтобы сократить эстетического приведения кода в порядок — линтеры. Их можно настроить как хуки, перед комитом/сейвом будет все форматироваться или ругаться. Для похапе (мб и для других) есть кодсниффер, который переоткрывает таск разрабу, который закомитил код не по стилю. Так что некоторые вещи при рефакторинге можно автоматизировать, а другие спасут вашу попу в случае "Мэй Дэй". Но по кодстайлу надо договориться на берегу, чтобы потом не было "так положено".
Antervis
29.06.2018 05:15скобки, расставленные по фен шую, никак не влияют на баги в коде
chelovekkakvse
29.06.2018 08:51Они влияют на читабельность кода. А читабельность кода влияет на скорость правок, дополнений и тд. Если в команде единый кодстайл, то каждому будет легче читать чужой код. В общем-то рефакторинг не только (и не столько) про «скобки по феншую».
Antervis
29.06.2018 10:47«Они не влияют на баги в коде» потому что подгонка кодстайла никак не поменяет поведение кода, как наблюдаемое/тестируемое, так и внутреннее. Я не считаю это полноценным рефакторингом, в отличие от переработки логики или архитектуры приложения.
chelovekkakvse
29.06.2018 11:04Ну так я и написал, что рефакторинг не столько про код-стайл. Однако автор поста кодстайл приравнял к рефаторингу, поэтому я и обратил внимание.
Не соглашусь с Вами по поводу переработки логики программы. Можно оптимизировать алгоритмы, переопределить классы, использовать паттерны, но если у вас на выходе должен быть массив данных определенного формата, то он де должен и остаться после реафкторинга.
AngusMetall
29.06.2018 16:20Да да, был у нас на работе новичек, брали как синьёра, когда на кодревью ему указали на то, что у нас принято по кодстайлу писать if конструкцию со скобками и на отдельной строке, дал понять, что мы все мудаки и вообще придираемся. На следующем же полуреквесте он с рефакторингом кода накосячил, убрав оператор из блока и соответственно из всего блока if выполнялся только первый оператор, а мы мудаки, да. Держу сонаркуб в режиме стайлнаци, до этого держал стайлкоп, как раз от тех кто считает, что стайл гайды для лохов.
Antervis
29.06.2018 05:12давайте перефразируем «не рефакторить лишний раз» в «до рефакторинга лучше не доводить». То, что сегодня можно быстро доработать за час, через месяц придется переделывать целые сутки, а через полгода придется две недели рефакторить. И дальновидность в определении таких мест очень важна.
А когда проект довели до состояния "наверно проще отрефакторить, чем продолжать так", лучше всей командой оценить трудозатраты в обоих случаях и что в конечном итоге принесет больше прибыли конторе
nleschev
29.06.2018 07:56К дерьмовому коду прилипают лишние трудозтраты в будущем (скорее всего, если это не одноразовый код). Это чистая экономика. Молодые разработчики часто думают, что они молодцы когда быстро реализуют тот или иной функционал и часто не понимают какую бомбу закладывают себе на будущее.
Рефакторинг — это уже терапия, плата за скорость разработки в прошлых периодах. Нужно выбрасывать старый код, нужно модифицировать под текущие реальности если в этом ощущается экономическая целесообразность. Понять это можно только экспертно. Все рефакторить — глупо и дорого.
ACherabaev
Мне кажется или все в итоге все сводится к «работает — не трогай»? Не в каком-то конкретном случае, а абстрактно.
Или все таки стоит делать послабления в сторону «давайте разберем этот окаменелый кал, его уже невозможно ни читать, ни поддерживать»?
Loriowar Автор
В общем-то да. Вы правы. С одной стороны "работает — не трогай". А если трогаешь — осознавай зачем и кому это принесёт пользу.
"Давайте разберёмся" — это конкретная задача с конкретной целью. Ничего против не имею. Но просто так лезть в код, потому что он старый/неподдерживаемый, но работает и последние 3 года его не трогали — это плохая затея.