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

«Синдром сантехника»: правила работы с легаси-кодом в тестировании

Время на прочтение9 мин
Количество просмотров18K
Всего голосов 30: ↑29 и ↓1+28
Комментарии48

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

Ну что тут сказать… Все так и есть, в погоне за «правильным» и «красивым» чрезмерно самонадеянными товарищами очень часто (не всегда конечно) получается известная субстанция, которая сама по себе вдруг оказывается и «некрасивой» и вдобавок «неправильной». И не особо рабочей.
Хотя чего греха таить, 20 лет назад мне тоже много что казалось «неправильным». А сейчас мой код/подход для молодых кажется «неправильным, несовременным, олд-чего-то-там» (хотя если честно, то все равно каждый раз после сдачи проекта, его хочется в куче мест переделать). Но он хотя бы работает, как было задумано, поэтому тут в игру вступает общеизвестное правило «Если работает — ничего не трогай, ничего не меняй».
НЛО прилетело и опубликовало эту надпись здесь
Нет конечно :)
Просто у людей с описанным «синдромом сантехника» он (да и любой другой, кроме их собственного) вызывает желание «все переделать».

Например, я часто использую вариации венгерской нотации при работе (во-первых, я к ней еще с прошлого века привык, а во-вторых удобно, когда код приходится писать сразу на нескольких ЯП в рамках одного проекта. С моей точки зрения конечно), а «продвинутой молодежи» это не нравится, мол это несовременно и не нужно, да и вообще, как так можно «hello world» написать как
echo 'hello world';

а не как

...
class HelloWorld {
...
...
...
}
...
$SayHello = new HelloWorld();
...


Да и вообще, на этом говне никто не пишет, нужно использовать ЯП ХХХ.
И скобки нужно расставлять вот так
class foo {
...
}

А мол ты пишешь «криво и неправильно» вот так:
class Foo 
{
...
}


И все в таком духе :) Для описанного в статье «дартаньяна» неважно кто и как писал код, ему важно показать, что он — «дартаньян», а остальные — криворукие нубы.

Оставлять скобку на отдельной строке объективно лучше, так как позволяет мозгу отпарсить структуру кода без чтения текста. Если скобку не выделить, придется читать, так как есть разные причины для отступа следующей строки.

Ну я так и предпочитаю делать. На мой взгляд, читаемость у такого варианта:
if (a != b)
   {
   for (i = 0; i < 10; i ++)
        {
        switch (e)
             {
             case ...:
                  break;
              ...
             default:
                  ....
             }
        e += i;
        }
   }

лучше, чем у такого:
if (a != b)  {
   for (i = 0; i < 10; i ++) {
        switch (e) {
             case ...:
                  break;
              ...
             default:
                  ....
             }
        e+=i;
        }
   }


Но сколько я уже слышал в свой адрес комментариев, что вариант 1 «неправильно и вообще делать нужно вариант 2, ибо только он есть тру»…

Может, конечно, оно у кого-то и тру, но лично мне второй вариант читать тяжелее.

Имхо, тут вы неправы. Я предпочитаю не противопоставлять себя команде в непринципиальных (не приводящих к значительному ухудшению системы) мелочах.


Отсутствие разногласий в команде важнее, чем личные предпочтения по скобочкам.

Имхо, тут вы неправы.

В чем? В том, что мне нравится/привычно/удобно скобки расставлять вот так? :)
Когда я работаю один, меня все устраивает в моей стилистике. Я свободно читаю свой код спустя и 10 лет. Переделать конечно хочется, ага. Но читается легко.

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

Если команду для проекта набираю я, то там могут быть претензии к совсем непонятному для меня коду, но обычно я не требую сменить стиль, достаточно более подробного комментирования кода, на тот случай, если в нем придется рыться через хх лет. Исключения составляют лишь очевидные малопонятные костыли.
почему бы не делать по массовым стандартам?
Вот я щас пришел в проект, где сперва писали в одном стиле, потом в другом, потом в третьем. И все не по стандартом. Код читать нереально.
где-то перенос скобки, где-то нет, а где-то вобще БЕЗ скобок!
Ну и вот что, теперь для чтения кода надо тратить в 2 раза больше времени минимум. понимание логики и того дольше.
Хотя бы потому, что когда тянешь весь проект самостоятельно, используя 3-4 разных ЯП, которые предполагают разные стандарты, то это местами сильно тормозит процесс.
(например, из недавнего: на Cи делаем демона-прокладку, который опрашивает некоторую аппаратную часть (датчики) и складывает сырые данные, PHP данные сортирует, облагораживает, распределяет по БД, ну а поверх этого работает фронт на JS/HTML/CSS)
Поэтому подвести все к единому виду — на мой взгляд нормальная идея.

