Pull to refresh

Comments 57

Собственно, Фаулер писал об этом еще в 2011 году, да и оригиналу уже больше двух лет. Но такой подход целиком и полностью поддерживаю.

Вы невнимательно читали Фаулера. В приведённой вами статье Фаулер опонирует оппортунистическом рефакторинге, или рефакторинге ради рефакторинга. Его выводы к этой статье неимеют отношения.

Разве?


But a team that's using refactoring well should hardly ever need to plan refactoring, instead seeing refactoring as a constant stream of small adjustments that keep the project on the happy curve...
Вы правы невнимательно прочитал. В чём я не согласен с автором и Фаулером, что этот совет вреден и неприменим к large-scale refactoring.

Ну почему же, одно другому не мешает. Если применять описанный подход, то large-scale refactoring понадобится гораздо позже. Рано или поздно, конечно же, всё равно понадобится – энтропия, как ни крути, не убывает.


Да и вообще, «always leave the code behind in a better state than you found it» – это очень хорошая мысль, к этому, я считаю, разработчик всегда должен стремиться.

UFO just landed and posted this here
А, я даже не заметил. Ну минусы так минусы.
Буду знать про личку, благодарю.
Два смысловых предложения сильно разбавленные водой, с добавлением картинок (для совсем уж дебилов). IMHO не стоило тратить силы на перевод.
А как по мне — так говорить об этом время от времени стОит.
И в разных форматах — в т.ч. с веселенькими картинками.

Такого рода знания «ветром не надУет».
В принципе до такого рода знаний можно и самостоятельно догадаться.
Я сразу подумал что эти желтые пометки в виде жуков поверх «клякс» это баги
ну да
«Перекуем баги на фичи» :)
Корпоративный блог — просто добавь воды.
В реальном мире встречаются задачи с дедлайнами и легаси модули с сильной связностью. Разумно сочетать оба подхода.
С точки зреняи управления — любая «работа» должна быть где-то отмечена для учета и понимая результата.
Если мы прячем проблему «под коврик» до добра это не доводит. Если по «правилам» запланированная работа помещается в Backlog — значит она там должна быть.
Вопрос в том, что эта работа будет явно прописана в виде «сделать хорошо» или неявно, в виде «делаем фичу + чуть рефакторинга». Но если это общая работа, вроде рефакторинг API, рефакторинга архитектуры, переделать доменную модель, безопастность и т.д. то делать ее неявно — способ усугубить проблему.
«делаем фичу + чуть рефакторинга

Нет, эта постановка задачи не имеет смысла. Чуть рефакторинга (а если быть точным, то не «чуть» а «столько, сколько необходимо») — это и есть часть «делаем фичу».
Вы же не пишете в задачи: «делаем фичу + читаем ее описание + думаем как ее запрограмить + программируем ее + компилируем + тестируем перед запуском + исправляем баги если она запустилась + рефакторим связанный код»

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

Наверное, отмечу очевидное, но это уже описано в самом процесе разработке, которому следуют в рамках реализации проекта.
В данном случае
читаем ее описание + думаем как ее запрограмить
играем «в покер» :)
тестируем перед запуском + исправляем баги если она запустилась
— вроде как UnitTests и QA
так что «все ходы записаны» так или иначе :)
Простите, а вы сами не проверяете задачу на работоспособность перед тем, как отдавать QA? Допустим, задача клиентская и ее можно реально увидеть.

Более того, вы пишете прям в задаче "+ отдать на проверку QA"?

играем «в покер» :)

И что? Пишете в названии задачи «поиграть в планинг-покер»? И вы на планинг покере детально продумываете задачу каждую? Не верю Все-равно на рабочем месте она допродумывается.

Основная идея — есть вещи, которые очевидны и от которых никуда не деться — продумывание, рефакторинг, написание кода, запуск тестов, исправление красных тестов. Они все само собою разумеющиеся когда ставится задача.
Простите, а вы сами не проверяете задачу на работоспособность перед тем, как отдавать QA?

