Pull to refresh

Comments 193

С комментариями есть только одна проблема: их нужно сопровождать. Если у вас написан комментарий, то никто не будет проверять, что информация в нём устарела.
Сломать комментарий очень просто, а вот тесты на актуальность комментариев пока не придумали. В итоге есть хорошее правило: «можешь не писать комментарий, лучше не пиши». Лучше пусть человек потратит время и прочитает код, который есть истина, чем будет полагаться на устаревший коммент.
Если логика мудёрная и экономически целесообразно иметь и сопровождать комментарий/документацию, то, конечно надо писать комментарий.
С комментариями проблема в том, что в универах их заставляют писать, потом их заставляют писать всякие гайдлайны и code conventions'ы. В итоге мы получаем кучу бесполезной, и часто генерённой писанины. Которая вместо экономии времени разработчика нужна для прохождения препода или линтера.
а вот тесты на актуальность комментариев пока не придумали

В rust придумали очень давно)


В итоге мы получаем кучу бесполезной, и часто генерённой писанины.

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


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

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

Если я начинаю читать комментарии, то очевидно, я дошел до точки, когда то месиво из костылей 5-7 летней давности, что называют кодом, не смогло дать мне нужной информации.
К сожалению есть и противоложный подход, когда разработчики в принципе не хотят читать код, а читают только комментарии. А кот — только когда нет никакой возможности понять что проиходит без чтения кода.

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

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

А ещё тут хорошо видно, что "комментарий врёт". Может быть решили расширить формат, но комментарий обновить забыли. Кто-то подумает, что тут ошибка, исправит её и всё сломает.

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


А если есть только ошибочный код, без комментария — то ошибку могут очень не скоро найти.

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

А если он неиспользуется — ну не пофигу ли?

Найдут, конечно. Клиент найдет в виде неожиданного падения, которое он не может повторить (но раз в полгода у десятка клиентов это происходит). Или в виде сдвинутой ячейки в одной строке отчёта, которую придётся поправить вручную.

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

тесты на актуальность комментариев пока не придумали

doctest из питона или хаскеля уже не считается?

Это уже «код, оформленный в виде комментариев», а не комментарии.

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

Многое верно, но есть спорные моменты. По моему опыту:
1. Пишите TODO. У вас всегда в коде будут вещи, на которые у вас сейчас нет времени. И с большой вероятностью вы их забудете. Поэтому сделайте себе за правило: если вы что-то держите в уме, но прямо сейчас вы это сделать не сможете или просто не хотите, просто напишите TODO.
2. Пишите комментарии, отвечающие на вопрос «почему?». Понятное дело, что метод findEmployees() ищет сотрудников, а метод calcEmployeeBonus() считает по ним бонусы. Но вот написать внутри calcEmployeeBonus человеческим языком, почему так работает алгоритм подсчета бонусов — это тот самый полезный комментарий, отвечающий на вопрос «почему?»
Совершенно согласен насчет todo. Но к сожалению, бывают умники, которые считают наличие todo в коде плохим признаком. И пишут правила к сонару. С уровнем critical :)

И правильно делают. Потому что todo в команде уже обычно не работает.


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

Это если вы никогда не делаете рефакторинг. У нас каждый раз рефакторинг начинается с просмотра всех TODO.

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

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


sshikov, обычно TODO это технический долг. Точно такие же задачи, как и баги, кмк.

Один из вариантов: задачи спринта, которые ты лично можешь сделать, заканчиваются и/или объявляется фичефриз, и начинаешь погашать техдолг.

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


Почему сразу не заносить такие задачи в трекер?

Потому что на начальном этапе развития проекта 99% ты будешь сидеть в трекере, т.к. задача заводится довольно долго, требует описания, плюс код много меняется и ты просто не сможешь найти точное место в коде, где нужно что-то сделать. К тому же, TODO бывают чем-то вроде "сделать это покрасивее", причем в момент написания ты понятия не имеешь как, но видишь, что текущий вариант слишком громоздкий. Я знаю, что это дерьмовая постановка задачи, но зато быстрая и удобная, особенно, когда сроки сжаты. Вместо 10 минут на заведение таски, тратить 40 секунд.

Таск можно заводить когда начинаешь работу. Плюс некоторые туду могут снабжаться комментариями типа "НЕ КОПИПАСТИТЬ, write-once код, переписать при повторном использовании, НЕ КОПИПАСТИТЬ" или "отрефакторить если ты читаешь этот код". То есть отдельной задачи не нужно заранее, если предполагается, что этот код "одноразовый".

Проверять, что никто из команды не стал менять вот эту вот 102 строчку кода в файле abc? Пардон, но такое все равно бывает, и выявляется на уровне слияния веток. И если у вас в команде не 100500 человек работает над одним файлом (а если работает — у вас все равно что-то не очень хорошо), то вопрос не работать вдвоем над одним todo обычно не является проблемой никаким образом. Один вопрос в чат: «Коллеги, я тут работаю над задачей такой-то, вижу там todo в коде (автор, дата), думаю поступить так-то, что скажете?» — и проблема решена.

Беда всех трекеров — отсутствие привязки к коду. На объяснение в описании задачи контекста может потребоваться много времени и всё-равно будет не понятно не работавшему с тем участком кода человеку.

TODO
  • Быстро завести
  • Точная привязка к коду

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

Мой выбор:


  • TODO для локальный проблем текущего фичабагабранча/глючных прототипов
  • TODO для неочевидных мест которые зависят об багов (с указанием конкетного бага, и с указанием в баге конкретного места с TODO которое надо потом почистить)
  • ничего — для очевидного инженерного долга (если это будет проблемой, кто-то следующий почистит, когда это станет возможно)
  • для остального — багтрекер
Когда вы работаете один над проектом, я еще могу поверить в полезность TODO комментариев, но как только вас становится довольно много, у вас появляется обычно система учета задач, где эти TODO никак не фигурируют.


Я вполне понимаю само это соображение, только на практике я никогда вообще не видел такой команды, где бы над одной базой кода работало скажем более 10 человек. Готов заранее согласиться, что мой опыт ограничен, но факты таковы. Я эти todo пишу либо для себя, либо для коллег, которых обычно от силы трое, ну может пятеро. А в основном — для себя самого. И так было в случаях когда команда была 5 человек, 10 или 50 — все равно было разделение на какие-то более мелкие проекты, никто группами по 50 никогда над одними файлами не работал.

>обычно TODO это технический долг
Обычно для вас? Ну ок, а почему вы думаете, что у меня так же?

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

>и просматриваете весь код, в поисках TODO,
А зачем просматривать, если тот же сонар или IDEA их список показывает? Так что да, периодически просмотр списка, выбор части идей из него на реализацию. А иногда — просто рефакторинг попутно (в процессе выполнения совершенно других задач).

Ну то есть, вы (как видимо и авторы тех правил для сонара) считаете, что раз в проекте растет число todo — это растет техдолг, и с этим надо бороться. А я смотрю на это так, что растет число идей, как (с точным указанием, где именно) нам улучшить имеющийся код. Отсюда и разные подходы — бороться с идеями очевидно не нужно, у них другой жизненный цикл, и другие инструменты.
Вы всей командой садитесь вместе в условную пятницу и просматриваете весь код, в поисках TODO, которые нужно закрыть

Я начал писать TODO в 1998-м году, когда в очередной версии Delphi IDE появилось окошко, которое собирало и показывало все TODO в проекте, с отметками о выполнении. Сейчас есть IDE, которые так не умеют?
> обычно TODO это технический долг.

// TODO заменить алгоритм на линейный, когда юзеров будет больше 100


Долг? Да, потенциально.

Надо сейчас исправлять? Нет. Может, потребуется завтра, а может, через 5 лет. А может, и никогда.

Полезный комментарий, когда дойдёт до анализа узкого места, или чуть раньше при расширении? Да, безусловно.

Полезно держать этот комментарий именно тут, а не где-то в левой документации? Да, безусловно.
Полезно держать этот комментарий именно тут, а не где-то в левой документации? Да, безусловно.
Я даже не знаю как назвать такую деятельность: «саботаж» или «диверсия». В любом случае обнаружение подобной закладки без соотвествующего бага — повод поговорить о профпригодности и, возможно, об увольнении.

Почему? Потому что вы скрываете критически важную информацию ото всех заинтересованных лиц — и подводите компанию, потенциально, под серьёзные убытки. Потому что совершенно очевивдно, что ваш комментарий будет обнаружен не того, когда вместо Rasperri PI за $35 кому-то придётся купить сервер на двух Xeon'ах, а когда затраты на ваш сервис превысят все разумные масштабы и кому-то будет поставлена задача ваш компонент заменить.

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

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

Не надо так делать: если ваш код имеет неочевидную сложность (нелинейную в большинстве случаев, хотя в случае сотрировок речь идёт об O(N log N), конечно), то об этом должно быть явно и чётко написано в документации. Желательно — с выделением жирным.

Исправлять это «прямо здесь и сейчас» может и не потребоваться — но эта информация должна быть легко доступной. И должна быть и в документации и в багтрекере. Только не забудьте в багтрекере пометить её как feature request, а не как bug, чтобы «борцуны с застарелыми багами» не закрыли её как «устаревший баг, по которому давно не было действий».