Ну а в команде обычно работаешь с 1 ЯП, там какой стандарт задали, так и делаешь, ведь скачек между стилистикой нет.
Ну да, у каждого своя логично выстроенная правда и своё видение мира в целом.
Ну получается а после все хоть огнем гори)

Ведь некоторые программисты делают как им удобно, а не как конечному клиенту.

Конечно, может быть частный случай, когда вы и есть клиент/директор, который ввел правила удобства ведущему программисту-мастеру на все ЯП. Все равно без него все пойдет прахом :)
Если бы все всегда были одинаковые, не было бы технического прогресса :)
Если первый вариант выглядит действительно так, как вы задумывали, то могу отметить что я кажется первый раз в жизни вижу скобки сдвинутые на уровень вложенного кода и это выглядит как минимум странно
Это отголоски древних IDE, которые не умели в нормальное автоформатирование.
Поэтому процесс выглядел так: TAB в настройках = 4 и понеслась.

Претензий к коду, который предлагает автоформатирование современной IDE, типа
if (a != b)
{
    ...
    bla-bla
    ...
}

у меня тоже нет. А вот короткую запись я обычно разворачиваю вручную, или настраиваю на полную, если IDE это позволяет. Привычка.
У вас есть ссылки на исследования показывающие это?

Попытался сейчас найти но безуспешно. Суть была такова:
Если мы спрячем весь текст за неизвестными символами и посмотрим на код, то тот пример о котором мы сможем понять больше -> лучше.


Под спойлерами примеры. Откройте по очереди и попытайтесь понять что там спрятано.


Короткое форматирование
██████ ████ █████████████ █████ █████ ██████ █
    ████████████████████████
        ██████████████████████
        ███████████
        █████████████

    █████████

    ████████████████████████████
        ██████████████████████
    ████████
        ███████████████████████████
    █

    ████████████████████████████████████████████████████████████████

    ███████████████████████████████████████████████████
        ███████████████████████
            ██████████████████████████████████
        █
    █
█

Длинное форматирование
██████ ████ █████████████ █████ █████ ██████
█
    ████████████████████████
        ██████████████████████
        ███████████
        █████████████

    █████████

    ██████████████████████████
    █
        ██████████████████████
    █
    ████
    █
        ███████████████████████████
    █

    ██████████████
        ███████████
        ████████
        ████████████████████████████

    █████████████████████████████████████████████████
    █
        █████████████████████
        █
            ██████████████████████████████████
        █
    █
█

Оригинал
public void AMethod(TypeA arga, TypeB typeb)
{
    var path = Path.Combine(
        arga.someInternalProp,
        typeb.name,
        "a literal");

    Result r;

    if (Path.IsAbsolute(path))
    {
        r = DoSomeStuff(path);
    }
    else
    {
        r = DoSomeOtherStuff(path);
    }

    var response =
        r.Success ?
        r.Data :
        new NullResult(arga, typeb);

    using (var reader = new ResponseReader(response))
    {
        while (readre.Read())
        {
            Console.WriteLine(reader.Current);
        }
    }
}

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

Пишу так, как принято в языке (или как ide по умолчанию форматирует).
Для c#
{
},
для js, java {
}.
Проблем с переключением между двумя вариантами не испытываю, и с «самоваром» из одного языка не лезу в другой. Хорошо, когда за тебя уже решено, как надо делать.
есть PSR, реально код намного сложнее читается, когда инлайн операнды или скобки хрен пойми как понаставлены.
Есть стандарт — извольте следовать. Не хотите? Будут лететь камни.

Стандарты делаются как раз для того, чтоб другие люди быстрее и лучше вникали в код и все было единообразно
Мне казалось, что большинство таких вопросов покрываются стандартом кодирования и автоматическим форматированием. Энергию стоит тратить на что-то стоящее, чем обсуждать скобочки-пробельчики
А я вот и сейчас готов поработать на выходных или вечерами ради того, чтобы привести новый проект, попавший в мои руки, в приличный вид… Сначала документацию везде развешу, форматирование, потом выделю библиотечные методы, потом чуть сложнее переделки, стандартизацию, потом врапперы/хелперы какие-то для унификации, а дальше уже и «выхлоп» идёт — можно брать задачу, на которую раньше легко день уходил, делать за пару часов, а дальше идти пить пиво или ещё больше менять продукт.
Не согласен, если не закрывать тех-долг и не рефакторить старые решения рано или поздно разработка действительно придет к состоянию «продукт в существующих условиях умирает». При разработке нужно закладывать под это бюджет, с первого раза сложно получить стройную систему (особенно когда требования часто меняются). Кривая архитектура, legacy и высокий технический долг — в лучшем случае потери бизнесом денег каждый раз когда требуется внести в эту систему правки (вместо 10 минут-ной доработки — несколько дней на то чтобы разобраться в паутине костылей и встроить ещё один костыль чтобы ничего не рухнуло), а в худшем — смерть проекта.
Надо считать выгоду от модификаций и сравнивать ее с «потерями бизнеса».
… потери бизнесом денег каждый раз когда требуется внести в эту систему правки (вместо 10 минут-ной доработки — несколько дней на то чтобы разобраться в паутине костылей и встроить ещё один костыль чтобы ничего не рухнуло)

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