Интеграционные тесты вы проводите сами? Вы тратите, допустим, 30 минут на ввод всех тестовых данных? Вполне возможно.
А если у вас тестовый сценарий исполняется минут эдак 120 ..240?
Если у вас пару тысяч бизнес-функций и ваша «поделка» где-то после 30 справочиников и конфигуратора из 50 параметров? И таких у вас 5...10 человек в команде.
Тоже будете тестировать сами? Интересно было бы посмотреть на этот процес.
С точки зрения планирования — у вас 20 задач на имплементацию, которые влияют на, допустим, штук 50...100 бизнес требований. Куда вы поместите эту работу по анализу этого влияния? Как вы распланируете поставку продукта если не понимаете результата вы достигли или нет?
Какие-то вы крайние случаи рассказываете. Но неважно. Давайте упростим.

Вы ведь не дописываете в каждую задачу о имплементации фичи то, что ее надо программировать? Вот это так же само собой разумеющееся в пределах каждой задачи как и рефакторинг.
Ну мы без фанатизма :)
Но задачи типа «Бизнес анализ: документирование процесса XYZ» или «Бизнес анализ: построение доменной модели XZY» или «QA: подготовка тествых данных и сценария» для для фичи ### будет точно, если кто-то решит, что это потребует более 30 минут работы :)
А теперь осталось проснуться и пойти в школу. Подход описанный в данной статье хорош, но ни разу не применим к жизни. В результате каждый программист начнет перекраивать код под себя — «я художник, я так вижу». Кляксы не будут уничтожаться а лишь «перекрашиваться», «размножаться» и перемещаться по коду. У задачи по рефакторингу в бэклоге есть как минимум одно безусловное преимущество — сеньор или системный архитектор описывает текущую проблему и способ ее решения, что позволяет сокращать вариативность кода и упрощает приложение в целом. Если рефакторинг делает мидл, даже если его решение красивое, но принципиально новое, оно усложнит разработку и не даст никакого выигрыша.
Зависит от связности. Если «миддл» решит проблему в куске кода, хвосты которого не будут никому мешать, то всё хорошо. Даже если решение необычное, но не задевающее остальных.
В принципе все зависит от обстоятельств, я и не настаиваю что мои утверждения универсальны. Просто я видел как чувак прикрутил AspectJ потому что ему казалось что это стильно, модно, молодежно. На другом проекте я видел порядка 4 разных реализаций одной и той же функциональности и рефакторинг мог породить пятую. Проблема в том, что такой код на саппорте увеличивает требования к квалификации разработчика, а как следствие время и стоимость сопровождения.

Я сам сторонник чистого кода. На мой взгляд самое дешевое решение это связка «код ревью», «best practices» в каком-нибудь конфлюенсе (проектные или компании), повседневный рефакторинг для улучшения читабельности и глобальный рефакторинг по инициативе/под наздором сеньора / системного архитектора.
UFO just landed and posted this here
В теории это может выглядеть хорошо. Но мне, как начинающему разработчику, больше интересны примеры, которые я пока что не могу придумать.

Например, когда я впервые сел писать в «ООП-стиле», как мне казалось, я быстро скатился до использования одного божественного объекта для всей задачи. Да, я как-то пытался разбивать её на мелкие части, как-то осмысленно объединять в блоки процедур, но конечный результат меня не порадовал.

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

Конечно, цель статьи, как мне кажется, звучит так: «Пишите хороший код сразу, по возможности». Но об этом написана уже не одна серьёзная книга.

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

Скорее статья о том, что разрабатывая задачу вы переосмысливаете код и приводите весь связанный код к новым требованиям. Видите, что писали код и потекла абстракция — почините ее. Вылез критичный баг где-то в core, который мешает закончить задачу? Пофиксите этот баг, а не поставьте очередной костыль. И это все является по-умолчанию частью вашей задачи.

С каждой новой задачей код должен расти, а не обрастать.

А вопрос в том, как уже это сделать, как понять, где именно код воняет и как его исправить — к другим статьям.
UFO just landed and posted this here
Пример №1: пилили-пилили, и вдруг оказывается, что технология вообще не подходит по производительности

Простите, но тогда это нельзя называть рефакторингом ;)
UFO just landed and posted this here
UFO just landed and posted this here
На самом деле есть несколько неплохих признаков:
— вам сложно добавлять новый код
— старый код работает не всегда предсказуемо и вы не понимаете почему, баги появляются там, где вроде не должны появлятся и вы все никак их не можете починить
— вы не можете понять, почему именно работает этот код
— размер методов большой, но вы не можете его разделить

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

И вот мы смотрим на код и понимаем, что с ним проблема. Может можно упростить флоу? Постараться сделать из древовидных зависимостей — линейные. (очень часто, кстати, необходимо идти методом упрощения, но это как раз приходит с практикой)

