Может ли закомментированный код быть ценной документацией?

83

Я написал следующий код:

if (boutique == null) {
    boutique = new Boutique();

    boutique.setSite(site);
    boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
    boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
    boutique.setNom(fluxBoutique.getNom());
    boutique.setSelected(false);
    boutique.setIdWebSC(fluxBoutique.getId());
    boutique.setDateModification(new Date());

    boutiqueDao.persist(boutique);
} else {
    boutique.setSite(site);
    boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
    boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
    boutique.setNom(fluxBoutique.getNom());
    //boutique.setSelected(false);
    boutique.setIdWebSC(fluxBoutique.getId());
    boutique.setDateModification(new Date());

    boutiqueDao.merge(boutique);
}

Здесь есть закомментированная строка. Но я думаю, что это делает код более понятным, делая очевидной разницу между ifи else. Разница еще более заметна с помощью цветовой подсветки.

Может ли комментирование такого кода быть хорошей идеей?

Алексис Дафреной
источник

Ответы:

109

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

Во-первых, закомментированный код не компилируется. Это очевидно, но это означает, что:

  1. Код может даже не работать.

  2. Когда изменяются зависимости комментария, он, очевидно, не сломается.

Комментируемый код - это очень «мертвый код». Чем дольше он там сидит, тем больше он гниет и все меньше и меньше приносит пользу следующему разработчику.

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

Рассмотрим альтернативы:

  • Если цель состоит в том, чтобы привести примеры использования функции / API, тогда предоставьте модульный тест. Модульные тесты - это реальный код, и он сломается, когда он больше не будет корректным.
  • Если целью является сохранение предыдущей версии кода, используйте систему контроля версий. Я бы предпочел проверить предыдущую версию, а затем переключать комментарии по всей базе кода, чтобы «отменить» изменение.
  • Если целью является поддержка альтернативной версии того же кода, используйте систему контроля версий (снова). В конце концов, для этого и нужны ветви.
  • Если цель состоит в том, чтобы уточнить структуру, подумайте, как можно реструктурировать код, чтобы сделать его более очевидным. Большинство других ответов являются хорошими примерами того, как вы можете это сделать.
Крис Питман
источник
5
Я думаю, что вы упускаете одну важную причину: Документация: если цель состоит в том, чтобы задокументировать альтернативные варианты дизайна, вместо исходного кода должно быть предоставлено объяснение альтернативы и особенно причина, по которой она была исключена.
Сариен
14
Варианты дизайна лучше объясняются на человеческом языке, чем на языке программирования.
Марк Э. Хаас
3
Как могли бы последующие разработчики, принявшие мой проект, узнать, что в исходном контроле существует альтернативная / предыдущая / неудачная реализация? Ожидается ли, что новые разработчики пройдут все истории версий и журналы изменений? Или это обычная практика использовать комментарии для ссылки на хэш предыдущего коммита для каждой полезной альтернативной реализации? Если это так, я никогда не заметил.
Муби
Здесь есть одна оговорка. Иногда два эквивалентных кодовых подхода могут отличаться по производительности и надежности так, что один является производительным, а другой - читаемым. В таком случае допустимо использовать вариант-исполнитель, но поместить читаемый вариант в комментарии, чтобы было легче понять назначение кода. Иногда (закомментированная) строка кода может быть более понятной, чем подробное объяснение.
Флатер
263

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

Если вы создадите boutiqueDao.mergeOrPersistметод, вы можете переписать его так:

if (boutique == null) {
    boutique = new Boutique();
    boutique.setSelected(false);
}

boutique.setSite(site);
boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
boutique.setNom(fluxBoutique.getNom());
boutique.setIdWebSC(fluxBoutique.getId());
boutique.setDateModification(new Date());

boutiqueDao.mergeOrPersist(boutique);

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

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


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

bool isNewBoutique = boutique == null;
if (isNewBoutique) {
    boutique = new Boutique();
    boutique.setSelected(false);
}

boutique.setSite(site);
boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE + fluxBoutique.getLogo());
boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE + fluxBoutique.getUrl());
boutique.setNom(fluxBoutique.getNom());
boutique.setIdWebSC(fluxBoutique.getId());
boutique.setDateModification(new Date());

if (isNewBoutique)
    boutiqueDao.persist(boutique);
else
    boutiqueDao.merge(boutique);
CodesInChaos
источник
166

Это абсолютно ужасающая идея. Это не проясняет, что цель. Разработчик закомментировал строку по ошибке? Чтобы что-то проверить? В чем дело?!

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

JustAnotherUserYouMayKnowOrNot
источник
9
Согласовано. Я бы предположил, увидев, что закомментированная строка - это старое поведение, которое было удалено. Если нужен комментарий, он должен быть на естественном языке, а не в коде.
Жюль
4
Я полностью согласен! Недавно я часами подряд пытался понять и очистить некоторые приложения, которые я унаследовал, и которые практически не читаются из-за этой практики. Он также включает в себя код, который был отключен от всего другого кода, но не удален! Я считаю, что это основная цель систем контроля версий. Он имеет комментарии, а также изменения, которые идут с ними. В конце концов, из-за этой практики у меня было по крайней мере 2 недели работы, добавленной к моей тарелке.
Бсара
похожая точка зрения в этом посте: не загрязняйте кодовую базу закомментированным кодом
Ник Алексеев
120