Всё зависит от процессов в проекте. Есть есть единая политика работы с todo типа пересмотра их на, например, каждой ретроспективе спринта, то todo лучше непонятных feature request. И todo не малозаметны, их гораздо проще встроить в процессы так, чтобы они "кричали" о себе (например давать пушить коммит только с форсом, если есть todo в затронутом файле). А подробной документации может и не быть вовсе. И кто в здравом уме при появлении проблем с производительностью полезет смотреть FR годовой давности?

Есть есть единая политика работы с todo типа пересмотра их на, например, каждой ретроспективе спринта, то todo лучше непонятных feature request.
Вы это сейчас всерьёз? Как часто у вас проходят «ретроспективы спринта», сколько их будет за 5 лет?

Фразу:
Может, потребуется завтра, а может, через 5 лет. А может, и никогда.
ведь не я написал.

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

И todo не малозаметны, их гораздо проще встроить в процессы так, чтобы они «кричали» о себе (например давать пушить коммит только с форсом, если есть todo в затронутом файле).
Вы можете встраивать что угодно куда угодно, но если у вас TODO живут в коде по 5 лет (а это, ещё раз повторюсь, был основной посыл), то их будет много и большая их часть будет неактуальна.

И кто в здравом уме при появлении проблем с производительностью полезет смотреть FR годовой давности?
А это уж как вы его опишите. Если напишите, что-нибудь типа «Creation of new attachment in tracker would be slow if there would be more than about 100 users. Fix that.» с замечанием «Because FooBar.cc uses O(N³) algorithm we are not ready to support thousands of users, but in our startup there are only 12 employees thus it shoudn't be a problem.», то этот FR автоматически всплывёт по запросу «slow attachments» и нужно будет только перевести его из FR в настойщий bug добавив что-нибудь типа «out startup was bought by BazBar Corp and there are now 100000 employees thus it's now P0 bug».

А подробной документации может и не быть вовсе.
Ну тогда тем более нужно всё аккуратно в багтрекер заносить.
Вы это сейчас всерьёз?

Вполне. Но если в спринте обычно появляется 20 туду, то это не значит, что через 100 спринтов будет 2000 туду. Какие-то теряют актуальность в принципе, какие-то делаются по ходу, какие-то признаются нецелесообразными, а какие-то объединяются в задачи, как правило на рефакторинг и входя в скоуп следующего спринта.


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


то этот FR автоматически всплывёт по запросу «slow attachments»

Не автоматически, а только если кто-то введёт именно этот запрос. Настоящие баги (на неработающую функциональность) люди дублируют постоянно, а тут и не баг вовсе.


Ну тогда тем более нужно всё аккуратно в багтрекер заносить.

Почему в багтреккер? Почему не аккуратно в код?

И вовсе необязательно просматривать все сотни каждый раз, достаточно, обычно, смотреть туду в часто изменяемых файлах.
В таком случае TODO, написанный 5 лет назад вряд ли кто увидит…

Настоящие баги (на неработающую функциональность) люди дублируют постоянно, а тут и не баг вовсе.
Ну как не баг — баг. Просто не «выстреливающий», пока фирма маленькая.

Почему в багтреккер? Почему не аккуратно в код?
Потому что багтрекер — это место, предназначенное для обсуждения проблем с кодом. А код — для этого, всё-таки, не предназначен. Зачем вообще существует багтрекер? Почему вы не заносите все проблемы прямо в сам код?
Речь шла, ещё раз повторюсь, о проблеме, про которую нужно будет не забыть лет через пять.

Извините, но весь этот Аджайл, доведённый до апофигея, для таких вещей не годится.

То что вы показали — это, скорее, плохая замена CodeReview. Плохая, потому что лет через 5 я смогу найти дискуссию в Gerrit'е: она привязана к CL'ям, CL'я остаются у меня в репе, по ним можно выйти на дискуссию.

А вот треды в Stack или MS Teams… их вообще неясно как искать…
В таком случае TODO, написанный 5 лет назад вряд ли кто увидит…

Много техник есть, чтобы увидеть.


Ну как не баг — баг. Просто не «выстреливающий», пока фирма маленькая.

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


Потому что багтрекер — это место, предназначенное для обсуждения проблем с кодом.

Судя по названию всё же не для обсуждения, а для фиксации и отслеживания состояния.


Почему вы не заносите все проблемы прямо в сам код?

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


А в коде фиксируем потенциальные проблемы, технический долг на наблюдаемое поведение не влияющий. При этом сохраняем тесную связь с конкретным местом кода. Вот занесём в баг-треккер "отрефакторить класс WebUser так, чтобы база из него не дергалась", а через 5 лет, когда руки дойдут, обнаружим, что класса такого нет. Закрываем баг как неактуальный? А класс просто переименован, поскольку принято решение, что все интерфейсы буду веб, соотвественно и все юзеры тоже и нет смысла держать WebUser, DesktopUser, CliUser и т. п. И git не понял, что это переименование, удалил один файл и создал другой. А вот тудушечка "Отрефакторить этот класс, чтобы не лез в базу тому, кто следующий будет менять его" никуда не денется.

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

Я не против TODO. Я против TODO, в которых содержится «сакральное знание», более нигде не доступное.

Потому что исчезновение TODO в результате рефакторинга — как раз-таки случается, а баг — можно удалить только админом и только если есть юридическая опасность. То есть с приктической точки зрения — удалить нельзя.

Я как-то упустил, что вы не против туду, но туду должна быть с ссылкой на баг или, скорее, фичереквест. Текст с описанием потенциальной проблемы должен дублироваться?


А вообще я сторонник действий по ситуации. Если туду типа "подумать как уменьшить количество зависимостей и нужно ли", то что в таске так и писать "подумать?"


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

Текст с описанием потенциальной проблемы должен дублироваться?
Если это одна строка — тогда да. Но скорее более подробное описание должно быть в баге.

Если туду типа «подумать как уменьшить количество зависимостей и нужно ли», то что в таске так и писать «подумать?»
Именно так. Вообще как раз очень хороший пример случая, когда бывает полезно на это периодически просматривать. В первой версии можно написать «этот компонент содержит 30 строк и включает в себя 25 .h файлов — возможно стоит объединить его с каким-то другим компонентом». Потом кто-то другой убирает пяток зависимостей и/или смотрит на структуру компонентов. И так далее.

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

Решили сделать временный костыль, но информация о том, что это временный костыль зафиксирована в лучшем случае в комментариях к таске.
Ну дык система контроля версий же существует. Если этот «временный костыль» отлично проработает 10 лет и ничего не развалится — значит это не «временный костыль», а вполне себе «достаточно надёжное решение». А если оно обрушится — ну тут и можно будет поднять вопрос: с какого перепугу кто-то сабмитит костыли не заводя соотвествующих TODO и багов.

Вообще нет ничего более постоянного, чем временное: самый надёжный способ добиться того, чтобы ваш «временный костыль» не остался в коде навечно… не делать «временных костылей» изначально.

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

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


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

Ваш суперагрессивный комментарий основан на ряде допущений, которые истинны только где-то вокруг вас:

1. Что эти TODO/FIXME/etc. никто никогда не видит.
У нас на них смотрят при любом подозрении на проблему, а ещё регулярно их выгрепывают из кода и смотрят, что из них надо рассмотреть в ближайшее время.
То есть их статус в этом смысле полностью аналогичен внутренней документации (как та, что делается doxygenʼом и аналогами).

2. Что тикет в багтрекере со статусом feature request будет виден лучше, чем такое TODO.
Реально же оно ничем не лучше, а часто хуже из-за того, что такие тикеты накапливаются сотнями, искать по ним не легче, а часто тяжелее, и те же «борцуны» будут закрывать их не меньше, чтобы не маячили.

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

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

И вот это

> обнаружение подобной закладки без соотвествующего бага — повод поговорить о профпригодности и, возможно, об увольнении

говорит не в пользу качества внутренних процессов на вашем месте работы.
И слова «саботаж» и «диверсия» тут должны относиться не к такой пометке самой по себе, а к ситуации, когда «верхи» неспособны уточнить у «низов» проработку технического задания, прежде чем принимать решение, или «низы» не в курсе общих процессов в фирме.
Такие эффекты неизбежны в случае, когда фирма становится успешной и в ней появляется хотя бы 1000 работников. Про ситуации, когда на фирме работают 10'000-100'000 человек и говорить не стоит — там, скорее, ситуация, когда все участники в курсе происходящего — редкость.

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

Вот именно. За это время пробежит миллион тикетов в трекере и заодно поменяется выбранный движок трекера (а что старый ещё есть и в нём что-то есть ценное, будет помнить 0.1% старожилов).

А код — если он есть, его можно смотреть и грепать. Хотя, если у вас нормально, что кода давно нет, а есть dll 15-летней давности — «ну извините» ((с) Вовочка).

> там, скорее, ситуация, когда все участники в курсе происходящего — редкость.

А «все» и не нужны. Нужны — существенные для каждой конкретной задачи.

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

Вот именно. За это время пробежит миллион тикетов в трекере и заодно поменяется выбранный движок трекера (а что старый ещё есть и в нём что-то есть ценное, будет помнить 0.1% старожилов).
Вы это из опыта или чисто гипотетически? Я работал в разных фирмах и, по опыту, могу судить, что движок трекера меняется реже, чем что-либо остальное. Главное — он меняется реже, чем меняются команды и, главное, увольняются и нанимаются люди.

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