Допустим, упрощать некуда, но код все-равно запутан. Применяем 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) 
));



И очень важно, как меня, понимать, что код делает и почему. Я видел программистов, которые просто вставляют какой-то, стараясь получить какой-то результат даже толком не понимая, для чего именно эта функция.
Высока вероятность, что этот код передается EF, а значит выносить код в отдельные функции в общем случае нельзя. (код транслируется в SQL, а там нет функции isUserActive)
Опять же подобное стоит упомянуть в комментарии, чтобы программа не упала в этом месте с ошибкой «Моя твоя не понимать» после инициативного рефакторинга.
Это, наверное, 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)
));


UFO just landed and posted this here
То есть для Вас эта статья — банальность. А как быть с теми, для кого она предназначена?

Так я и не критиковал эту статью за банальность. Наоборот, считаю, что сформулирована правильная идея, которую, к сожалению, не соблюдают даже опытные разработчики.

Я не совсем вас понимаю, ответ на какой именно вопрос вы хотите получить?

Вы хотите формализацию говнокода и обоснование? Сожалею, но, думаю, программистское сообщество впринципе не доросло до этого, если до сих пор обсуждает такие базовые вопросы, как нужно ООП или нет. То есть вам никто не ответит на это с увереностью и, уже тем более, никто не сделает это так, чтобы какой-либо другой опытный разработчик не раскритковал это в пух и прах. Я часто слышу критику в адрес того же Фаулера, хотя он и не пытался формализовать ГовноКод, а только описывал интересные приемы.

Вы описали известную методологию

Если вам это все известно, тогда в чем вопрос? И какие именно моменты неоднозначны? Чем?

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

Или, может, у вас вопрос — как отрефакторить код высокоуровнево, чтобы потом опуститься на уровень ниже?

Еще раз — в чем ваш вопрос и для кого вы хотите получить ответ? Потому что возникает впечатление, что у вас есть в команде джун, который не понимает, что такое говнокод, а вы не можете ему объяснить. Я угадал?

UFO just landed and posted this here
А мне статья понравилась, значит ли это, что я дебил плохой программист, которому пора проснуться и идти в школу?
Нет, не значит. Единственное предположение, которое я могу сделать на основании утверждения что статья понравилась, заключается в том, что вы, скорее всего не работали на крупных проектах / в больших командах. Это никак не характеризует вас как плохого программиста.

Что мне больше всего в этой статье не понравилось, так это то, что она вводит в заблуждение. С одной стороны «Мы вынуждены работать больше, просто ради сохранения прежней скорости.» С другой стороны в каждую задачу предлагают включать рефакторинг. Рефакторинг в подавляющем большинстве случаев замедляет выполнение единичной задачи по сравнению с костылем (именно поэтому костыли и плодятся). Если проводится рефакторинг изолированного участка кода, то в этом нет выгод, нехай себе будет черным ящиком. Если рефакторинг затрагивает как указывали выше API, архитектуру, безопасность, то делать это в одно лицо — значит породить кучу багов на регрессе.

