Хорошо ли оставлять комментарии об ошибках в коде?

15

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

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

Какая лучшая практика, которой нужно следовать?

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

Сарат
источник
3
Настоящий вопрос в том, ПОЧЕМУ они так делают. Это может быть очень старая процедура до контроля источника.
@anderson - я чувствую, что они не понимают, как использовать clearcase, когда он был представлен. Так что эта практика могла бы последовать дальше ...
сарат
обдумал выяснить?
Я не скажу, что это плохая практика. В любом случае одним из показателей качества программного обеспечения является процент комментариев в исходном коде.
Руди

Ответы:

27

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

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

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

Dysaster
источник
Отличный ответ. Я добавил еще несколько пунктов к своему вопросу. Пожалуйста, проверьте и обновите свой ответ. Спасибо. Кстати, вы могли бы помочь мне с командами clearcase для получения журналов фиксации из определенной ветви?
сарат
@ Сарат, боюсь, я никогда раньше не пользовался ClearCase. Не стесняйтесь использовать суперпользователя, чтобы спросить. Я уверен, что в курсе есть много людей, готовых помочь.
Dysaster
4
Хорошие системы отслеживания ошибок, такие как JIRA, могут просматривать журналы коммитов вашего SCM и извлекать информацию об ошибках. Просто подумайте, что вы что-то делаете для ошибки, и средство отслеживания ошибок автоматически добавляет примечание к сообщению об ошибке, в котором коммит X ссылается на ошибку.
@ Андерсен - Спасибо, мы используем JIRA. Но мне нужно убедиться, правильно ли он используется или нет.
сарат
@ Thorbjørn А, тебе легко. Мне приходилось делать это вручную в тот день с помощью комбо CVS / Bugzilla (а позже SVN / Bugzilla). Добавьте ссылку на исправление в мой коммит и добавьте ссылку на коммит в bugzilla. Процесс подвержен ошибкам, и разработчики, как правило, забывают одно или другое. Но информация оказалась очень полезной в ряде случаев.
Dysaster
23

В основном плохая практика. Я не скажу, что это никогда не должно быть сделано. Иногда вы сталкиваетесь с чем-то вроде ошибки во внешнем API, которую вы должны обойти. Обходной путь может выглядеть совершенно мертвым, если вы не знаете об основной ошибке. В этом случае может быть хорошей идеей документировать ошибку в коде, чтобы коллеги или ваша более поздняя личность не пытались «исправить» явно мертвый код.

Чарльз Э. Грант
источник
3
Согласовано. Добавьте такого рода комментарии, когда они добавляют значительную ценность. Но не делайте их только ради поворота ручки.
quick_now
Хорошо, это поддерживает идею комментариев, говорящих "Почему" некоторый код есть. См. Programmers.stackexchange.com/questions/1/…
Габриэль
Я делал это несколько раз: код, который, на первый взгляд, полностью не поддается логике («для чего этот код * здесь?»), Но на самом деле существует по очень веской причине, поэтому вы хотите, чтобы люди осторожно ходили вокруг Это. Затем добавьте предупреждающий комментарий, объясняющий, почему этот код может облегчить жизнь всем. Затем, если кто-то может придумать лучший способ решения исходной проблемы, то, скажем ему все, ему известно - они знают, почему был создан код braindead, и что он должен сделать, поэтому гораздо проще реализовать альтернативу. решение.
CVN
9

Плохая практика Комментарии устаревают и загромождают код. Информация по-прежнему доступна в истории версий вашей системы SCC, если это необходимо.

Craig
источник
3

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

Fejd
источник
Это ложная дихотомия. Почему вы не можете иметь комментарии к дизайну («именно поэтому мы сделали xy z»), когда это необходимо, и упоминать идентификаторы ошибок в сообщениях коммита?
Мэтью Фляшен
Где это говорит, что ты не можешь? Я имел в виду идентификаторы ошибок в коде, а не сообщения фиксации VCS.
Фейд
Извините, я неправильно понял. Я думал, что вы говорите, что бесполезно размещать идентификаторы ошибок где-либо.
Мэтью Флашен
Нет проблем, это просто означает, что мой ответ не был достаточно ясен. :)
Фейд
2

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

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

refro
источник
Я полагаю, что прокомментированный код должен быть проверен SCM и отказываться даже быть зафиксированным ... Это добавляет беспорядок в код только ради того, чтобы получить 5 минут поиска в истории SCM, если вам понадобится какой-нибудь гипотетический день в будущем. обратно.
Жюльен Ронкалья
Согласились, но постарайтесь реализовать это в SourceSafe: D В нашей проектной команде мы работали с рецензиями, поэтому пока эти блоки отклоняются рецензентом.
refro
О SourceSafe ... извините за вас и вашу команду.
Жюльен Ронкалья
Не будь, это лучше, чем ничего. И на данный момент все новые разработки делаются с Subversion, поэтому с каждым месяцем я все меньше и меньше делаю с SourceSafe.
refro
1

Просто чтобы добавить к тому, что Dyaster et al. Я сказал, что, хотя JIRA обладает действительно хорошими способностями отображать изменения, связанные с исправлениями ошибок, абсолютное лучшее место для документирования ошибок - это тестовый пример. Если код не ясен без комментария, указывающего, какая ошибка была исправлена, это «запах кода». Тем не менее, если у вас нет времени, чтобы очистить запах, комментарий должен относиться к тестовому примеру, где должно быть гораздо более очевидно, почему код делает то, что делает. Если у вас нет времени написать тестовый пример, объясняющий исправление ошибки, значит, ошибка еще не исправлена, она просто отложена.

Бен Хокинг
источник
1

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

Кроме того, вы можете использовать команду blame / annotate / praise вашей системы контроля версий, чтобы помочь заменить эти комментарии. Затем, когда вы запускаете что-то вроде:

vcs blame file.ext

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

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

Я не знаю, что Clear Case имеет для этой функции.

Мэтью Флэшен
источник