Насчет команды — да, от размеров конечно же зависит.
поддержу, TODO хорошая практика. Я еще и дату указываю в нем, иногда полезно окинуть взглядом проект отфильтровав по todo и провести профайлинг например
todo имеют право на жизнь, при условии что вы их таки устраняете не позднее чем через условные несколько дней, а в основном код существует без них. Но тут легко перейти грань и вот уже новые todo теряются на фоне сотен старых и очень старых. Нужна какая-то периодическая проверка что ли, на наличие todo в проекте и чтобы прям настойчиво напоминало, что надо от них избавиться.
todo имеют право на жизнь, при условии что вы их таки устраняете не позднее чем через условные несколько дней
А если это изменение, не возможно применить в текущем мажорном релизе из-за обратной совместимости? Удобно оставлять себе пометку об этом: "@todo Fix this in 3.x".

Но тут легко перейти грань и вот уже новые todo теряются на фоне сотен старых и очень старых. Нужна какая-то периодическая проверка что ли, на наличие todo в проекте и чтобы прям настойчиво напоминало, что надо от них избавиться.
В PhpStorm нужен всего один клик, чтобы найти все todo-шки в проекте.
UFO just landed and posted this here

А можете привести пример ответа на вопрос "почему бонусы считаются именно так?"?

//Код не настоящий, но кейс расчета бонусов настоящий
int PlannedTurnover = GetSalesPlan(salesmanId, currentYear);
int ActualTurnover = GetSales(salesmanId, currentYear);
//Для сделок, оплата по которым проходит за наличный расчет, необходимо применять
//корректировку базы начисления бонуса на специальный процент, который компенсирует
//дополнительные расходы на работу с наличными, чтобы уравнять для менеджеров
//доходность со сделок с оплатами обоих типов.
if (ActualTurnover >= PlannedTurnover)
Bonus = GetWiredPayments(salesmanId, currentYear) * BonusPercent + GetCashPayments(salesmanId, currentYear) * BonusPercent * CashDiscount;

Мне кажется что коммент тут компенсирует не отражающее значение название константы CashDiscount.


И судя по всему тут не проценты как написано в комменте а доли (нет деления на 100)

Мне кажется что коммент тут компенсирует не отражающее значение название константы CashDiscount.

А как её нужно назвать, чтобы было понятно, что она введена для того, чтобы уравнять для менеджеров доходность со сделок с оплатами обоих типов? ;)
И судя по всему тут не проценты как написано в комменте а доли (нет деления на 100)

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

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

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

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

Мне кажется что коммент тут компенсирует не отражающее значение название константы CashDiscount.

Коммент относится к формуле же, а не к константе, показывает почему эту константу надо там использовать (уравнять доходность для менеджеров). Она же не в любых операциях с cash применяется.

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

Сейчас вам посоветуют всю эту информацию запихнуть в названия функций и переменных.

Посоветовали бы, если бы в этом комментарии содержалась какая то дополнительная информация. Сейчас же он просто повторяет код. Разве что он содержит подробности о расчёте CashDiscount (чтобы компенсировал). Но при изменении расчёта CashDiscount никто этот комментарий не обновит, так что верить ему не стоит.

В комментарии необязательно должна быть какая-то уникальная информация. На пачке соли написано "Соль", чтобы не нужно было открывать и пробовать, чтобы узнать, что же там такое. Когда комментарий соответствует коду, это подтверждает, что код написан правильно, так как было задумано.

На пачке соли в магазине — это написано. А на солонке в ресторане — нет.

Подумайте почему и к чему ближе ваш метод в Java.
Ну кстати в Java есть Java-doc — я обычно всю вспомогательную информацию выношу в него.

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

IDE вполне может подсвечивать TODO или даже список выдавать.
Если код по какой то причине сейчас написать нельзя — то уж лучше пусть TODO будет, чем потом забыть (да, можно вики отдельную вести только это ЕЩЕ хуже).

У нас в команде принято при написании TODO следующим (первым) словом указывать логин того, кому оно предназначено (даже если самому себе). Несмотря на "недоскрам" у каждого кода есть владелец — тот, кому с кодом проще работать.
Ну и список TODO регулярно просматриваем.

Лучше всё же не логин, а e-mail.

У нас AD и Exchange — это одно и то же.
Иногда, правда, используем инициалы, но редко.

UFO just landed and posted this here
Неясно, за что минусуют. Мне часто приходилось сталкиваться с ситуацией, когда «что за нафиг тут творится» удавалось хоть как-то разобрать только с комменатриями. Такие комменты, как описано у Вас, можно только приветствовать. Как хитро ни называй функции, всегда встречаются сложные бизнес логики, которые надо комментировать обязательно. Ну если, конечно, не вносить весь комментарий в название
getCustomerWithBonusSortedByBonusLevelWithLastYearPurchasesForLastMonth и где-то здесь List
UFO just landed and posted this here
а если в методе один SQL запрос то как правильно его назвать с учётом того, что есть или будут подобные методы для других выборок кастомеров?
а если в методе один SQL запрос то как правильно его назвать с учётом того, что есть или будут подобные методы для других выборок кастомеров?
Если ваш SQL так нечитаем, что аж нужен комментарий — почему вы не покроете его слоем абстракции?
getCustomer()
 .WithBonus()
 .SortedByBonus()
 .LevelWithLastYear()
 .WithPurchasesForLastMonth()
public interface CustomerRepository
{
   IEnumerable<Customer> GetCustomers(ISpecification specification);
}

public class CustomersWithoutBonuses : ISpecification {}
public class CustomersWithBonus : ISpecification {}