Гораздо проще перед коммитом делать обязательное код-ревью, что помогает следить за единообразием кода и не плодить совсем уж лютый треш. В повседневной работе допустим рефакторинг улучшающий читабельность кода, но не меняющий функциональность (как в примере под спойлером здесь https://habrahabr.ru/company/infopulse/blog/316236/#comment_9934212). Все остальное — это отдельная задача под руководством тех. лида / архитектора.
Все остальное — это отдельная задача под руководством тех. лида / архитектора.

Вижу проблему в том, что на сам рефакторинг менеджмент не выделяет ресурсы. Большая часть доморощенного управленчесского состава мыслит следующим образом (к сожалению): решение задачи принесёт прибыть? делаем: не делаем.
Если бы менеджеры проектов оперировали деньгами, это было бы просто счастье. С ними можно было бы разговаривать. К сожалению, зачастую менеджеры руководствуются принципом «лишь бы заказчик не поднял бучу». Они ведь не владельцы бизнеса, а такие же наемные работники (как правило на окладе с символической премией). Поэтому привет «а я правки к ТЗ принес» или «заказчик очень просил сделать срочно и я уже пообещал». На мой взгляд нужно качественно делать работу на своем уровне. Если не согласен с тем куда и как движется компания — менять работодателя. Это не отменяет попыток достучаться и объяснить, но бюрократия не должна подменять собой программирование.
«заказчик очень просил сделать срочно и я уже пообещал»

Было и такое :)
Два ответа на выбор:
Сам пообещал — сам сделай.
или
А теперь объясни, как мне из кубиков О П Ж А сложить слово «счастье»? Вот тебе ресурсы — планируй. сделаю по твоему плану.
Обычно после такого доходит чего наобещали.
Обратная связь тоже позволяет менджерам себя обучать и меньше фигни творить
UFO just landed and posted this here
Работал на проекте с легаси и без код-ревью. В статье есть дельные мысли. От себя добавлю, что полезно TODO-шки писать — что именно добавить или отрефакторить. Когда мысли есть, а времени на реализацию нету. Можно пару месяцев их только ставить, потом придет какая-нибудь задача, наткнешься на пару штук — о, точно, вот заодно и сделаю. Зачастую и задача потом проще решается.
Работал на проекте усыпанном TODO-шками, выглядит как летопись лени разработчиков)
UFO just landed and posted this here
Ключевые слова — «когда мысли есть, а времени на реализацию нету». Если у вас есть возможность вместо работы по текущей задаче делать «сразу нормально» в несвязанном с ней участке кода, где вы вдруг заметили недоработку, я рад за вас.
Лишнего времени никогда нет. Менеджеры всегда накладывают задач чуть больше, чем можно прожевать и поэтому список текущих (якобы важных и срочных) задач никогда не иссякает. В таких условиях разработчик только сам может поставить приоритет и сказать — я буду делать рефакторинг, независимо от того, выделено на него время или нет.
Да это понятно, только может быть, что делаете вы рефакторинг, и находите другое место, которое тоже требует рефакторинга. Можно вместо задачи заниматься сплошным рефакторингом, а можно оставить пометку и сделать позже, при выполнении другой задачи. Это не столько лень, сколько правильное распределение ресурсов (времени и сил).
Если костыли не заменять на нормальный код сразу, то со временем софт может превратиться в такую химеру, что её будет проще пристрелить и сделать с нуля, чем рефакторить всю цепочку багов, костылей и пометок «сделать позже».
Ну так вот, как при реализации задачи встретили костыль в коде, связанном с задачей, так сразу и заменять. О чем и написано в статье. А если в коде, не связанном с задачей, то максимум пометку оставить, потому что иначе так можно и всю систему отрефакторить. Если есть время, то почему бы и нет, но времени обычно не хватает.
Я считаю, что нельзя к данному вопросу подходить с бинарной логикой типа всегда рефакторить/всегда отклыдывать. Разработчик должен принимать решение рефакторить или не рефакторить исходя из конкретной ситуации. Например, если задача горит, над плечом повис менеджер проекта, которого в режиме онлайн ректально карает заказчик — тут без костыле-ориентированного подхода никак. Если есть возможность подправить костыль или закрыть «долг», пусть и не связанный с текущей задачей — почему бы и нет.
Другое дело, что если софт живёт и развивается рано или поздно ПРИХОДИТСЯ менять минорную (а то и мажорную) версию, т.к. накапливается багаж вещей, которые на применяемой архитектуре/языке/фреймворке сделать невозможно или сложно и опять-таки завязано на велосипедо-строительство. Именно с выделением ресурсов на эти работы связана основная печаль.
Проблему вижу в том, что далеко не всегда сразу понятно как должно быть, особенно в нашем быстро меняющемся мире. Сегодня заложил одну архитектуру, завтра заказчику захотелось «то-же самое, но с перламутровыми пуговицами». Плюс растёт скилл программиста. Где-то слышал фразу, что основным критерием развития программиста является то, стыдно ему за свой код более года назад или нет (ну как-то так). Открыл исходник годичной давности, ужаснулся, зачесались руки переписать заново с применением новых знаний/технологий/фреймворков.
Очень наивная и вредная позиция, которая не позволяет иссправить значительные недостатки архитектуры. И для достаточно больших проэктов просто не работает.
При итеративной разработке требования накапливаются и изменяются, поэтому нужен периодический large-scale рефакторинг. Вашей задачей является учить клиента.
В вашем случае когда вы размазывате получается ситуация тоже плохая. По факту вы обанываете клиента. если клиент не желает учится это может быть допустимым вариантом, в других случаях очень вредный совет.
Sign up to leave a comment.