Нет, это ужасная идея. Основываясь на этом фрагменте кода, мне приходят в голову следующие мысли:

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

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

Нет никакой разумной причины проверять закомментированный код в хранилище.

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

Dibbeke
источник
1
Однако я избавляюсь от дублированного кода, я думаю, это вряд ли можно рассматривать как оптимизацию.
Алексис Дафреной,
23
это оптимизация для читабельности человека
JK.
11
@ Traroth вы можете оптимизировать по скорости, использованию памяти, энергопотреблению или любой другой метрике, поэтому я не вижу, что вы не можете оптимизировать по удобочитаемости (хотя как метрика это немного шероховатее)
jk.
3
Действительно, я имел в виду читабельность человеком. Небольшой намек: ваша самая большая ответственность в программировании - ваш код. Таким образом, меньше действительно больше здесь.
Диббеке
4
Программное обеспечение как обязательство также рассматривается по адресу c2.com/cgi/wiki?SoftwareAsLiability. Оттуда: «Создание большего количества кода не всегда выгодно. Код стоит дорого тестировать и поддерживать, поэтому, если такую ​​же работу можно выполнить с меньшим количеством кода, это плюс. Не закомментируйте мертвый код, просто удалите его. Закомментированный код очень быстро устареет и станет бесполезным, так что вы можете также удалить его раньше, чем позже, чтобы избавиться от беспорядка. Сохраняйте хорошие резервные копии, чтобы упростить его. «.
ниндзяль
51

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

function fill(boutique) {    
  boutique.setSite(site);
  boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
  boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
  boutique.setNom(fluxBoutique.getNom());
  boutique.setIdWebSC(fluxBoutique.getId());
  boutique.setDateModification(new Date());
}    

function create() {
  boutique = new Boutique();      
  fill(boutique);
  boutique.setSelected(false);
  return boutiqueDao.persist(boutique);
}

function update(boutique) {
  fill(boutiquie);
  return boutiquieDao.merge(boutique); 
}

function createOrUpdate(boutique) {
  if (boutique == null) {
    return create();
  }
  return update(boutique);  
}
Александр Торстлинг
источник
6
Я думаю, что это самое чистое предложение здесь.
Алексис Дафреной
+1, и я бы также добавил, что чем больше вы избегаете обходить nullобъекты, тем лучше (я считаю, что это решение является хорошим примером).
Надир Сампаоли
Я бы передать в boutiqueDaoкачестве входных данных createи update.
Happy Green Kid Naps
Как это может работать? Как узнать, когда вызывать create, а когда вызывать update? Оригинальный код смотрит на бутик и знает, нужно ли его обновить или создать. Это просто ничего не делает, пока вы не позвоните создать или обновить ...
Лирион
Лирион: Тривиально, я добавлю этот код также для ясности.
Александр Торстлинг
27

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

// The following code is obvious but does not work because of <x>
// <offending code>
<uglier answer that actually does work>

Это предупреждение тому, кто увидит это позже, что очевидного улучшения нет.

Редактировать: я говорю о чем-то маленьком. Если он большой, ты объясни вместо этого.

Лорен Печтель
источник
5
Что не так с // the following part done like it is because of X? Объясните , почему вы сделали что - то, как вы сделали, не то, почему вы сделали не на него в каком - то конкретном пути. В вашем конкретном примере это полностью исключает необходимость в потенциально большом блоке закомментированного кода. (Я не понизил голос, но могу понять, почему за него проголосовали.)
CVn
13
Майкл, потому что это дает понять другие кодеры (и сами дни / недели / месяц спустя) , что да, вы же попробовать , что очиститель / более умный подход, но нет, он не работает из - за X, поэтому они не должны попробуйте еще раз. Я думаю, что это абсолютно законный подход и проголосовал за этот печально похороненный ответ.
Гаррет Олбрайт
1
@GarrettAlbright: Спасибо, я рад видеть, что кто-то получает это.
Лорен Печтел
3
@LorenPechtel: Мало того, я собирался писать более или менее точно так же. Есть ситуации, когда очень и очень полезно быстро узнать, какие «очевидные» решения уже были опробованы безуспешно и почему они не работают.
JensG
3
Помимо неудачного кода с объяснением, я бы также закомментировал альтернативные реализации кода, которые могли бы быть более эффективными в другой производственной среде. Например, я кодировал как прямую экспоненциальную временную версию алгоритма, так и сложную полиномиальную временную версию. Но в текущем производстве nмало, и экспоненциальный алгоритм гораздо быстрее. Если nвпоследствии что- то изменится, как будущий разработчик, следящий за моим проектом, узнает о другой реализации кода, скрытого в сотнях коммитов в управлении исходным кодом?
Moobie
14

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

// Нет необходимости отменять выбор этого бутика, потому что [WHATEVER]