`

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

Почему длинное название функции для вас абсурдно?

UFO just landed and posted this here

А если их название не полностью отражает того, что они делают?

UFO just landed and posted this here

Тут есть нескоро моментов. Если получится копия реализации, значит пытались назвать функцию не по тому "что" она делает а по тому "как" она эта делает — если по размышлении не получается выделить "что" то можно посмотреть вариант просто заинлайнить (getCustomers().OrderBy(x => x.BirthDay) для меня читаемее чем getCustomers и не сильно отличается чем getCustomersOrderedByBirthDay. Очень длинное отражающее назначение название часто сигнал что необходим рефакторинг.

UFO just landed and posted this here
Именно потому, что это снижает, а не повышает понятность кода. Проще написать пару строк нормальным языком, с предлогами и артиклями, чем городить монстра, не помещающегося в буфер человеческой памяти. Нужно иметь неплохую подготовку, например, свободно владеть немецким, чтобы понимать сложно составленные слова на 50+символов. Которые будут, к тому же, замусоривать и весь код, где они встречаются. getCustomer, провалившись в которую либо наведя курсор, я увижу всю необходимую мне уточняющую информацию, написанную нормальным языком.
А изменение, приводящее к устареванию коммента без его исправления/удаления, не должно проходить пуллреквест.

Главное, непонятно, зачем эти монстры? Выигрыш-то в чём?
Главное, непонятно, зачем эти монстры? Выигрыш-то в чём?
Выигрыш в том, что человек, увидевший их в stack trace сможет примерно понять что произошло. Комментарии там не показываются и мышку наводить тоже не так просто — особенно если у вас несколько версий «в поле» ушли.
Обычно падает либо что-то глубоко библиотечное вроде SQL либо сетевой компоненты, либо NPE, либо памяти не хватило.
В первом случае названия в моём коде не имеют значения, достаточно понимать, что мы полезли в базу/в сеть.
Во втором и в третьем случае мне сложно представить ситуацию, когда стэктрэйс будет анализировать человек без доступа к исходникам, основываясь только на названиях функций.
Если ваша функция называется getCustomer, она должна возвращать «Customer». Не их коллекцию, отсортированную\отфильтрованную по неким магическим константам, а именно одного покупателя. Если getCustomers — то коллекцию, но опять же — не отсортированную\отфильтрованную. Например, у вас в логике фильтр «старше 30-ти лет», а другой программист будет ломать голову почему у него неполный список покупателей вернулся — вроде обычный метод использовал.

Даже если это функция у объекта FilterGt30?

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

UFO just landed and posted this here

Пишу разные комментарии, но мастхэв считаю следующими:


  • ссылки на человеческие описания (обычно на вики) имплементированных алгоритмов, формул и т. п.
  • ссылки на issues сторонних пакет для различных воркэраундов в своём коде
  • полуформальные описания типов там, где средства ЯП не позволяют их сделать достаточно чёткими, однозначными и читаемыми
Я делаю так же + при копировании кусков кода в разные проекты (всегда же есть какой-то долго живущий шаблон кода) такие комментарии тоже копирую.
Полностью поддерживаю основной посыл статьи — хороший код в комментариях не нуждается.
В то же время метод:
List<Employee> find(Status status)
вполне самодокументован: его сигнатура сообщает, что эта функция возвращает список сотрудников по статусу. А вот название getEmployeeByStatus дублирует эту информацию, да еще и рискует стать таким же лживым, как старые комментарии, когда тип аргумента помеяют, а переименовать забудут.

Пример из раздела «Сложные выражения» пригодился бы и в разделе «Комментарии врут»
// формат по шаблону kk:mm:ss EEE, MMM dd, yyy
Pattern timePattern = Pattern.compile("\\d*:\\d*:\\d* \\w*, \\w*, \\d*, \\d*");
В каждом «блоке» ( kk, mm или yyy например) количество симоволов на самом деле не ограничено ни сверху ни снизу, а после MMM забыта запятая.

Поддержу DrPass — комментарий должен объяснять, почему так странно сделано, если уж пришлось делать странно. А насчет TODO — сомневаюсь: нормально их писать, чтобы не забыть наполнить реализацией заглушку. Но порой сталкиваешься с чьим-то TODO, оставленным в проекте десять лет назад — и как, спрашивается, оценивать его актуальноть)
Все же такие конструкции
List<Employee> find(Status status)
часто живут в соседстве приблизительно с таким
List<Employee> findAll()
Employee findById(long id)
List<Employee> findByFilter(EmployeeFilter filter)
... и так далее

поэтому уточнение имени метода иногда облегчает сильно чтение

Возможно, когда их много стоит минимально-понятно их разделять. Но изначальный код вполне себе отвечает на вопрос "что возвращает метод" (почему-то автору это было не ясно), возвращает ли вообще и какого формата, а также что требуется для работы функции. Собственно минимально-возможное название find отвечает на вопрос, что, собственно, делает метод с параметром, чтоб получить результат.
Касательно вашего примера:


Employee findById(long id) 
=> 
Employee getById(long id)

List<Employee> findAll()
=> 
List<Employee> getAll()

и


List<Employee> findByFilter(EmployeeFilter filter)
=> 
List<Employee> filter(List<Employee> employees, EmployeeFilter filter)

(вообще у коллекции там должен быть встроенный метод фильтр)
Но тут надо понимать, что есть filter, не является ли это condition или просто string который определяет тип/группу employee. Или же это метод, который принимает параметром employee (predicate). Ни один из вариантов при этом, собственно, не является строго фильтром.

Фильтр в моеи примере, это враппер 5..10 параметров поиска, паттерн билдер, маппер которому отдают такой фильтр, умеет его резолвить в sql запрос

Но порой сталкиваешься с чьим-то TODO, оставленным в проекте десять лет назад — и как, спрашивается, оценивать его актуальноть)
Очень просто: формат доджен быть таким:
// TODO(b/12345678): fix that after we would merge branches ABC and XYZ

И вот уже в багтрекере — подробно описывается что у вас там происходит и когда вы сможете этот TODO выполнить.

Это очень хорошее дополнение к рекомендации из статьи: если вам лень заводить баг, описывать на кого он блокирован и прочее — значит вы пытаетесь «отложить на будущее» что-то простое. Сделайте сейчас — и забудьте. Никах TODO.

Если же TODO вот прямо сейчас неисполним… то это значит, что у него — внешняя зависимость. Нужно сделать большой рефакторинг. Или дождаться обновления какой-то библиотеки. Или ещё чего-нибудь в этом духе. Для отслеживания этого всего существуют багтрекеры…
Бывает и так, разумеется (в смысле, что-то простое, на что лень заводить баг).

Почему баг, кстати? Я вот никогда не завожу todo на баги, потому что если вы уже знаете, что тут баг — вы себе враг что-ли, чтобы его откладывать? Теоретически могу представить — но с трудом. Как раз далеко не всегда это баги, и далеко не всегда это простое. Простое в смысле решения, разумеется, потому что в смысле формулировки это уложилось в один комментарий.

>у него — внешняя зависимость
Ну, да. Например, нужно подумать над этим. Прикиньте, бывают такие странные вещи, над которыми нужно поразмыслить какое-то время. И еще важно то, что нам решение сейчас не нужно. А возможно подумав мы придем к выводу, что оно не нужно вообще, что и так хорошо. Это такой способ записывать свои мысли. Дополнительный к баг трекеру.
Как раз далеко не всегда это баги, и далеко не всегда это простое.
Это уже расщепление «волос». Если это не баг, а фича — она тоже идёт в багтрекер (ни видел ни разу чтобы фичи как-то отдельно обрабатывались, везде где встречал это всегда «bug» с пометкой «feature request»).

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

Например, нужно подумать над этим. Прикиньте, бывают такие странные вещи, над которыми нужно поразмыслить какое-то время.
Представляю. У нас в багтрекере половина «багов» — это разные предложения по улучшению. И над ними люди периодически задумываются.

Разница очень проста: если вы напишите что-то в TODO — думать над этим будете вы один. А если в багтрегкер — то, как правило, не только вы.

Это такой способ записывать свои мысли. Дополнительный к баг трекеру.
Вот странно почему-то: как в базу данных ходить — так «дополнительные варианты мимо абстракций» считаются «запахом» (да, иногда писать SQL приходится, но в целом — лучше понять, почему ORM вам не даёт сделать то, что нужно). А как комментарии — так это, вдруг, достоинством становится.

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

Вот тут я с вами не соглашусь. Багтрекер — это для того, что нужно сделать, принять решение и т.д. Это, по сути, «официальные» документы. TODO в коде — это просто пометки, что-то вроде блокнота. Которые в общем случае актуальны только вместе с соседним куском кода. Соответственно, и видеть их нужно лишь конкретно тому, кто туда полез, а не висеть перед глазами у всей команды.
Можете привести пример осмысленного TODO (которые нельзя просто взять и сделать) и который, при этом, был бы «актуален только вместе с соседним куском кода»?
Можете привести пример осмысленного TODO (которые нельзя просто взять и сделать)

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

Это могу, но я на 100% уверен, что почти для всех, в том числе и для вас, это совершенно очевидно.
Вы пишете в TODO то, что вам пришло в голову по мере написания кода, и что вы не хотите или вам некогда просто взять и сделать сейчас.
Что именно вам мешает сделать какую-то локальную фигню вот прямо «сейчас»? Пока вы ждёте пока ревьюер посмотрит на то, что вы понаписали?

Это могу, но я на 100% уверен, что почти для всех, в том числе и для вас, это совершенно очевидно.
Нет, неочевидно. Я видел массу таких TODO — и это всегда выливалось в то, что CL рассматривался три дня вместо трёх часов. За это время все эти «локальные» TODO можно было 10 раз сделать.

А если они ограничены чем-то вне мелкого локального участка кода — ну так тогда и становится актуальным то, о чём я говорю: ссылки на другие баги, обсуждение и прочее.
Что именно вам мешает сделать какую-то локальную фигню вот прямо «сейчас»? Пока вы ждёте пока ревьюер посмотрит на то, что вы понаписали?

Другая фигня, которая для меня имеет более высокий приоритет. Необходимость отдать сырой код как можно быстрее, т.к. сроки презентации новой фичи назначены, и она должна быть реализована в каком угодно сыром виде, но должна быть реализована, а допиливание в спокойном режиме будет после того, как инвесторы удовлетворятся красивой картинкой. Да или просто желание провести сегодня больше времени с семьей. Или что-то ещё. Зачем это объяснять? Вы с этим сами миллион раз сталкивались, и в данном случае заупрямились больше из принципа, чем из-за того, что я неправ :)
Вы с этим сами миллион раз сталкивались, и в данном случае заупрямились больше из принципа, чем из-за того, что я неправ :)
Откуда такая увернность?

Необходимость отдать сырой код как можно быстрее, т.к. сроки презентации новой фичи назначены, и она должна быть реализована в каком угодно сыром виде, но должна быть реализована, а допиливание в спокойном режиме будет после того, как инвесторы удовлетворятся красивой картинкой.
И вот посреди всего этого вы находите время "подумать над этим"? И написать TODO, который через месяц вам что-то прояснит, а не запутает?

Да вы супермен, однако.

И да — я видел код, который писался в спешке и в котором кто-то навставлял кучу TODO. Много раз. И мне всегда было этих людей очень-очень жалко. Потому что… ну они же выкинули кучу времени зря! Я ведь если буду править этот код в спешке же — читать это всё не буду. Не до того. А если я переписываю код «набело» — то я, скорее всего, выкину это всё — не читая, опять-таки. Ибо оно всё равно всё нерелевантно, потому что у кода, написанного в спешке, как правило, проблема не с какими-то мелочами, а с архитектурой. А если я кардинально меняю архитектуру — то зачем мне все эти TODO, которые столько мелочны, что даже баг под них открыть не хватает духу?

Мне кажется, что в большинстве случаев эти TODO — это такая попытка самообмана: да, я пишу корявый код, потому что сроки, потому что до отпуска успеть надо… но он же «почти хороший»! Нужно только чуть-чуть допилить — и всё!

Не работает. Ну вот оооочень редко я видел, чтобы кто-то что-то потом с этими TODO делал. Как правило — они так и висят там пока код не будет сделан «нормально»… что делает большую часть из этих комментариев нерелевантными.
Откуда такая увернность?

Потому что в природе не существует программистов, у которых не бывает более приоритетных задач, срочных дедлайнов, где надо сделать грязно, но быстро, а также каких-то других личных интересов. А если и существуют, то их ничтожные доли процента, и вероятность встретить на Хабре одного из них неотличима от нуля.
И вот посреди всего этого вы находите время «подумать над этим»? И написать TODO, который через месяц вам что-то прояснит, а не запутает?

Простите, посреди чего? Я вроде бы не говорил, что там все в режиме 24х7 с осоловелыми глазами и в памперсах, чтобы не отвлекаться на пи-пи, пишут код в дичайшем кранче. Я имел в виду типовую ситуацию, обычную для подавляющего большинства ИТ-компаний: вот дата демонстрации проекта инвесторам/заказчикам, к ней должны работать такие-то функции, чтобы их показать. Работать в достаточном для демо состоянии.
Это нормальный режим работы, без кранча (ну, вернее, у кого как :-). Реализуется та архитектура, которая и будет дальше. Но где-то будет отсутствовать валидация входных данных. Где-то UI будет сыроват. Где-то полезная настройка будет забита в константу.
И да — я видел код, который писался в спешке и в котором кто-то навставлял кучу TODO. Много раз

Понимаете, то, что вы видели, никакого отношения к TODO не имеет. Вы увидели плохую организацию работы программиста, и с ней TODO. И построили в голове корреляцию, которой на самом деле нет. TODO пишут и плохие, и хорошие программисты. Только вторые их закрывают.
Где-то полезная настройка будет забита в константу.
И вы не хотите сообщить тестировщикам о том, что им не нужно пытаться менять эту настройку — потому что она, пока что, всё равно не работает?

Где-то UI будет сыроват.
И, опять, таки — об этом тестировщики и редакторы документации должны знать, чтобы не тратить время на описание этого UI и/или чтобы не забыть обновить скриншоты, когда его поправят.

Извините, но пока вы ещё не привели ни одного примера между «заменить глобальную переменную „i“ на что-то более понятное» (ну так возьми и замени — чего тут писать-то?) и «у нас тут куча недоделок» (о которых другим участникам команды было бы неплохо знать, чтобы на них «не наступать» "… а для этого есть багтрекер).

TODO пишут и плохие, и хорошие программисты. Только вторые их закрывают.
Пока что это всё ещё остаётся голословным утверждением, которое предполагается принять «на веру». Ну не должен «часто встречающийся случай» TODO, который слишком велик, для того, чтобы его «вот прям щаз» исполнить, но слишком мал, чтобы было осмысленно заводить issue, столько умственных усилий занимать — если он действительно часто встречается.

Максимально близкое, что мне встречалось это что-нибудь типа «а вот этот конструктор должен быть explicit, но если я прямо щаз его таким сделаю — то поломаю вот тот другой CL, который уже третий день на review у другой команды».

На что резонное решение от ревьера: «сделай CL в ветке с тем изменением, я его потом посмотрю, когда они „добро“ дадут».

Я могу, как-то где-то, понять зачем TODO были нужны в эпоху CVS/SVN, когда у проекта была одна «история»… Но сегодня ведь это не так!

И вы не хотите сообщить тестировщикам о том, что им не нужно пытаться менять эту настройку — потому что она, пока что, всё равно не работает?

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

Я выше привел два конкретных примера и один общий, вы их прочли и даже спорить с ними пытались. Так что не извиню :)
На что резонное решение от ревьера: «сделай CL в ветке с тем изменением, я его потом посмотрю, когда они „добро“ дадут».

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

В большинстве проектов, с которыми я сталкиваюсь (будь то GCC, Clang, Chromium или наши внутренние проекты) 100% кода проходят review (хотя бы формальное) — просто потому, что без этого технически невозможно его залить.

И я как-то упустил из виду тот факт, что бывает как-то иначе.

Да, пожалуй если вы порождаете «поток сознания», на который никто не смотрит и, соответственно, не проверяет на понимаемость — тогда TODO с пометками «для себя» могут иметь какой-то смысл… просто странно это — мы же вроде о хорошем коде говорили изначально, а не про «тяп-ляп и в продакшн».

В смысле «сообщить»? А с чего вы решили, что они вообще будут тестировать то, что их не просили?
И это, собственно, оттуда же: что делает в репозитории код, который нельзя тестировать? Опять-таки: во всех проектах, с которыми я сталкивался за последние лет 5-10 сам факт того, что какой-то код попал в репу означает что его можно тестировать (а иначе зачем он там?). Про то, что есть ещё места, где люди могут порождать десятки «локальных» версий программ и устраивать путаницу — я не подумал… но, опять-таки: а какое всё это имеет отношение к качественному коду?

В моих частных проектах я и TODO пишу и багтрекер не использую… потому что это — мои проекты, тут я, как бы, могу что угодно творить… но когда вы начали говорить о том, что вы в TODO на баги не ссылаетесь — то, по крайней мере, стало понятно, что речь не идёт про ваш личный проект, откуда и код-ревью и всё остальное как бы следуют автоматически…
Ну вот, а теперь вы становитесь в позу «я Д'Артаньян, а вы все...»
Нет, вы не правы. Отсутствие Code Review — не признак плохого проекта. Не признак непрофессиональности. Код, который никем не аппрувится, ничем не хуже того, который аппрувится. Это нормальный код, а не поток сознания, как вы сказали. Более того, Code Review — всего лишь одна из практик организации работ, и вариант, когда ваш коллега не тратит время на вычитку вашего кода, а занят созданием других фич, ничем не хуже. Это не тяп-ляп.
И это, собственно, оттуда же: что делает в репозитории код, который нельзя тестировать?

Хочу вам также рассказать, что он делает то же самое, что и код, который можно тестировать. Он там хранится. Это главное назначение репозитория. И вы опять ошибаетесь, коммит кода в репу никак не должен коррелировать с его необходимостью тестирования. Коммит — это всего лишь фиксация изменений, а не передача кода по конвейеру в другие лапы. Передача тестировщикам осуществляется как раз через багтрекер. Если фича реализована, она поступает на тестирование. Если не реализована, не поступает.
речь не идёт про ваш личный проект, откуда и код-ревью и всё остальное как бы следуют автоматически…

И, повторюсь про Д`Артаньяна, нет, ничего этого не следует. Например, вы покупаете билеты на самолёт через систему, при написании которой не было никакого Code Review (я это точно знаю :-), и при этом мир не рухнул, вы остались живы. Это же касается и биржевых трейдеров, и интернет-магазинов, и много чего другого. Подавляющее большинство профессиональных команд прекрасно обходятся без этой практики.
«у нас тут куча недоделок» (о которых другим участникам команды было бы неплохо знать, чтобы на них «не наступать» "… а для этого есть багтрекер)

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

А вы прочитайте с чего тред начался. Если в коде недоделка, то вы увидите там TODO, пойдёте и прочитаете «историю вопроса»

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

Можете привести пример осмысленного TODO (которые нельзя просто взять и сделать) и который, при этом, был бы «актуален только вместе с соседним куском кода»?

Поискал в проекте Chromium, там много вполне обоснованных TODO без задач.


https://cs.chromium.org/chromium/src/base/bind.h?l=270
TODO(tzik): Deprecate this and migrate to OnceCallback and RepeatingCallback, once they get ready.

https://cs.chromium.org/chromium/src/ash/shell.cc?l=1045
TODO(oshima): Move as many controllers before creating RootWindowController as possible.

https://cs.chromium.org/chromium/src/url/origin.h?l=97
TODO(dcheng): This behavior is not consistent with Blink's notion of file URLs, which always creates an opaque origin.

https://cs.chromium.org/chromium/src/dbus/bus.h?l=236
TODO(satorux): Remove the service name parameter as the caller of RequestOwnership() knows the service name.

https://cs.chromium.org/chromium/src/url/origin.h?l=284
TODO(nick): Should this optimization move into UnguessableToken, once it no longer treats the Null case specially?

https://cs.chromium.org/chromium/src/crypto/hmac.h?l=40
TODO(abarth): Add a PreferredKeyLength() member function.
Если вы внимательно присмотритесь — то увидите там заменитель: имя человека, написавшего TODO. У него можно спросить. Но время показало, что это хуже, чем ссылка на баг: человек может уволиться или забыть. Баг в багтрекере надёжнее.

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

Имя автора туду можно смотреть по логам VCS

К сожалению это далеко не такая простая задача, как кажется. Потому что рефакторинги склонны приводить к тому, что «git blame» на TODO будет выдавать совсем не того человека, который его завёл. Да, откопать, потратив на это пару часов сможете вытащить из истории нужную информацию — но это лишает TODO всякого смысла: за такое время вы, скорее всего, и без всяких описаний сможете понять что тут происходит. Так что имя человека, который TODO написал — это минимум… но баг — лучше. Потому что человек может уволиться, а баг — нет.
В IDEA правой кнопкой по строчке, «показать историю коммитов» и можно увидеть всю историю по ней. Занимает секунд 30 максимум. Пользуйтесь нормальными инструментами, не придется тратить пару часов на элементарные операции
В IDEA правой кнопкой по строчке, «показать историю коммитов» и можно увидеть всю историю по ней.
Вот прям всю? Дойдёт то точки где стиль комментарием сменили и вместно // начали их со /* начинать? Заметит, что кто-то добавил пропущенный артикль? А потом пойдёт из Git в SVN? И в P4 заглянет, который до SVN был? А к вендорам, которые этот код изначально у себя разрабатывали — как ваша магическая IDEA вообще заходит — не расскажите?

Занимает секунд 30 максимум.
Если у вас на проекте это занимает «секунд 30 максимум», то это не значит, что у других — всё то же самое. Возьмите хотя бы GCC и посмотрите на пресловутый reload.c — а это далеко не самый страшный вариант, уверяю вас.

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

Так-то, чтобы разобраться в своём личном проекте, конечно, пара часов не требуются — ну так там и без багтрекера можно обойтись, а кто-то и без VCS обходится…
Дойдёт то точки где стиль комментарием сменили и вместно // начали их со /* начинать?

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

Заметит, что кто-то добавил пропущенный артикль?
Аналогично, почему нет?

А потом пойдёт из Git в SVN?
Да, если сохранили историю, когда мигрировали.

И в P4 заглянет, который до SVN был?
Понятия не имею

А к вендорам, которые этот код изначально у себя разрабатывали — как ваша магическая IDEA вообще заходит — не расскажите?
А как вам поможет имя возле TODO в таком случае? Вы ведь контекст разговора не забыли?

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

И пока у вас никто не «причёсывает» чужой код, не правит в комментариях стилистический и орфографические ошибки.
И причесывают и правят.

контрибуторов не начинается меряться тысячами
Ооо. Ну тогда все понятно, очевидно имя в формате TODO(Вася) или TODO(nick) поможет значительно сильнее, чем история коммитов VCS, что я смыслю вообще?

Но с ссылкой на багтрекер я согласен, это хорошая идея. Я лишь не понимаю, зачем писать имя, если оно и так может быть легко получено через историю контроля версий.
Да, гит ведь помечает строку как измененную, а не как удаленную и новую.
Я вам открою страшную тайну: Git — не помечает ничего. Совсем ничего. Он это всё вычисляет в момент, когда вы делаете «Git blame». И чего он там навычисляет — зависит, условно говоря, от фазы луны.

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

Вы ведь контекст разговора не забыли?
Нет, не забыл.

А как вам поможет имя возле TODO в таком случае?
Там, вообще-то не просто имя, а account name. На chromium.org, android.org, llvm.org и так далее (хотя вот это вот уже приходится угадывать иногда). И да — люди вполне могут сохранять подобные адреса после увольнения. И имеют их работая в самых разных компаниях.

Не понимаю, что должно мешать посмотреть историю по строке в большом файле.
Тут вопрос не в размере файла, в размере истории. Git вам очень часто будет давать ссылку вовсе не на тот commit, где что-то сделали изначально, а на тот, где что-то кардинально переформатировали, перенсли в другое место и так далее.

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

Но вот только это ни разу не «кликнуть по строчке и увидеть изначальный commit».

Я лишь не понимаю, зачем писать имя, если оно и так может быть легко получено через историю контроля версий.
Ну Ok, если для вас это легко, то вот вам домашнее задание: скажите кто и когда назначил аттрибут aapcs-vfp функциям вот в этой вот репе. Конкретный небольшой файл (7289299240a3a3c7f05c6c92c12426a99ec2c894), конкретная строка (61я), никаких секретных репозиториев, к которым у вас нет доступа…

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

git subtree вроде бы для этого.

git subtree — это совсем о другом. Это если вы хотите иметь проект в нескольких репозиториях. А если вам это надоело и вы решили слить всё из нескольких репозиториев в один? А потом разделить всё обратно?

Вы, наверное, путаете subtree и submodules.

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

Ну вот как udev, например. Что там было до того, как его разработка переехала в репозиторий systemd? Ну что-то было… но в другом репозитории и «в другом мире».

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

Но в простых-то случаях и человек где-то рядом и контекст ещё «не уплыл».
При этом, что довольно очевидно, больше всего вопросов возникает как раз с TODO, которые написаны давно

Ну а каков может быть выход? Багтрекер? Да не поможет, т.к. разобрать авгиевы конюшни багтрекера, которые наполнялись лет пять-десять — задача ничуть не легче. Наоборот, в случае TODO по крайней мере сразу видно, в каком месте кода может потребоваться доработка. И если уже не требуется, его нужно просто снести. Ещё в случае TODO обычно сразу понятно, актуально ли оно для текущей версии кода, т.к. TODO пишутся именно там, где требовалась какая-либо доработка, и если этот код дошел в текущую версию, то и комментарии к нему скорее всего актуальны. А в багтрекере зачастую придется целое расследование проводить, что там за issue, сохранилось ли в текущей версии и т.д… И фишка в том, что для давно и хорошо замаринованных тикетов его уж точно никто проводить не будет, а просто оставит висеть и дальше.
Так что серебряной пули нет, хотите вы или нет, но в сложных и долгоиграющих проектах всегда будут накапливаться недоделки, независимо от организации работ и ваших методик.
Да не поможет, т.к. разобрать авгиевы конюшни багтрекера, которые наполнялись лет пять-десять — задача ничуть не легче.
А зачем вам его разбирать? Вам в нём нужно только информацию найти.

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

А в багтрекере зачастую придется целое расследование проводить, что там за issue, сохранилось ли в текущей версии и т.д…
А как иначе? Как бы совет из обсуждаемой статьи: если вам «сразу всё понятно», то не нужно разводить TODO — просто сделайте то, что вы там собрались написать и всё — он по-прежнему актуален.

И фишка в том, что для давно и хорошо замаринованных тикетов его уж точно никто проводить не будет, а просто оставит висеть и дальше.
И ваш ревьюрер, увидив что вы удаляете TODO на незакрытый баг — вам ровным счётом ничего не скажет???

Так что серебряной пули нет, хотите вы или нет, но в сложных и долгоиграющих проектах всегда будут накапливаться недоделки, независимо от организации работ и ваших методик.
Конечно. Но вот методика «написали TODO и забили» (а ровно этим кончается подход, когда автора TODO нельзя найти и спросить у него, зачем он его, всё-таки, написал) — порождает «грязь» гораздо быстрее.
А зачем вам его разбирать? Вам в нём нужно только информацию найти.

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

Ну т.е. вы предлагаете одну и ту же информацию ещё и дублировать? И в коде, и в трекере? У вас так много лишнего времени на бюрократию?
И ваш ревьюрер, увидив что вы удаляете TODO на незакрытый баг — вам ровным счётом ничего не скажет???

Д'Артаньян — 2: Возвращение Д'Артаньяна? Какой ещё ревьювер? Разве мы говорим только про публичный опенсурсный проект? Мы говорим про разработку вообще, а не конкретно про разработку gcc или хромиума. Положите вашу мантру с кодревью вон в тот ящик при входе, как будете уходить, заберёте ;)
порождает «грязь» гораздо быстрее.