Статья как раз об этом — перед тем как что-то переписывать, нужно очень-очень хорошо подумать и взвесить выгоду и риски.

Ну если вот эти «30 раз» гарантировано не могут повлиять на работоспособность проекта это одно. А если эта часть кода связана со сторонними сервисами? Тогда пара недель работы программиста «про запас» может оказаться дешевле временной неработоспособности элемента, из за сложности с внесением правок.
P.S. Понятно что это всё тонкости формулировки конкретного вопроса, от чего и зависит ответ.

Oracle как-то справляется.

Такие корпорации-гиганты могут годами и десятилетиями делать всякую стремную фигню, и все равно по инерции и за счет накопленной репутации и базы клиентов будут держаться на плаву.
А для компаний рангом поменьше это уже будет фатально. Тут где-то недавно была статья про проблемы Notepad++ — что мол там ужасная кодовая база, в итоге новые фичи появляются ооочень медленно, что позволило более новым редакторам типа Sublime вырваться вперед.
Согласен с вами полностью. На моём текущем месте имеем то, что несколько высокооплачиваемых специалистов по несколько дней дебажат нигде не задокументированное легаси.
Всё правильно, но автор упустил один положительный аспект переделки. Если 10 раз переделать свою операционную систему, то можно, наконец, понять как она работает и нереально прокачаться в кодинге. :)
Этот аспект положителен с точки зрения сотрудника, а статья все-таки скорее описывает ситуацию с точки зрения бизнеса.
из старенького можно привести BIOS, под которым достаточно много работали с мультимедиа-продуктами, например, сочиняли музыку.

У меня аж челюсть отвисла — как под BIOS можно написать мультимедиа-приложения? Пока не понял, что имеется ввиду BeOS. Правда на сегодняшний день она не так уж и мертва — вышла бета-версия Haiku — преемницы BeOS.
Я от этого абзаца целиком вздрогнул. Сперва BIOS, потом Atom… Нет, я конечно допускаю что был прям какой-то компьютер Atom, после смерти которого разработчикам пришлось перейти на Intel(компьютер? процессор? WTF!?), но совпадения слишком уж совпадения. Особенно эпично выглядит на контрасте с остальным текстом
Двоякие ощущения от этой статьи, ибо от нее складывается ощущение, что legacy-код лучше не трогать. Я категорически не понимаю этого, для меня это из разряда старческого консерватизма, который не редко вредит, чем помогает.

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

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

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

Статья вредная и однобокая. Переделывать старый код не только можно, но и надо, главное делать это осторожно, планомерно и частями. Ну и соответственно чтобы аргументов было чуть по более, чем у человека с синдромом сантехника.
А вы точно читали статью? Раздел «Мудрость проверенная опытным путем» говорит то же самое что и вы просто другими словами. А в остальной части напоминают, что не всегда стоит этим заниматься.
Синдром сантехника наблюдается у любого разработчика. Это нормально. Ненормально — не уметь его сдерживать
Вы чуть-чуть путаете «переписать» и «отрефакторить». Рефакторинг кода — это самое важное для программы, кроме её создания. Рефакторинг — единственный метод сделать настоящий code review на высоком уровне (по уровню абстракций). На момент написания ТЗ было неполным, а полным оно стало в момент окончания написания кода. И уже имея на руках полное ТЗ можно начинать писать нормально. После того, как в коде появится стопятьсот новых фич, придётся рефакторить снова.

Единственное основание для не-рефакторинга — maintanance only режим, в котором новые фичи не появляются. Если появляются — надо.

