Когда начинается новый проект — его код чист и прекрасен. Самое время проектировать красивые абстракции, писать хорошие интерфейсы и профессиональные реализации. Жизнь прекрасна! Наш проект тоже.
Мы продолжаем добавлять функционал, всё идёт гладко. Потом в некоторый момент нам приходится немного срезать угол, чуть-чуть свернуть с намеченной дорожки, добавить совсем-совсем маленькую обработку одного частного случая… Нет-нет, никаких «костылей»! Просто немного необходимого кода, приближающего красивую теорию к работающей практической реализации. Ничто не предвещает беды и разработка идёт достаточно бодро.
Тем ни менее, на полотне нашей идеальной картины кода всё-же возникают небольшие кляксы. Некоторые люди называют их «техническим долгом». Это не означает, что вы действительно кому-то что-то задолжали: это просто не очень хороший код. Но он как-то работает, решает свои задачи.
По мере продолжения разработки мы будем натыкаться на подобные места в коде всё чаще и каждый раз у нас будет выбор: броситься грудью на амбразуру и переписать всё начисто или обойти проблемное место ради решения другой задачи (той, над которой вы работаете в данный момент). Иногда мы бросаемся в бой, но всё-же чаще мы выбираем вариант с обходным путём.
В каждом отдельном случае это замедляет нас лишь немного. Но, поскольку по мере приближения дедлайна требования к скорости разработки лишь растут, нам приходится постепенно снижать планку своих требований к качеству кода — и это приводит к росту количества «клякс».
Эти новые проблемные места (в дополнение к старым) тормозят нас всё больше и больше. Мы осознаём наличие проблем, но спешка в плане решения текущих задач не позволяет сфокусироваться на чём-либо ещё. Мы вынуждены работать больше, просто ради сохранения прежней скорости.
В какой-то момент вдруг оказывается, что половина кода, который мы пишем, предназначена для подпорок, обходов, хаков и преодоления других типов препятствий, созданных нами же ранее. Тут хороший код, там плохой, а здесь мы рыбу заворачивали.
Каждая прогулка по этому минному полю вместо прямой линии от точки А до точки Б вдруг становится запутанным маршрутом по лабиринту, без всякой гарантии достижения конечной цели. Даже сверх-усилия уже не позволяют вам двигаться к цели с той же скоростью, что и раньше.
Важность проблем теперь видна невооруженным глазом. И их нельзя исправить, просто выбросив всё и начав проект с нуля. Нам нужно выполнить много задач по рефакторингу кода, а на эти задачи нам нужно время, которое придётся просить у заказчика. Часто это время нам не будет выделено: мы ведь просим время для того, чтобы исправить собственный код, на разработку которого уже выделялось время ранее и о готовности которого мы же ранее сами и заявляли.
Даже если время и будет выделено, не стоит рассчитывать на быстрый результат. Мы сможем исправить лишь те проблемы, которые видим конкретно сейчас и лишь в том объёме, на который хватит выделенного времени (а его не будет много). Мы писали этот плохой код много недель (или месяцев), и у нас точно не будет столько же времени чтобы переписать этот код.
Это не тот путь, которым нам нужно идти. Длинные периоды рефакторинга не приносят большой и заметной сразу пользы проекту. Их очень трудно продать заказчику, поскольку он не увидит, за какую функциональность у него просят денег. То есть это плохая идея. Что же делать?
Упрощать! При получении каждой следующей задачи, мы намечаем план её реализации. Если по ходу этого плана мы упираемся в «кляксу» технического долга, то задача по его рефакторингу становится частью реализации текущей фичи. Конечно, мы не можем взяться сразу за все плохие места в коде. Но этого делать и не нужно! Если по ходу реализации какой-нибудь новой функциональности вы исправите больше старых «клякс», чем создадите новых подпорок и костылей — общее качество кода проекта повысится. В следующий раз, когда при работе над новой задаче вам вдруг придётся обратится к уже исправленному месту — вы с удовольствием отметите, что оно больше не требует полировки напильником, а сразу работает хорошо и выглядит приятно. Так и происходит разработка программного обеспечения.
Возможно, разработка каждой отдельной фичи таким образом займёт чуть-чуть больше времени, чем вы предполагали изначально. Но разработка всего набора функционала с таким подходом займёт меньше времени, поскольку на последних этапах перед дедлайном вы будете иметь возможность работать с относительно чистой кодовой базой, добавление нового функционала не будет отнимать время на поиски обходных путей. Кроме того, такой лично ваш подход к разработке хорошо скажется и на производительности всей команды.
Повторение — мать учения. С каждой новой фичей мы делаем код немного чище. Лишь чуть-чуть, но каждый раз. Много раз по «чуть-чуть» в какой-то момент позволят со спокойной душой написать новую, хорошо работающую фичу, за которую не стыдно, вместо того, чтобы рвать на себе волосы и бороться с желанием удалить вообще весь этот мусор.
Момент осознания полезности непрерывного рефакторинга не так далёк, как вам может показаться. Некоторые люди начинали замечать его уже к концу того же спринта, в котором они начали использовать данный подход. Силы непрерывности и инкрементальности процесса дают о себе знать очень быстро. С какого-то момента на выполнение новой задачи с рефакторингом начинает уходить МЕНЬШЕ времени, чем на выполнение этой же задачи без рефакторинга (в силу потраченного ранее времени на улучшения сопутствующего кода).
Работа идёт лучше, код становится чище и мы выдаём заказчику больше функционала, чем могли ранее. Все в выигрыше.
Так что рефакторинг — это не задача в Backlog, это часть каждой задачи в нём.
Комментарии (57)
seokirill
27.11.2016 19:07-5«тем нЕ менее»*
s-kozlov
28.11.2016 13:26Заминусовали, видимо, за то, что на орфографические ошибки принято указывать в личке, иначе в каждом втором посте половина комметариев будет про них. Просто не все честно заслужили четверку/пятерку по русскому и освоили инструменты проверки орфографии.
seokirill
28.11.2016 16:22+3А, я даже не заметил. Ну минусы так минусы.
Буду знать про личку, благодарю.
cranium256
27.11.2016 19:07+10Два смысловых предложения сильно разбавленные водой, с добавлением картинок (для совсем уж дебилов). IMHO не стоило тратить силы на перевод.
netpilgrim
27.11.2016 23:12В реальном мире встречаются задачи с дедлайнами и легаси модули с сильной связностью. Разумно сочетать оба подхода.
Lofer
27.11.2016 23:12+2С точки зреняи управления — любая «работа» должна быть где-то отмечена для учета и понимая результата.
Если мы прячем проблему «под коврик» до добра это не доводит. Если по «правилам» запланированная работа помещается в Backlog — значит она там должна быть.
Вопрос в том, что эта работа будет явно прописана в виде «сделать хорошо» или неявно, в виде «делаем фичу + чуть рефакторинга». Но если это общая работа, вроде рефакторинг API, рефакторинга архитектуры, переделать доменную модель, безопастность и т.д. то делать ее неявно — способ усугубить проблему.
TheShock
28.11.2016 00:37«делаем фичу + чуть рефакторинга
Нет, эта постановка задачи не имеет смысла. Чуть рефакторинга (а если быть точным, то не «чуть» а «столько, сколько необходимо») — это и есть часть «делаем фичу».
Вы же не пишете в задачи: «делаем фичу + читаем ее описание + думаем как ее запрограмить + программируем ее + компилируем + тестируем перед запуском + исправляем баги если она запустилась + рефакторим связанный код»
Все это само собой разумеется и выбрасывать любую часть этого абсурдно, а описывать не имеет никакого смысла. Хотя я знаю программистов, которые не проверяют код который написали)) Таким может и стоило писать в задачу, что код нужно проверить?Lofer
28.11.2016 01:28а описывать не имеет никакого смысла
Наверное, отмечу очевидное, но это уже описано в самом процесе разработке, которому следуют в рамках реализации проекта.
В данном случаечитаем ее описание + думаем как ее запрограмить
играем «в покер» :)
тестируем перед запуском + исправляем баги если она запустилась
— вроде как UnitTests и QA
так что «все ходы записаны» так или иначе :)TheShock
28.11.2016 01:38+1Простите, а вы сами не проверяете задачу на работоспособность перед тем, как отдавать QA? Допустим, задача клиентская и ее можно реально увидеть.
Более того, вы пишете прям в задаче "+ отдать на проверку QA"?
играем «в покер» :)
И что? Пишете в названии задачи «поиграть в планинг-покер»? И вы на планинг покере детально продумываете задачу каждую? Не верю Все-равно на рабочем месте она допродумывается.
Основная идея — есть вещи, которые очевидны и от которых никуда не деться — продумывание, рефакторинг, написание кода, запуск тестов, исправление красных тестов. Они все само собою разумеющиеся когда ставится задача.Lofer
28.11.2016 02:01Простите, а вы сами не проверяете задачу на работоспособность перед тем, как отдавать QA?
Интеграционные тесты вы проводите сами? Вы тратите, допустим, 30 минут на ввод всех тестовых данных? Вполне возможно.
А если у вас тестовый сценарий исполняется минут эдак 120 ..240?
Если у вас пару тысяч бизнес-функций и ваша «поделка» где-то после 30 справочиников и конфигуратора из 50 параметров? И таких у вас 5...10 человек в команде.
Тоже будете тестировать сами? Интересно было бы посмотреть на этот процес.
С точки зрения планирования — у вас 20 задач на имплементацию, которые влияют на, допустим, штук 50...100 бизнес требований. Куда вы поместите эту работу по анализу этого влияния? Как вы распланируете поставку продукта если не понимаете результата вы достигли или нет?TheShock
28.11.2016 02:21+1Какие-то вы крайние случаи рассказываете. Но неважно. Давайте упростим.
Вы ведь не дописываете в каждую задачу о имплементации фичи то, что ее надо программировать? Вот это так же само собой разумеющееся в пределах каждой задачи как и рефакторинг.Lofer
28.11.2016 17:09Ну мы без фанатизма :)
Но задачи типа «Бизнес анализ: документирование процесса XYZ» или «Бизнес анализ: построение доменной модели XZY» или «QA: подготовка тествых данных и сценария» для для фичи ### будет точно, если кто-то решит, что это потребует более 30 минут работы :)
TerraV
27.11.2016 23:12+10А теперь осталось проснуться и пойти в школу. Подход описанный в данной статье хорош, но ни разу не применим к жизни. В результате каждый программист начнет перекраивать код под себя — «я художник, я так вижу». Кляксы не будут уничтожаться а лишь «перекрашиваться», «размножаться» и перемещаться по коду. У задачи по рефакторингу в бэклоге есть как минимум одно безусловное преимущество — сеньор или системный архитектор описывает текущую проблему и способ ее решения, что позволяет сокращать вариативность кода и упрощает приложение в целом. Если рефакторинг делает мидл, даже если его решение красивое, но принципиально новое, оно усложнит разработку и не даст никакого выигрыша.
amarao
28.11.2016 20:39Зависит от связности. Если «миддл» решит проблему в куске кода, хвосты которого не будут никому мешать, то всё хорошо. Даже если решение необычное, но не задевающее остальных.
TerraV
28.11.2016 20:54+2В принципе все зависит от обстоятельств, я и не настаиваю что мои утверждения универсальны. Просто я видел как чувак прикрутил AspectJ потому что ему казалось что это стильно, модно, молодежно. На другом проекте я видел порядка 4 разных реализаций одной и той же функциональности и рефакторинг мог породить пятую. Проблема в том, что такой код на саппорте увеличивает требования к квалификации разработчика, а как следствие время и стоимость сопровождения.
Я сам сторонник чистого кода. На мой взгляд самое дешевое решение это связка «код ревью», «best practices» в каком-нибудь конфлюенсе (проектные или компании), повседневный рефакторинг для улучшения читабельности и глобальный рефакторинг по инициативе/под наздором сеньора / системного архитектора.
Lyosha12
27.11.2016 23:12+1В теории это может выглядеть хорошо. Но мне, как начинающему разработчику, больше интересны примеры, которые я пока что не могу придумать.
Например, когда я впервые сел писать в «ООП-стиле», как мне казалось, я быстро скатился до использования одного божественного объекта для всей задачи. Да, я как-то пытался разбивать её на мелкие части, как-то осмысленно объединять в блоки процедур, но конечный результат меня не порадовал.
В итоге я увидел, что код можно было бы написать короче и проще, используя несколько объектов, а не логических блоков с процедурами.
Конечно, цель статьи, как мне кажется, звучит так: «Пишите хороший код сразу, по возможности». Но об этом написана уже не одна серьёзная книга.
Можно спросить Вас о методах, которые Вы сами применяли для «постепенного» очищения не совсем приятного кода? Может быть, всё дело в шаблонах проектирования? В книге Стефана К. Дьюхэрста так и говорится: «Множество проблем сопровождения возникает из-за игнорирования общепринятых шаблонов проектирования».TheShock
28.11.2016 00:41+3Конечно, цель статьи, как мне кажется, звучит так: «Пишите хороший код сразу, по возможности».
Скорее статья о том, что разрабатывая задачу вы переосмысливаете код и приводите весь связанный код к новым требованиям. Видите, что писали код и потекла абстракция — почините ее. Вылез критичный баг где-то в core, который мешает закончить задачу? Пофиксите этот баг, а не поставьте очередной костыль. И это все является по-умолчанию частью вашей задачи.
С каждой новой задачей код должен расти, а не обрастать.
А вопрос в том, как уже это сделать, как понять, где именно код воняет и как его исправить — к другим статьям.
raacer
28.11.2016 01:01Я в целом согласен, только это работает не во всех случаях.
Можно сделать рефакторинг какой-то отдельной функции или даже нескольких классов, если это улучшает положение дел. Но иногда проблема не очевидна до тех пор, пока не обнаруживается, что какую-то задачу решить просто невозможно. И тогда требуется очень большой рефакторинг солидной части кода, который не укладывается во время, отведенное на задачу. Пример №1: пилили-пилили, и вдруг оказывается, что технология вообще не подходит по производительности. Пример №2: нашли баг, стали исправлять — вылезло два новых, стали исправлять их — вылезло четыре новых, и тогда поняли, что код просто плох.
Особенно сложно — со стартапами, когда не знаешь, куда судьба заведет проект. И вдвойне сложно — с новыми методиками. Думаю, проблему можно решить, имея в команде очень опытного архитектора с богатым опытом принятия ошибочных решений. Но этим архитекторам нужно откуда-то браться, и кто-то должен пострадать. Хотелось бы увидеть статью о том, как предотвращать или решать такие проблемы, не имея огромного опыта за плечами.TheShock
28.11.2016 01:39Пример №1: пилили-пилили, и вдруг оказывается, что технология вообще не подходит по производительности
Простите, но тогда это нельзя называть рефакторингом ;)raacer
28.11.2016 05:08Да, точно )
Ладно, тогда пилили-пилили, и вдруг выяснилось, что в архитектуру не была заложена какая-то базовая вещь, о необходимости которой узнали и поняли только под конец. Например, сделали какой-то элемент системы монолитным, а оказалось, что его надо разделить и испольщовать кусками, только он прошит вдоль и поперек и насквозь и никак не разделяется, и отдельные его функции по отдельности не работают. В таких случаях большую часть работы нужно переделывать полностью, это не то что порефакторить в рамках текущей задачи.
raacer
28.11.2016 05:23Вы выше пишете про потекшие абстракции. Я вот думаю, что для того, чтобы это видеть, надо десятки раз обжечься, чтобы нутром чувствовать, что что-то куда-то потекло… :) Но люлям, достигшим этого погимания, уже не нужно объяснять в детских картинках, что такое рефакторинг и зачем он нужен — они и спрашивать никого не будут, рефакторить им или нет. Или я не прав, и есть какая-то методика достоверно определить наличие потекших абстракций, не имя практического опыта столкновения с их последствиями?
TheShock
28.11.2016 06:44+1На самом деле есть несколько неплохих признаков:
— вам сложно добавлять новый код
— старый код работает не всегда предсказуемо и вы не понимаете почему, баги появляются там, где вроде не должны появлятся и вы все никак их не можете починить
— вы не можете понять, почему именно работает этот код
— размер методов большой, но вы не можете его разделить
Необходимо помнить, что паттерны — рекомендации, возможные пути решения, а не незыблемые законы. Не стоит переписывать прям сейчас код только потому что он не соответствует какому-то признаку (ну имеет, скажем, две ответственности, а не одну), но при этом прекрасно выполняет свою функцию, легко поддерживается и расширяется.
И вот мы смотрим на код и понимаем, что с ним проблема. Может можно упростить флоу? Постараться сделать из древовидных зависимостей — линейные. (очень часто, кстати, необходимо идти методом упрощения, но это как раз приходит с практикой)
Допустим, упрощать некуда, но код все-равно запутан. Применяем SRP — не многовато ли ответственности? Может стоит вынести делегировать какой-то функционал?
Лично мне помогает вот еще что. Я представляю, что пишу фреймворк для большого количества людей. И задаю себе вопросы:
— А что можно вынести в библиотечную функцию, чтобы переиспользовать более абстрактно? (это сократит сложность локального кода)
— А как можно описать интерфейс этого класса, чтобы он был самодокументируемым?
Иногда самый банальный вынос в методы и инвертирование условий может убрать «магичность» кода. В старом топике был пример:
Заголовок спойлера// Magic. Do not touch. var profilesFound = profiles.GetProfiles(x => !( x.ApplicationName != applicationName || !(usernameToMatch == null || x.Username == usernameToMatch) || !(userInactiveSinceDate == null || x.LastActivityDate <= (DateTime)userInactiveSinceDate) || !(authenticationOption == ProfileAuthenticationOption.Anonymous && x.IsAnonymous || authenticationOption == ProfileAuthenticationOption.Authenticated && !x.IsAnonymous) ));
Во-первых, надо убрать линее отрицание и правильно сформатировать:
// Magic. Do not touch. var profilesFound = profiles.GetProfiles(x => ( x.ApplicationName == applicationName && ( usernameToMatch == null || x.Username == usernameToMatch ) && ( userInactiveSinceDate == null || x.LastActivityDate <= (DateTime)userInactiveSinceDate ) && ( (x.IsAnonymous && authenticationOption == ProfileAuthenticationOption.Anonymous) || (!x.IsAnonymous && authenticationOption == ProfileAuthenticationOption.Authenticated) ) ));
Во-вторых-повыносить куски кода в методы. Не знаю, что это за язык, но как-то так:
isCorrectApplication (x) { return x.ApplicationName == applicationName; } isUsernameMatched (x) { return usernameToMatch == null || x.Username == usernameToMatch; } isUserActive (x) { return userInactiveSinceDate == null || x.LastActivityDate <= (DateTime)userInactiveSinceDate; } isAuthenticationAllowed (x) { return isAnonymousAuthentication(x) || isProfileAuthentication(x); } isAnonymousAuthentication (x) { return (x.IsAnonymous && authenticationOption == ProfileAuthenticationOption.Anonymous); } isProfileAuthentication (x) { return (!x.IsAnonymous && authenticationOption == ProfileAuthenticationOption.Authenticated) }
А теперь написать нормальный, легкоподдерживаемый, очевидный с первого взгляда код.
var profilesFound = profiles.GetProfiles(x => ( isCorrectApplication(x) && isUsernameMatched(x) && isUserActive(x) && isAuthenticationAllowed(x) ));
Deosis
28.11.2016 07:38+1Высока вероятность, что этот код передается EF, а значит выносить код в отдельные функции в общем случае нельзя. (код транслируется в SQL, а там нет функции isUserActive)
Опять же подобное стоит упомянуть в комментарии, чтобы программа не упала в этом месте с ошибкой «Моя твоя не понимать» после инициативного рефакторинга.TheShock
29.11.2016 11:03Это, наверное, EntityFramework? Простите, не силен в Шарпах. Как формируется запрос из лямбды? В чем механика? Неужели никаких инструментов для реюза кода? Но допустим, все-равно можно его привести к удобочитаемому виду за счет комментариев (а они хороший альтернативный способ, когда мы ограничены платформой). Уверен, такой код значительно легче читать и поддерживать, чем оригинальный, где написано «Magic. Do not touch»:
var allowedOptions = [ ProfileAuthenticationOption.Anonymous, ProfileAuthenticationOption.Authenticated ]; bool isAnonymousOption = ( authenticationOption == ProfileAuthenticationOption.Anonymous ); var profilesFound = profiles.GetProfiles(x => ( // is correct application x.ApplicationName == applicationName // is username matched && (usernameToMatch == null || x.Username == usernameToMatch) // is user active && (userInactiveSinceDate == null || x.LastActivityDate <= (DateTime) userInactiveSinceDate) // is authentication allowed && (allowedOptions.Has(authenticationOption) && x.IsAnonymous == isAnonymousOption) ));
raacer
28.11.2016 14:09Вы, очевидно, опытный разработчик, умеющий рефакторить свой код по ходу работы. Смотреть на код и понимать, что в нем проблема, можно, если Вы уже сталкивались с последствиями этой проблемы в других проектах. То есть для Вас эта статья — банальность. А как быть с теми, для кого она предназначена?
Вы описали известную методологию, позволяющую писать хороший код (некоторые моменты не однозначны, но не в этом суть). Дело в том, что зная теорию, но не осознавая ее важности и не имея опыта ее применения/неприменения, легко упустить из виду какие-то места, и не задать себе нужные вопросы в нужные моменты по причине обычной лени и стремления сохранить энергию, и просто непонимания, в каких именно местах эти вопросы нужно задавать. Я почему-то сильно сомневаюсь, что какой-то программист, выйдя из ВУЗа, сразу начинает добросовестно использовать все эти методики. А что делать, когда уже наговнокодили горы клякс? Когда эти кляксы оторваны друг от друга, неопытному разработчику трудно понять, что это кляксы, он относительно легко обходит их, и любые незначительные затруднения может списать на свою неопытность. А когда одна клякса строится на базе другой, и выстраивается целая пирамида клякс, похожая на один большой клубок — что тогда делать? Статья бодро рекомендует проложить путь через все эти кляксы, что подразумевает частичный местный рефакторинг отдельных клякс. А он невозможен, когда все переплелось. Частично можно разве что костыли повставлять.TheShock
29.11.2016 10:49+2То есть для Вас эта статья — банальность. А как быть с теми, для кого она предназначена?
Так я и не критиковал эту статью за банальность. Наоборот, считаю, что сформулирована правильная идея, которую, к сожалению, не соблюдают даже опытные разработчики.
Я не совсем вас понимаю, ответ на какой именно вопрос вы хотите получить?
Вы хотите формализацию говнокода и обоснование? Сожалею, но, думаю, программистское сообщество впринципе не доросло до этого, если до сих пор обсуждает такие базовые вопросы, как нужно ООП или нет. То есть вам никто не ответит на это с увереностью и, уже тем более, никто не сделает это так, чтобы какой-либо другой опытный разработчик не раскритковал это в пух и прах. Я часто слышу критику в адрес того же Фаулера, хотя он и не пытался формализовать ГовноКод, а только описывал интересные приемы.
Вы описали известную методологию
Если вам это все известно, тогда в чем вопрос? И какие именно моменты неоднозначны? Чем?
выстраивается целая пирамида клякс, похожая на один большой клубок — что тогда делать?
Или, может, у вас вопрос — как отрефакторить код высокоуровнево, чтобы потом опуститься на уровень ниже?
Еще раз — в чем ваш вопрос и для кого вы хотите получить ответ? Потому что возникает впечатление, что у вас есть в команде джун, который не понимает, что такое говнокод, а вы не можете ему объяснить. Я угадал?
raacer
29.11.2016 15:11Правильно, Вы не критикуете, критикую я :) Я считаю, что молодым программистам она не поможет, а опытным она не нужна, так как они либо уже умеют, либо вообще не хотят следить за качеством, потому что быдлокодеры.
Вопрос:
когда одна клякса строится на базе другой, и выстраивается целая пирамида клякс, похожая на один большой клубок — что тогда делать?
Данная статья описывает как раз такой случай. И я думаю, что местный рефакторинг в таких случаях можно только красиво изобразить на картинках, а в реальной жизни это работать не будет, придется переделывать всю архитектуру, и переводит на нее по крайней мере часть пооекта.
Varkus
28.11.2016 12:00А мне статья понравилась, значит ли это, что я
дебилплохой программист, которому пора проснуться и идти в школу?TerraV
28.11.2016 12:22Нет, не значит. Единственное предположение, которое я могу сделать на основании утверждения что статья понравилась, заключается в том, что вы, скорее всего не работали на крупных проектах / в больших командах. Это никак не характеризует вас как плохого программиста.
Что мне больше всего в этой статье не понравилось, так это то, что она вводит в заблуждение. С одной стороны «Мы вынуждены работать больше, просто ради сохранения прежней скорости.» С другой стороны в каждую задачу предлагают включать рефакторинг. Рефакторинг в подавляющем большинстве случаев замедляет выполнение единичной задачи по сравнению с костылем (именно поэтому костыли и плодятся). Если проводится рефакторинг изолированного участка кода, то в этом нет выгод, нехай себе будет черным ящиком. Если рефакторинг затрагивает как указывали выше API, архитектуру, безопасность, то делать это в одно лицо — значит породить кучу багов на регрессе.
Гораздо проще перед коммитом делать обязательное код-ревью, что помогает следить за единообразием кода и не плодить совсем уж лютый треш. В повседневной работе допустим рефакторинг улучшающий читабельность кода, но не меняющий функциональность (как в примере под спойлером здесь https://habrahabr.ru/company/infopulse/blog/316236/#comment_9934212). Все остальное — это отдельная задача под руководством тех. лида / архитектора.aquamakc
28.11.2016 12:46+1Все остальное — это отдельная задача под руководством тех. лида / архитектора.
Вижу проблему в том, что на сам рефакторинг менеджмент не выделяет ресурсы. Большая часть доморощенного управленчесского состава мыслит следующим образом (к сожалению): решение задачи принесёт прибыть? делаем: не делаем.TerraV
28.11.2016 13:15Если бы менеджеры проектов оперировали деньгами, это было бы просто счастье. С ними можно было бы разговаривать. К сожалению, зачастую менеджеры руководствуются принципом «лишь бы заказчик не поднял бучу». Они ведь не владельцы бизнеса, а такие же наемные работники (как правило на окладе с символической премией). Поэтому привет «а я правки к ТЗ принес» или «заказчик очень просил сделать срочно и я уже пообещал». На мой взгляд нужно качественно делать работу на своем уровне. Если не согласен с тем куда и как движется компания — менять работодателя. Это не отменяет попыток достучаться и объяснить, но бюрократия не должна подменять собой программирование.
Lofer
28.11.2016 17:35+1«заказчик очень просил сделать срочно и я уже пообещал»
Было и такое :)
Два ответа на выбор:
Сам пообещал — сам сделай.
или
А теперь объясни, как мне из кубиков О П Ж А сложить слово «счастье»? Вот тебе ресурсы — планируй. сделаю по твоему плану.
Обычно после такого доходит чего наобещали.
Обратная связь тоже позволяет менджерам себя обучать и меньше фигни творить
raacer
28.11.2016 14:18А Вы хотя бы уточните, чем именно и почему она Вам понравилась. А то как можно судить о Вас, при этом ничего не зная?
Мне статься тоже понравилась, забавная такая в стиле «рефакторинг для самых маленьких». Только решить насущные проблемы она мне не помогла, все равно в очередной раз придется полностью переписывать чей-то говнокод.
michael_vostrikov
28.11.2016 17:01+1Работал на проекте с легаси и без код-ревью. В статье есть дельные мысли. От себя добавлю, что полезно TODO-шки писать — что именно добавить или отрефакторить. Когда мысли есть, а времени на реализацию нету. Можно пару месяцев их только ставить, потом придет какая-нибудь задача, наткнешься на пару штук — о, точно, вот заодно и сделаю. Зачастую и задача потом проще решается.
netpilgrim
28.11.2016 17:44+2Работал на проекте усыпанном TODO-шками, выглядит как летопись лени разработчиков)
s-kozlov
29.11.2016 06:24Вместо «сделаю сразу нормально» — «наговнякаю и поставлю TODO». Ну-ну. Вот после таких деятелей и приходится делать глобальный рефакторинг.
michael_vostrikov
29.11.2016 06:54+1Ключевые слова — «когда мысли есть, а времени на реализацию нету». Если у вас есть возможность вместо работы по текущей задаче делать «сразу нормально» в несвязанном с ней участке кода, где вы вдруг заметили недоработку, я рад за вас.
qw1
29.11.2016 10:09+2Лишнего времени никогда нет. Менеджеры всегда накладывают задач чуть больше, чем можно прожевать и поэтому список текущих (якобы важных и срочных) задач никогда не иссякает. В таких условиях разработчик только сам может поставить приоритет и сказать — я буду делать рефакторинг, независимо от того, выделено на него время или нет.
michael_vostrikov
29.11.2016 10:19Да это понятно, только может быть, что делаете вы рефакторинг, и находите другое место, которое тоже требует рефакторинга. Можно вместо задачи заниматься сплошным рефакторингом, а можно оставить пометку и сделать позже, при выполнении другой задачи. Это не столько лень, сколько правильное распределение ресурсов (времени и сил).
aquamakc
29.11.2016 10:40+2Если костыли не заменять на нормальный код сразу, то со временем софт может превратиться в такую химеру, что её будет проще пристрелить и сделать с нуля, чем рефакторить всю цепочку багов, костылей и пометок «сделать позже».
michael_vostrikov
29.11.2016 11:16Ну так вот, как при реализации задачи встретили костыль в коде, связанном с задачей, так сразу и заменять. О чем и написано в статье. А если в коде, не связанном с задачей, то максимум пометку оставить, потому что иначе так можно и всю систему отрефакторить. Если есть время, то почему бы и нет, но времени обычно не хватает.
aquamakc
29.11.2016 11:25Я считаю, что нельзя к данному вопросу подходить с бинарной логикой типа всегда рефакторить/всегда отклыдывать. Разработчик должен принимать решение рефакторить или не рефакторить исходя из конкретной ситуации. Например, если задача горит, над плечом повис менеджер проекта, которого в режиме онлайн ректально карает заказчик — тут без костыле-ориентированного подхода никак. Если есть возможность подправить костыль или закрыть «долг», пусть и не связанный с текущей задачей — почему бы и нет.
Другое дело, что если софт живёт и развивается рано или поздно ПРИХОДИТСЯ менять минорную (а то и мажорную) версию, т.к. накапливается багаж вещей, которые на применяемой архитектуре/языке/фреймворке сделать невозможно или сложно и опять-таки завязано на велосипедо-строительство. Именно с выделением ресурсов на эти работы связана основная печаль.
aquamakc
29.11.2016 10:46Проблему вижу в том, что далеко не всегда сразу понятно как должно быть, особенно в нашем быстро меняющемся мире. Сегодня заложил одну архитектуру, завтра заказчику захотелось «то-же самое, но с перламутровыми пуговицами». Плюс растёт скилл программиста. Где-то слышал фразу, что основным критерием развития программиста является то, стыдно ему за свой код более года назад или нет (ну как-то так). Открыл исходник годичной давности, ужаснулся, зачесались руки переписать заново с применением новых знаний/технологий/фреймворков.
stoune
29.11.2016 16:31+1Очень наивная и вредная позиция, которая не позволяет иссправить значительные недостатки архитектуры. И для достаточно больших проэктов просто не работает.
При итеративной разработке требования накапливаются и изменяются, поэтому нужен периодический large-scale рефакторинг. Вашей задачей является учить клиента.
В вашем случае когда вы размазывате получается ситуация тоже плохая. По факту вы обанываете клиента. если клиент не желает учится это может быть допустимым вариантом, в других случаях очень вредный совет.
ookami_kb
Собственно, Фаулер писал об этом еще в 2011 году, да и оригиналу уже больше двух лет. Но такой подход целиком и полностью поддерживаю.
stoune
Вы невнимательно читали Фаулера. В приведённой вами статье Фаулер опонирует оппортунистическом рефакторинге, или рефакторинге ради рефакторинга. Его выводы к этой статье неимеют отношения.
ookami_kb
Разве?
stoune
Вы правы невнимательно прочитал. В чём я не согласен с автором и Фаулером, что этот совет вреден и неприменим к large-scale refactoring.
ookami_kb
Ну почему же, одно другому не мешает. Если применять описанный подход, то large-scale refactoring понадобится гораздо позже. Рано или поздно, конечно же, всё равно понадобится – энтропия, как ни крути, не убывает.
Да и вообще, «always leave the code behind in a better state than you found it» – это очень хорошая мысль, к этому, я считаю, разработчик всегда должен стремиться.