Вы ошибаетесь. TODO в коде касается только того куска кода, где оно написано, и никак не влияет на работу команды. А запись в багтрекере касается всех. И хлам в багтрекере куда неприятнее.
В багтрекере у вас будет описание тикета, кто/когда его завел и возможно какие-то комментарии из той эпохи, когда его создавали.Вам этого будет достаточно, чтобы понять, делать его или нет?
Вы пропустили важную вещь — у меня будут ссылки на другие баги, связанные с этим. И да, этого может быть недостаточно — но если у вас нет ссылки на багтрекер, то у вас не будет даже этого.

У вас так много лишнего времени на бюрократию?
Да, разумеется. TODO не должно быть «индульгенций» на написание грязного кода. Если вы можете за 10-15 минут грязь убрать — то её нужно убрать и всё. А если нет — то вот эти самые 10-15 минут как раз и уйдут на подробное описание проблемы и заведение бага.

Положите вашу мантру с кодревью вон в тот ящик при входе, как будете уходить, заберёте ;)
В таком случае разговор можно заканчивать, так как никакого другого способа писать код, в котором можете разобраться не только вы наука пока не придумала. Вот просто не придумала — и всё.

Как книга, не прошедшая корректоров — это не книга, а, в лучшем случае, «сырая» рукопись, так и код, не прошедший ревьюеров — это не код, а, в лучшем случае, полуфабрикат. Потому я просто не собираюсь обсуждать подобный код. И нет, это никакое не Д'Артаньянство. Я тоже часто пишу код, который не проходит ревьюеров. Как и писатели-классики, вы не поверите, писали письма — и без всяких корректоров. Но как я не буду обсуждать вопрос «как написать качественную статью или книгу без корректоров», так я не буду обсуждать, «как писать качественный код без ревьюеров». Это просто невозможно.