А вот «всё переписать» — это чаще всего от нежелания читать старый код. Никто не любит читать код. Но надо. Так что не переписывать, а рефакторить.
Есть один знакомый, так при установке игры на d: в Games, установщик предлагал c: и Program Files, так он удалял всё полностью и заново прописывал, да, делал ошибки в пути, но принципиально удалял всё строку. Второй же — редактировал Program Files до Games. В первом случае товарищу не нравится ничей код, всё нитак, а второй же — может воспринимать любой код. Так что, эти всё подходы к чтению и переделыванию «создаются» еще в детстве. И их сложно перебороть.
Не в поддержку или упрёк, но просто факт, к чему может привести такое легаси. Достаточно вспомнить недавний перевод тут на хабре о дремучих джунглях оракловской субд. Тоже много легаси, всё покрыто-перепокрыто тестами, всё работает, но есть куча флагов и поведения с какими-то неявными (до внесения изменений) зависимостями.
Рефакторинг, всё же, требуется по мере развития продукта, иначе может получиться кадавр.
А в таком контексте даже интереснее… Просто я как прикладной разработчик, считаю Oracle — лучшей sql-субд с которой приходилось работать, поэтому читал про внутренности с несколько смешанными чувствами.
О том, насколько громадное количество наслоений и костылей представляет из себя Oracle DB, я понял уже после того, как в первый раз настраивал Toad: больше 10 лет прошло, а я всё ещё помню название файла tnsnames.ora, и какой адок там творится.
Ну объективно, первичная настройка подключения к ораклу — то еще удовольствие. Зато почти одноразовое. Но что за проблемы у вас были с tnsnames? У него насколько помню довольно очевидная структура
С другой стороны я прекрасно понимаю откуда там внутри плотное переплетение костылей. СУБД которая должна быстро работать на богатой куче архитектур и осей, причем коммерческая и при этом живущая дольше меня. Идеальные условия для разведения как по мне :)
Проблем особо не было, только непонимание, нафига вообще всё вот это извращение нужно, чего не могли сделать нормальный публичный сетевой RPC.

Непонятно несколько моментов:


Получится, что нового ничего нет, а старое уже сломали

Как так? Неужели совершенно нет истории изменений и/или всё переписывание ведётся на мастер-ветке? Если так, но не в переписывании проблема, и даже не в увольнении сотрудника.


С точки зрения бизнеса жить нужно сегодняшним днем и обеспечивать непрерывность получения доходов

Действительно ли так много бизнеса, следующего этим постулатам? А как же пресловутый Customer Retention, над которым теперь все так трясутся? Это, само собой, к теме статьи относится только косвенно.

Мудрость, проверенная опытным путем

I. Лучшее — враг хорошего.


Всегда ненавидел эту фразу… Зачем делать посредственно и называть свой посредственный, плохо продуманный и плохо расширяемый код словом «хорошее», если можно сделать по настоящему хорошо и грамотно? Называть желание человек улучшить и оптимизировать код синдромом сантехника — значит просто бояться критики и обсуждений. Если кто-то хочет улучшить код — почему бы не выслушать его аргументы и не привести в ответ свои? Это просто защита серости, которую лучше из суеверия не трогать, лишь бы она не развалилась.
В копилочку ненависти:
Лучше, чем надо — не надо.
и
Работает? — не лезь.
«don't fix what isn't broken»
«не мешай машине ездить»
НЛО прилетело и опубликовало эту надпись здесь
Стоит так-же упомянуть обратную сторону проблемы, когда бизнесс\команда не хочет переписывать legacy, либо делает это «на отстань». Так-же я не могу принять переписывание в виде «делаем новый фронтенд» + «прокси к легаси бекенду». А когда приходит формулировка «эта функция используется в куче мест продукта и если мы её перепишем, то нужно поддерживать обе версии» — хочется всё спалить.
На КДПВ микросервисы собственной персоной
Ну как можно было болт перепутать с гайкой? Болт там был, а за приключения на собственную задницу награждают орденом серебряной отвёртки.
Обычно «взять и переписать», идет вместе с нереальными сроками.
А давайте мы этот legacy код перепишем!
Тут работы на пару месяцев!
Хотя да. Его надо переписывать (рефакторить). Но шаг, за шагом.
Написал тесты на старую логику.
К этим тестам написал новую реализацию.
Долго, нудно и не интересно.
Обычно «сантехники» «сдуваются» на второй-третьей неделе :-)
Потому и следует начинать с написания тестов. Чтобы если/когда задор ушёл — выхлопом были хотя бы тесты, а не наполовину переписанный код.

Причём, с моей точки зрения, тесты в этом всём — как раз самое скучное, поэтому когда у меня снова случится обострение — большая часть тестов уже будет написана, и тут уже отвертеться много сложнее. :)
Зарегистрируйтесь на Хабре, чтобы оставить комментарий