Что делать, если код, представленный для проверки кода, кажется слишком сложным?

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

Каков наилучший способ действий в этой ситуации? Настаивать на выполнении? Частичный переход? Сначала перефакторинг? Исправить ошибки и принять технический долг ? Проведите оценку риска по этим параметрам, а затем решите? Что-то еще?

114 голосов | спросил Brad Thomas 15 ThuEurope/Moscow2016-12-15T19:23:01+03:00Europe/Moscow12bEurope/MoscowThu, 15 Dec 2016 19:23:01 +0300 2016, 19:23:01

8 ответов


251

Если он не может быть просмотрен, он не может пройти проверку.

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

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

ответил Kevin Fee 15 ThuEurope/Moscow2016-12-15T19:32:08+03:00Europe/Moscow12bEurope/MoscowThu, 15 Dec 2016 19:32:08 +0300 2016, 19:32:08
45

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

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

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

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

ответил Thomas Owens 15 ThuEurope/Moscow2016-12-15T19:38:11+03:00Europe/Moscow12bEurope/MoscowThu, 15 Dec 2016 19:38:11 +0300 2016, 19:38:11
30
  

Проверка правильности правильности вручную с помощью проверки кода, однако, очень сложна и требует много времени, даже если это вообще возможно.

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

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

ответил DepressedDaniel 16 FriEurope/Moscow2016-12-16T03:50:16+03:00Europe/Moscow12bEurope/MoscowFri, 16 Dec 2016 03:50:16 +0300 2016, 03:50:16
15
  

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

Много лет назад на самом деле моя работа заключалась в том, чтобы сделать то же самое, оценив домашнюю работу учащихся. И хотя многие доставляли какое-то разумное качество с ошибкой здесь и там, были два, кто выделялся. Оба всегда отправляли код, у которого не было ошибок. Один представленный код, который я мог читать сверху и снизу на высокой скорости и отмечать как 100% правильно с нулевым усилием. Другой представленный код, который был одним WTF за другим, но каким-то образом удалось избежать ошибок. Абсолютная боль, которую нужно отметить.

Сегодня во втором случае его код будет отклонен в обзоре кода. Если проверка правильности очень сложная и трудоемкая, то это проблема с кодом. Порядочный программист выяснит, как решить проблему (требуется время X), и перед тем, как дать ему рефакторинг кода, он так не выполняет работу, а очевидно выполняет задание. Это занимает намного меньше, чем X, и экономит много времени в будущем. Часто, обнаруживая ошибки, прежде чем они даже идут на этап обзора кода. Затем, сделав обзор кода намного быстрее. И все время в будущем, делая код легче адаптироваться.

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

ответил gnasher729 16 FriEurope/Moscow2016-12-16T13:32:12+03:00Europe/Moscow12bEurope/MoscowFri, 16 Dec 2016 13:32:12 +0300 2016, 13:32:12
6

Является ли этот старый код измененным? (100 строк кода, измененных в кодовой базе 10000 строк, все еще незначительное изменение). Иногда возникают ограничения по времени, и разработчики вынуждены оставаться в старой и неудобной структуре, просто потому, что полная переписывание займет еще больше времени и станет выходом из бюджета , + обычно есть риск, который может стоить миллионы долларов при неправильной оценке. Если это старый код, в большинстве случаев вам придется жить с ним. Если вы не понимаете это самостоятельно, поговорите с ними и слушайте, что они говорят, попытайтесь понять. Помните, что вам может быть трудно следовать за вами, но отлично подходит для других людей. Возьмите их сторону, посмотрите на них с их конца.

Является ли этот новый код ? В зависимости от временных ограничений вы должны защищать рефакторинг как можно больше. Можно ли потратить больше времени на проверку кода, если это необходимо. Вы не должны набирать время на 15 минут, получить идею и двигаться дальше. Если автор потратил неделю на то, чтобы что-то написать, все равно можно потратить 4-8 часов, чтобы просмотреть его. Ваша цель здесь - помочь им реорганизовать. Вы не просто возвращаете код, говорящий «refactor now now». Посмотрите, какие методы можно разбить, попробуйте придумать идеи для введения новых классов и т. Д.

ответил Neolisk 16 FriEurope/Moscow2016-12-16T20:21:10+03:00Europe/Moscow12bEurope/MoscowFri, 16 Dec 2016 20:21:10 +0300 2016, 20:21:10
2

Зачастую «сложные» патчи /списки изменений - это те, которые делают сразу несколько разных вещей. Есть новый код, удаленный код, реорганизованный код, перемещенный код, расширенные тесты; это затрудняет просмотр большой картины.

Общей подсказкой является то, что патч огромен, но его описание крошечное: «Реализовать $ FOO».

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

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

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

Что касается тактично, я обычно говорю это как о своей проблеме, а затем прошу авторскую помощь: «У меня возникли проблемы после всего, что происходит здесь. Не могли бы вы разбить этот патч на более мелкие шаги, чтобы помочь мне понять, как это все вместе? " Иногда необходимо делать конкретные предложения для небольших шагов.

Такой большой патч, как «Реализовать $ FOO», превращается в серию патчей типа:

  1. Ввести новую версию Frobnicate, которая берет пару итераторов, потому что мне нужно будет вызвать ее с помощью последовательностей, отличных от вектора, для реализации $ FOO.
  2. Переключить всех существующих абонентов Frobnicate на использование новой версии.
  3. Удалить старый Frobnicate.
  4. Frobnicate делал слишком много. Фактор шага refrumple в свой собственный метод и добавьте тесты для этого.
  5. Ввести Zerzify с помощью тестов. Пока не используется, но мне нужно это для $ FOO.
  6. Внедрить $ FOO с точки зрения Zerzify и нового Frobnicate.

Обратите внимание, что шаги 1-5 не производят никаких функциональных изменений продукта. Они тривиальны для обзора, включая обеспечение того, чтобы у вас были все правильные тесты. Даже если шаг 6 все еще «сложный», по крайней мере, он фокусируется на $ FOO. И журнал, естественно, дает вам гораздо лучшее представление о том, как был реализован $ FOO (и почему Frobnicate был изменен).

ответил Adrian McCarthy 18 SunEurope/Moscow2016-12-18T21:34:21+03:00Europe/Moscow12bEurope/MoscowSun, 18 Dec 2016 21:34:21 +0300 2016, 21:34:21
1

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

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

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

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

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

ответил c_maker 19 MonEurope/Moscow2016-12-19T13:54:29+03:00Europe/Moscow12bEurope/MoscowMon, 19 Dec 2016 13:54:29 +0300 2016, 13:54:29
0

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

Если это не хорошо протестировано, это хорошее место для начала просмотра.

ответил Tom Squires 15 ThuEurope/Moscow2016-12-15T19:36:21+03:00Europe/Moscow12bEurope/MoscowThu, 15 Dec 2016 19:36:21 +0300 2016, 19:36:21

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132