TODO в коде касается только того куска кода, где оно написано, и никак не влияет на работу команды.
Влияет. Уже хотя бы тем, что в продакшн уходит неоревьюенный код. Какие там вы для себя TODO оставляете в папке «experimental» или «contrib» (в разных проектах бывают разные названия… в паре известных мне проектов такая папка вообще «tools» называлась) — никого не волнует. Но если вы посмотрите код в общую репу — то вот уже там TODO должны быть обработаны правильно и оттуда должны быть ссылки на баги. Потому что это уже не для себя. Это уже для других.

GIT вообще не про сохранность истории изменений. Он про истории снепшотов файлов. И subtree позволяет работать с историями таких вот поддеревьев из снепшотов. https://git-scm.com/book/ru/v1/%D0%98%D0%BD%D1%81%D1%82%D1%80%D1%83%D0%BC%D0%B5%D0%BD%D1%82%D1%8B-Git-%D0%A1%D0%BB%D0%B8%D1%8F%D0%BD%D0%B8%D0%B5-%D0%BF%D0%BE%D0%B4%D0%B4%D0%B5%D1%80%D0%B5%D0%B2%D1%8C%D0%B5%D0%B2

Можете пояснить на примере? Вот есть udev. До 2012го года он разрабатывался как отдельный проект, а потом переехал вот сюда.

Как мне с помощью ваших «волшебных» команд узнать что с ним случилось между версиями 2010го и 2011го года?

Почему-то все рассказывающие про магию VCS и рассказывающие, что всё просто: нужно просто сделать «хрясь-бумс-дзинь» — и всё материализуется… исчезают, когда им предлагают сделать «хрясь-бумс-дзинь» в конкретной VCS с конкретным файлом. Странно, правда?

А теперь вы похоже перепутали собеседников.)

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

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

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

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