Тем не менее, я думаю, что есть некоторые ситуации, когда выход (или даже добавление закомментированного) кода не предосудителен. При использовании чего-то вроде MATLAB или NumPY часто можно написать эквивалентный код, который либо 1) выполняет итерацию по массиву, обрабатывая один элемент за раз, либо 2) обрабатывает весь массив одновременно. В некоторых случаях последнее намного быстрее, но также намного сложнее для чтения. Если я заменю некоторый код его векторизованным эквивалентом, я вставлю исходный код в соседний комментарий, например так:

%% Векторизованный код ниже делает это:

% for ii in 1:N
%    for jj in 1:N
%      etc.

%, но матричная версия работает примерно в 15 раз быстрее при типичном вводе (MK, 03/10/2013)

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

Мэтт Краузе
источник
«Очевидно, нужно позаботиться о том, чтобы две версии на самом деле делали одно и то же, и чтобы комментарий либо синхронизировался с…» - тут же вы объяснили, почему это не очень хорошая идея.
слеске
1
Ну, это проблема со всеми комментариями, верно? Некоторый векторизованный код достаточно непрозрачен, чтобы комментарии были полезны, а наличие «развернутой» версии может быть полезно для отладки.
Мэтт Краузе
Правда. Тем не менее, я постараюсь сделать комментарий как можно более кратким, а не использовать полный исходный код. В любом случае, если у вас есть пример, вопрос о том, как лучше сделать его читабельным, был бы хорошим вопросом (здесь или на codereview.se).
Слеське
1
В вашем последнем случае я бы оставил оба варианта кода как скомпилированный код.
CodesInChaos
12

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

## Enable support for mouse input:
# enable_mouse = true

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

Карл Смит
источник
7

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

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

Майк Ван
источник
2
Это звучит довольно верно. Многие люди (включая меня) считают, что их код настолько хорош, что ему не нужна документация. Тем не менее, все остальные в мире находят это пустяком, если это не было тщательно задокументировано и прокомментировано.
Райан Амос
« Код только самодокументирует человека, написавшего код ». Пожалуйста, выберите часть сложного, некомментированного кода, который вы написали год назад, и попытайтесь понять его за ограниченное время. Ты не можешь? По электронной почте Ой.
JensG
Я думаю, что это немного более нюанс. Много хорошо написанного кода понятно, и его можно понять без комментариев. Проблема состоит в том, чтобы попытаться выяснить более широкую картину (даже на довольно локальном уровне), когда у вас есть только сложные детали, чтобы продолжить. Комментарии хороши для объяснения неочевидных фрагментов кода, но когда у вас есть хорошие строки документации, объясняющие, для чего фактически предназначены каждая функция, класс и модуль, вам потребуется гораздо меньше помощи, чтобы понять смысл реализации.
Карл Смит
4

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

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

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

Grady Player
источник
2

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

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

Name:           foomatic
Version:        3.14
 ...
Source0:        %{name}-%{version}.tar.gz

Для пакетов без исходных текстов мы закомментируем тег и добавим еще один комментарий над ним, чтобы сохранить стандартный формат и указать, что кто-то остановился и подумал о проблеме как о части процесса разработки:

Name:           barmatic
Version:        2.71
 ...
# This package has no sources.
# Source0:        %{name}-%{version}.tar.gz

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

if ( condition ) {
  foo();
  // Under most other circumstances, we would do a bar() here, but
  // we can't because the quux isn't activated yet.  We might call
  // bletch() later to rectify the situation.
  baz();
}
Blrfl
источник
5
возможно, что этот комментарий не закомментированный код все же.
JK.
1
@jk: Вы, возможно, правы.
Blrfl
1

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

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

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

Ложка
источник
1
Извините, но я не вижу никакого значения кода комментария. Закомментированный код не используется, поэтому ему не место в производственном коде.
Владимир Кочанчич
1
Пожалуйста, определите «используется».
JensG
Я думаю, что он имел в виду «казненный»
Алексис Дафреной
-2

Это не выглядит хорошим приятелем.

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

Вы действительно думаете, что один настоящий программист предпочтет удобочитаемость, а не логическую реализацию? (кстати, у нас есть комментарии и «дополнения» для добавления в наше логическое представление).

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

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

boutique.setSite (сайт) можно заменить на

setsiteof.boutique (сайт). Существуют различные аспекты и перспективы ООП, благодаря которым вы можете повысить читабельность.

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

аура
источник
15
«Насколько мне известно, нужно написать код для компилятора». О, пожалуйста, не надо. Вот так у вас получаются чудовища, которые выглядят так, как будто их можно было взять прямо с конкурса «Запутанный С» и тому подобное. Компьютеры являются бинарными, в то время как люди используют нечеткую логику (кстати, для владельцев домашних животных это удваивается). Компьютерное время сегодня почти бесплатное (в основном только потребление электроэнергии), тогда как время программиста сравнительно очень дорого. Напишите код для людей, и компилятор поймет это. Напишите код для компилятора, не обращая внимания на людей, и вы не будете иметь много друзей в команде.
CVn
3
« написать код для компилятора » - на самом деле вы этого не делаете. Человек, которого вы должны иметь в виду, это тот, кто передал задачу по поддержанию вашего кода.
JensG