Что я должен делать как рецензент, когда код «должен быть» плохим?

Я работаю в плохо спроектированном, сверхнадежном проекте, где обязательные обзоры кода были введены несколько месяцев назад.

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

Как мне вести себя в отношении этого конкретного обзора? Есть ли способ для меня сохранить целостность и оставаться конструктивным?

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

Просто:

1. Документируйте свой технический долг

Вы определили фрагмент кода, который работает, но имеет некоторые технические проблемы. Это технический долг . Как и другие виды долга, со временем оно ухудшается, если с ним не бороться.

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

2. Постепенно погасить свой долг

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

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

В качестве примечания: ищите новую работу. Этот не станет лучше.

Цели кода, который вы просматриваете:

• Для отправки функции, которая должна работать в соответствии с требованиями.

• Сократить рост технического долга.

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

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

• Внешний код не моя вина. Я просто реализую эту функцию, и мне нет дела до всей кодовой базы.

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

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

• Написание нового кода - это возможность реорганизовать старый код.

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

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

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

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

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

Ни вы, ни ваш коллега не в состоянии пройти сотни тысяч строк кода, чтобы использовать MVC, каркас журналирования или ORM повсюду. На самом деле, это потребует месяцев работы: представьте, что вы внедряете MVC; Сколько времени это займет? Или как насчет ORM в ситуациях, когда SQL-запросы хаотически генерировались посредством конкатенации (со случайными местами для SQL-инъекций) внутри кода, который никто не мог понять?

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

• Старый способ обработки запросов,

• Путь MVC,

• Старый механизм регистрации,

• Каркас каротажа,

• Прямой доступ к базе данных с SQL-запросами, созданными на лету,

• ОРМ.

В одном проекте, над которым я работал, было четыре (!) Каркаса ведения журнала, используемых бок о бок (плюс ручное ведение журнала). Причина в том, что каждый раз, когда кто-то хотел что-то записывать, не было единого подхода к этому, поэтому вместо изучения новой инфраструктуры (которая во всех случаях использовалась только в 5% кодовой базы), можно было просто добавить другую, которую он уже знает. Представьте себе беспорядок.

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

• Найдите все места, где ведется традиционное ведение журнала (т. Е. Когда осуществляется прямой доступ к файлу журнала), и убедитесь, что все они вызывают одни и те же методы.

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

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

• Используйте недавно переработанные методы в новой функции.

• Перейдите к структуре ведения журнала: единственный затронутый код - это код в выделенной библиотеке.

Если вы делаете обзоры кода как часть вашего процесса разработки; затем вы должны установить правила, по которым вы «судите» код, который вы просматриваете.

Это должно войти в ваше «определение выполненного» и может быть руководством по стилю, документом по архитектуре для кодовой базы или статическим анализом, проверкой на юридические требования, независимо от того, что компания решит, это ее требования к «качеству кода»

Как только вы это сделаете, проверки кода станут само собой разумеющимся, соблюдаются ли инструкции по стилю, есть ли у нас требуемый процент покрытия тестами и т. Д. И т. Д.

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

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

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

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

В местах, где я проводил проверку кода, я принимал и одобрял отправку кода, если это связано с обязательством (по крайней мере) провести некоторый рефакторинг. Будь то ошибка, история или обещание отправить еще один отзыв с (некоторым) рефакторингом.

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