У меня обычно нет проблем с пониманием конкретного небольшого куска кода (если только это не какой нибудь мозгодробительный трехэтажный шаблон на c++). А вот охватить более высокий уровень бывает очень сложно. Если в незнаеомом проекте сотни и тысячи файлов с исходниками, огромное число классов и функций, используются незнакомые сторонние библиотеки… тут никакие комментарии не помогут. Средств высокоуровневого структурироания и документирования кода практически нет.

Средств высокоуровневого структурироания и документирования кода практически нет.
раньше был Together Control Center, он умел комментарии из моделей переносить в методы. Но это давно было, сейчас по моему он мертв.
Я пишу автоматизированную систему тестирования. Много лет. Она ветвистая и сложная.
В Javadoc кладу такие «схемки»
и они помогают

Лично я ко всем правилам отношусь с учетом контекста. Например, комментирование кода в статье считается плохим тоном. Ок. Я однажды долго разбирался, почему очевидная кодовая конструкция не работает. Потратил неделю. Нашёл причину. Закомментировал этот кусок кода с пометкой «памятник невнимательности потому что ...» и подробное описание почему ТАК не работает.
Все советы надо пропускать через свою шкуру, а не слепо им следовать. Если слепо следовать советам, то может оказаться, что лучше За кодирование и не браться.
На минуту представил себе отсутствие комментариев в своё asm-е…
Возможно, что идеальный код не требует комментариев. Вопрос в том, сколько времени требуется для его написания. В сравнении со временем, которое требуется для написания комментариев, даже с учётом того, что на актуализацию комментариев при изменениях тоже нужно время.
Скорее я могу представить себе уменьшение количества комментариев со временем, если будет возможность (в т.ч. экономическая) дорабатывать код приближая его к идеальному роману. Но в возможность «сделать всё правильно с первого раза» я не верю. Я такого ещё ни разу в реальной жизни не встречал — только в мечтах руководства и показательных статьях в интернете.
ASM — это очень плохой пример. Если вы пишите на ASM (а не, скажем, на Ruby или Python), то, стало быть, ресурсы вас волноуют. А абстракции в ASM (в отличие, от, скажем, С++) — далеко не бесплатные.

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

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

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

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

Я и не ставлю. Но фактически в результате имеем, например такое:
    /** 
     * Конструктор - создание нового объекта с определенными значениями
     * @param maker - производитель
     * @param price - цена
     * @see Product#Product()
     */
    Product(String maker,double price){
        this.setMaker(maker);
        this.price=price;
    }

Тут кусок комментария, который в исходном коде не дает ничего нового, занимает больше строк чем код. Да, это документация, но читают ли ее разработчики? Часто javaDoc пишут не потому, что это нужно, а потому так принято и смотрится более «богато».
предоставить максимально детальное и полное описание

Комментарии для этого же.
    /** 
     * Конструктор - создание нового продукта из производителя и цены
     * @param maker - название производителя (human readable)
     * @param price - цена в основной валюте за единицу учёта
     * @see Product#Product() "у нас так принято"
     */
Код не мой, нашел на просторах интернета в статье о написании JavaDoc. Но пишут часто именно также, формально.
Ну пардон, давайте все-таки разделим необходимость таких комментариев, и их качество. Да, если написать просто цена — то комментарий ничего не дает. А если указать дополнительные свойства той цены (валюту, в частности), то очень даже начинает давать. И если вам не все равно, допустим, что это bid или ask — то вы скажете спасибо тому, кто это не забыл указать, потому что тип данных double вам этого не скажет.
Да, если написать просто цена — то комментарий ничего не дает..

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

Воспринимайте сигнатуру как часть документации. И пишите в *doc комментариях только то, что в сигнатуру не поместить технически или сделает её нечитаемой.

Поэтому мне гораздо больше нравится стиль, принятый в Qt (Doxygen такое тоже умеет, хз насчет Ява-дока):
    /** 
     * Creates a new Product with the given \a maker and \a price.
     */
    Product(String maker,double price){
        this.setMaker(maker);
        this.price=price;
    }


Количество боилерплейта сведено к минимуму (что не отменяет капитанства комментария, но а как прокомментировать конструктор?)
(что не отменяет капитанства комментария, но а как прокомментировать конструктор?)
Никак?
В данном примере название find получилось недостаточно информативным

и первое что предлагает сделать автор — прилепить к названию совершенно избыточный префикс get.

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

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

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

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

Насчет todo, опять-же у тебя есть план и видение развития кода. Просто поставил напоминание, что сделать. В дальнейшем это превращается в комментарий и служит для документирования.

И самое главное, эти сигнатуры очень неоднозначны.

Например что такое length и Count? Это вроде понятно.

Но вот пример очень и очень неоднозначного поведения.
docs.microsoft.com/ru-ru/dotnet/csharp/programming-guide/strings/index

Прекратите усердствовать, комментируя статью про комментарии. Текст статьи — это единственная истина. Читайте только статью! Да, в ней есть ошибки, но комментарии-то ведь никто не поддерживает.

Если мой комментарий не соответствует коду, то это не комментарий устарел, а код неправильный.


Обращаю внимание на слово "мой" желающим со мной поспорить.

Если вы можете говорить о коде «мой» — то вам вообще все эти советы не нужны. Когда разработчик на проекте один — они может хоть змейкой код по спирали писать. И любые правила придумывать.

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

Я говорю «мой» не только о коде, который я написал, но и о коде, который прошел мое ревью.


Код, где в комментариях не описано, что он делает и по каким контрактам, или описание не соответствует содержанию, мое ревью не пройдет.

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

Ваши дальнейшие действия?

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


Мои комментарии
/// <summary>
/// IPreferencesProvider предоставляет доступ к настройкам.
/// <para />
/// Настройки объединены в группы. У каждой группы свое уникальное имя. У каждой настройки свое уникальное имя в пределах группы.
/// <para />
/// Реализации данного интерфейса сделаны на основе следующих принципов:<para />
///     1. Количество настроек и размер значений позволяют загрузить в память одновременно все настройки.<para />
///     2. У каждой настройки имеется разумное значение по умолчанию.<para />
///     3. Поведение приложения зависит только от значения настройки, а не от наличия или отсутствия значения в системе хранения.<para />
///     4. Получение значения настройки реализовано быстро и эффективно, что позволяет вызывать его повсеместно.<para />
///     5. Значения настроек меняются редко, на несколько порядков реже, чем читаются.<para />
///     6. Объекты IPreferencesProvider безопасны при параллельном использовании.<para />
/// <para />
/// Если сущности, которые предполагается получать из системы управления настройками, не удовлетворяют указанным принципам, то использовать интерфейсы IPreferencesProvider и IPreferences нельзя, даже если по сигнатуре они подходят. Вместо этого следует воспользоваться другими библиотеками.
/// <para />
/// В модуле Preferences.RI реализованы 2 варианта поставщика настроек: DefaultPreferencesProvider и DebugPreferencesProvider.
/// <para />
/// DefaultPreferencesProvider использует простое кэширование, основанное на том, что настройки занимают мало памяти и редко меняются, и сохраняет в базе данных значение по умолчанию, если его в базе данных не было.
/// <para />
/// DebugPreferencesProvider берет значения из базы, не кэшируя и не сохраняя значений по умолчанию. Данная реализация предназначена для облегчения процесса разработки.
/// <para />
/// Автор интерфейса — Сергей Б. Вносить изменения в данный интерфейс можно только по согласованию с автором.
/// </summary>
public interface IPreferencesProvider
{
    /// <summary>
    /// Возвращает объект, предоставляющий доступ к настройкам.
    /// </summary>
    /// <param name="groupName">Имя группы настроек.</param>
    /// <exception cref="PreferencesException">Исключение, в случае любых ошибок при получении объекта реализующего <see cref="IPreferences"/> по заданному имени группы настроек.</exception>
    /// <returns>Объект доступа к настройкам.</returns>
    IPreferences GetPreferences(string groupName);
}
Если найдется настолько смелый разработчик, кто готов проигнорировать мои комментарии, то у него все равно ничего не получится.
Почему не получится?

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

Я прохожу и меняю все интерфейсы, где кто-то что-то получает по «groupName» и меняю на «groupObject». Комментарии, при этом, я не трогаю. Ваш комментарий начинает врать.

Ему просто выставят дефект, что код поломал контракт абстракции, и заставят переделывать.
Это с какого такого перепугу? Тесты работают, в проде тоже всё нормально… на основании чего вы тут собираетесь дефект заводить?

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

А потом Сергей Б. увольняется и возникают вопросы что делать с этим кодом и к кому общащаться за согласованием изменений.

И еще считаю что в закрытом коде не нужно писать имя автора когда есть blame. Сколько раз встречал ситуацию, когда в начале файла написано имя одного автора, а делаешь blame и видишь что 90% вклада в этот файл сделал совсем другой человек.

Не возникнет вопросов, что делать с этим кодом. Open-Closed principle будут соблюдать. Не нравится реализация интерфейса — welcome сделать новую. Не нравится интерфейс — напишите другой. В комментарии как раз об этом и написано.


Вы совершенно правы. Это как раз тот случай, когда блэйм покажет совсем другого автора. Есть такие организации, которые не занимаются разработкой, а покупают услуги разработчиков на стороне. У них даже версионного контроля кода нет, ибо не царское это дело — в коде ковыряться. Вот в такой конторе я однажды был вынужден кусок кода приложить к ТЗ. Закоммитил его сотрудник подрядчика. Пример гипертрофированный. Прямо так писать надо далеко не всегда. У меня есть примеры кода практически любого вида от такого, где вообще комментарии не нужны, до такого, как этот. Здесь я хотел продемонстрировать, что можно сделать, чтобы комментарии над кодом имели приоритет. Соотношение код/коммент = 1/30 + OCP.

Как-то сложно представить себе команду из нескольких тысяч разработчиков, с равными правами вносить изменения куда угодно. Не, ну не вопрос, берем условный Spark или там Chromium, или линукс… и вот вам несколько тысяч человек налицо, но ой, а что, у них разве так можно, чтобы менял кто попало? Можно реальный пример такого проекта?
Как-то сложно представить себе команду из нескольких тысяч разработчиков, с равными правами вносить изменения куда угодно.
А кто сказал про равные права?

а что, у них разве так можно, чтобы менял кто попало? Можно реальный пример такого проекта?
Ну возьмите Chromium, раз вы его подняли. Вот его репа. Вот его OWNERS. Как видите есть восемь человек, которые могут менять что угодно и где угодно. Ещё восемь — могут менять исходники, а вот эти двое — все тесты. Ну и дальше, понятно, чем «глубже» компонент, тем больше людей, которые могут его менять.

Учтите ещё, что это не люди, которые могут менять код — это люди, которые могут его принять. Сделать изменение — может вообще кто угодно.

Точно такая же схема в ядре Linux (хотя технически немного иначе реализованная).

А теперь подумайте: кто, скорее всего, сделает ваш комментарий недействительным: кто-то, кто будет править 100 строк непосредственно в вашем компоненте (и будет получать «добро» от вас) или кто-то, кто возьмёт Coccinelle и глобально трансформирует ваш код, как часть большого рефакторинга (и получит «добро» на свой patch в 100'000 строк от «владельца» всего проекта)?
>А кто сказал про равные права?
Ну, это со стороны так выглядит, когда вы пишете:

>код может, внезапно, поменяться кем-то, кто в вашу команду даже не входит

А в итоге получается, что «кем-то» — это на самом деле 8 человек, а не тысячи. Ну хорошо, не 8, а 38. Это условные владельцы мелкого компонента, и все, кто выше них, в некоторой (условной) иерархии?
А в итоге получается, что «кем-то» — это на самом деле 8 человек, а не тысячи. Ну хорошо, не 8, а 38.
Это не люди, которые могут что-то изменить. Это люди, которые могут подобный patch принять. А сделать его может кто угодно, в принципе-то.

Это условные владельцы мелкого компонента, и все, кто выше них, в некоторой (условной) иерархии?
Оне не «условная», она вполне себе безусловная: за каждый компонент есть кто-то, кто за него отвечает. И есть люди, которые отвечают за всё. А есть ещё, если присмотритесь, люди, которые могут править что-то в билдсистеме по всему проекту, менять любые пресабмиты и прочее.

Никаких «этот компонент править только согласовав с Васей Пупкиным» там нет.

Так посмотрите, какое там соотношение кода и комментариев. Подозреваю, что патч без комментов или ломающий соответствие комментов коду просто не заапрувят.


https://chromium.googlesource.com/chromium/src.git/+/refs/tags/77.0.3846.1/chrome/common/channel_info.h

Так посмотрите, какое там соотношение кода и комментариев.
Я бы сказал, что оно там «разное». Чем более стабильный код вы берёте — тем больше в нём будет комментариев. Вы взяли common! базовую библиотеку, над которой построено вообще всё. Конечно она будет прокомментирована «до упора».

А ткнитесь в какой-нибудь другой, чуть менее фундаментальный компонент… и там комментариев станет резко меньше. Вот так, например:

chromium.googlesource.com/chromium/src.git/+/refs/tags/77.0.3846.1/chrome/gpu/chrome_content_gpu_client.h

Значит, код бывает разный. Какой-то код требует документирования, а какой-то нет. OK.


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

Эти советы точно не сарказм?
Комментарий над таким выражением серьезно поможет коллегам-разработчикам понять ваш код.
… и дальше идёт комментарий, не вполне соответствующий коду.
А он сам свои рекомендации выполяет?
Ок, иду в гитхаб автора
# Read all lyrics int memory
for track in json_data["songs"]:
  all_data.append(track["lyrics"])
  all_labels.append(getTagNumber(track["tag"]))
# Post subreddit
print('post subreddit')
classify_users(X_posts_sub, Y_posts_sub, X_posts_sub_user, Y_posts_user)
print('------------------------------------------------------------')

# Comment subreddit
print('comment subreddit')
classify_users(X_comments_sub, Y_comments_sub, X_comments_sub_user, Y_comments_user)
print('------------------------------------------------------------')

# Posts
print('posts')
classify_users(X_posts, Y_posts, X_posts_user, Y_posts_user)
print('------------------------------------------------------------')

# Comments
print('comments')
classify_users(X_comments, Y_comments, X_comments_user, Y_comments_user)
print('------------------------------------------------------------')
# Get data from database
for doc in get_redditor_collection().find({}):
def process_request(self, data):
# parse request


А тут у человека ад и израиль с сериализацией. А здесь он умудрился не выкинуть из кода ключ авторизации, плюс можно наблюдать замечательную практику использования исключений (99 строка). Ну и закоментированный код там в общем-то повсюду.

Его точно стоит слушать? Почему у всех профессиональных блогопрограммистов с громкими статьями такой плохой код?
Плюс много — особенно сопоставляя требование заменить очевидный find(Status) с пожеланием «предупредить других разработчиков о возможных побочных эффектах или неприятностях» через комментарии.

Потому, что они профессионально пишут статьи, а не код

Я использую особый комментарий для маркировки грязных хаков.
Когда начинаешь писать какую-то большую функциональность, новый проект, часто делаешь функции — заглушки, которые вместо запроса к БД просто возвращают захардкоженое число, например. Отмечаем это место комментарием, и, когда придет время, все эти места легко можно будет найти.
//####
Он редко встречается в обычном коде и его легко найти визуально и с помощью IDE.
Вот сюда прямо напрашиваются TODO. Их IDEA моментально находит, плюс сам смысл тодо говорит вам, что переделай эту заглушку в нормальный код. Ну а сами решетки в текст тодо можно и добавить, дело вкуса.

IDEA позволяет любые маркеры настраивать, слово todo ей фактически не нужно

Согласен. Но получается, что Вы изобрели собственные todo, заменив это слово решетками.

Не я, а ittakir :) Я вам говорю, что IDE почти без разницы как в коде помечать todo. У JetBrains, кажется, 3 или 4 разных паттерна по умолчанию есть.

Немного разные назначения. TODO — это пометка на будущее, он может висеть в коде годами, и никто это так и не исправит.
А //#### — это временный грязный хак, который даже в коммит не попадет, скорее всего. То есть перед коммитом мы ищем все #### и исправляем их. Эдакий чек лист, чтобы не закоммитить «грязный» код.
// Константа для логических выражений
#define TRUE FALSE

// Константа для расчета траекторий
#define PI 3.1415097349871

// Оптимизация выполнения некоторых выражений
#define if while

tproger.ru/articles/preprocessor-fun

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

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

1) бывают оптимизации по ресурсам. была понятная функция, но жрала ресурсы. Переписали, уменьшили потребление даже не на порядок, а убрали множитель из сложности, но без поллитры в коде не разберешься
2) какие-то хитрые матпреобразования, за которыми целые теории лежат. Сортировки, криптография, матанализ, статанализ, теорвер и т. п.

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

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

В команде, код написанный одним человеком, читает потом несколько других. А даже один читатель может потратить больше времени на чтение, чем если он напишет его заново. Если в проекте приняты коммиты непонятно чего с описанием, что если чукча не только писатель, то сам разберется, я просто ухожу из него по принципу «не работай с мудаками». Имхо, не стоит экономить свое время на поиск способа, чтобы объяснить свой код: комментарий может отсутствовать только если он есть где-то выше и автор не пожалел минуты, чтобы взглянуть еще раз на код и подумать, как он будет читаться. Если проект публичный и читателей тысячи, то требования к комментированию еще выше.
UFO just landed and posted this here
Я пишу комментарии только к методам в интерфейсах и к самим интерфейсам. Иногда они выражают «контракт» наподобие «переопределил hashcode(), переопредели и equals()». Многие вещи кодом не описать. Из последнего, контракт «назови перечисление так же, как и имя ResourceBundle, назови константы из перечисления так же, как и поля в ResourceBundle».
Грубо говоря — хорошему коду они не нужны, а плохому не помогут.
Если читатель хочет понять, что делает функция и алгоритм, то это можно впихнуть в имя функции. Но если он хочет понять, как он это делает, то это похоже на рассматривание картины под лупой: кусочек виден детально, а всю картину увидеть сложно.

Запрос SQL на 20 страницах к десяткам таблиц. Или посмотрите функцию поиска по АХО-КОРАСИК (при условии что нет никаких описаний) и попробуйте понять, КАК она работает чисто из кода.
Эта статья — перекомпонованная и видоизменненная глава «Комментарии» из Чистого Кода Мартина Роберта.
Все должно быть в меру.
Проблема большинства, даже опытных программистов, в том, что они считают, что их имена переменным, методам, классам самые понятные для всех :) И поэтому не хотят сопровождать код комментариями вообще :)

Комментарии полезны, если они к месту.
TODO полезны, если они к месту.
FIXME полезны, если они к месту.

(с) Кэп.
Sign up to leave a comment.