Search
Write a publication
Pull to refresh

Comments 44

лучший комментарий — это тот, который не нужен

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

Да, это так. Роберт Мартин довольно эпатажный автор. Но смысл его высказывания, тем не менее, понятен.

Комментировать что делает одна строка кода малоосмысленно.

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

Или когда есть достаточно объемное ТЗ со сложной логикой. И вам нужно привязать определенные блоки кода к определенным пунктам ТЗ. Тут без комментариев не обойтись.

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

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

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

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

Не всегда. Когда пишется что-то реально объемное и реально сложное, каждые 2-3 строки в метод выносить - только запутывать код. Не говоря уже о том, что это снижение производительности (каждый вызов метода - создание, а потом свертывание еще одного уровня стека). Если все это вас не волнует - вам повезло.

Метод - это некая законченная логическая единица. И она может быть достаточно сложной логически и неочевидной. Метод "каким клиентам нужно посылать уведомление" описывается в ТЗ на 10-15-ти страницах, а в коде выливается в 5 выборок (каждая из которых есть SQL почти на станицу по нескольким таблицам с кучей условий), а потом еще каждый элемент выборки проверяется по 3-4 дополнительным условиям (если еще и их в запрос включать, он будет непозволительно долго выполнятся т.к. станет безумно сложным).

И всегда останется вопрос - почему в методе needClientSendNotification эта самая необходимость проверяется именно по признакам А, Б и В именно в этой таблице, на не признакам Г, Д и Е в другой таблице (потому что можно и так и этак).

Вот простой пример. Нужно

Найти в таблице HDAPF наличие записи по условию
• HDACUS = CUS
• HDACLC = CLC
• HDATYP = ‘DOC’
• HDAMBN = 1,4,5
• И максимальным HDACRD

       SetGT ($Cus: $Clc: 'DOC') HDA02LF;
       readp HDA02LF;

       dow not %eof(HDA02LF) and 
           HDACUS = $Cus and 
           HDACLC = $Clc and 
           HDATYP = 'DOC';
         if HDAMBN in %list('1': '4': '5');
           @DAT = HDADAT;
           leave;
         endif;

         readp HDA02LF;
       enddo;

Чтобы понять что этот код работает правильно (а первая реакция человека будет - "а где проверка на максимальный CRD - его тут вообще нигде нет"), вам потребуется заглянуть в структуру индекса HDA02LF

И только там вы увидите что

     A                                      UNIQUE
     A          R HDAPFR                    PFILE(HDAPF)
     A          K HDACUS
     A          K HDACLC
     A          K HDATYP
     A          K HDACRD

Последнее поле - HDACRD. Т.е. все записи группы HDACUS-HDACLC-HDATYP отсортированы по возрастанию CRD. И поэтому поставив указатель на "максимальное значение в группе" (SetGT) и прочитав "запись назад" мы получим именно запись с максимальным CRD. Ну а дальше проверяем допусловие по HDAMBN и если онj не выполнятся - читаем еще назад пока не найдем нужное (а как нашли - выходим из цикла).

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

Но достаточно добавить комментарий

       // HDA02LF отсортирован по HDACRD. 
       // Посему ставим на конец цепочки CUS-CLC-DOC и идет назад пока не найдем запись с HDAMBN = 1,4,5

Как все становится очевидно.

 Не говоря уже о том, что это снижение производительности (каждый вызов метода - создание, а потом свертывание еще одного уровня стека). Если все это вас не волнует - вам повезло.

Инлайнинг, оптимизация хвостовой рекурсии?

зацепила фраза

это снижение производительности

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

это может приводить к снижению производительности

то и докопаться было бы не до чего =)

Когда пишется что-то реально объемное и реально сложное, каждые 2-3 строки в метод выносить - только запутывать код

Неправда. Если вы даёте методам осмысленные, качественные имена, код становится сильно понятнее.

Не говоря уже о том, что это снижение производительности

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

К счастью, производительность требуется достаточно редко – я не говорю о каких-то крайних применениях ПО - управление производством, геймдев и т.д., а об "обычных" системах. Поэтому при написании кода обязательно стараемся соблюдать его качество, о производительности не думаем – пока не прилетит соответствующая задача :).

И всегда останется вопрос - почему в методе needClientSendNotification эта самая необходимость проверяется именно по признакам А, Б и В именно в этой таблице, на не признакам Г, Д и Е в другой таблице (потому что можно и так и этак).

Ну т.е. Вы предлагаете в код в виде комментариев вносить требования (ТЗ)? Ну это такое.

Требования всегда есть в виде отдельной документации, именно туда надо ходить и смотреть – почему это устроено именно так, а не иначе.

Неправда. Если вы даёте методам осмысленные, качественные имена, код становится сильно понятнее

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

К счастью, производительность требуется достаточно редко – я не говорю о каких-то крайних применениях ПО - управление производством, геймдев и т.д., а об "обычных" системах.

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

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

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

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

Ну т.е. Вы предлагаете в код в виде комментариев вносить требования (ТЗ)? Ну это такое.

Это совершенно нормально. Потому что в наших ТЗ "алгоритм работы модуля" может содержать полтора-два десятка логических блоков. А то и больше. И аналитики никогда не пишет его до псевдокода. Он пишет что нужно сделать, а как это сделать наиболее эффективным способом - это уже задача разработчика.

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

будет крайне сложно по коду увидеть ошибку

В моей Вселенной для поиска ошибок придумали отладку (debug). Искать причину баги глазами по коду??? Увольте.

производительность и эффективность на первом месте

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

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

Конечно, именно это и есть роли аналитика (написать требования) и разработчика (написать код). Но почему при этом надо требования (ТЗ) запихивать в код в виде комментариев - мне решительно непонятно. Если при чтении кода у меня возникнет вопрос, почему используются именно A, B, C и не используются D, E, F - я открою постановку и прочитаю там. Замусоривать код лишней информацией вообще ни к чему.

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

На одном из проектов, где я техлидил, комментарии были просто запрещены (с парой исключений), и при этом никто не страдал, в том числе новички, код был ясен и понятен.

В моей Вселенной для поиска ошибок придумали отладку (debug). Искать причину баги глазами по коду??? Увольте.

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

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

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

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

На одном из проектов, где я техлидил, комментарии были просто запрещены (с парой исключений), и при этом никто не страдал, в том числе новички, код был ясен и понятен.

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

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

Вы предлагаете в код в виде комментариев вносить требования (ТЗ)? Ну это такое.

Про ТЗ соглашусь. Попытка вывести прямое, однозначное соответствие между ТЗ и кодом может привести к сомнительным решениям. ТЗ лучше проецируется на приемочные тесты, чем на код.

Увы, но нет.

При работе с долгоживущими и постоянно развивающимися проектами вам на доработку придет код, написанный не вами и 5 лет назад. И ТЗ где написано что "в п.3.9.10 нужно внести вот такие изменения". И, поверьте, если в коде нет комментариев связывающих блок кода с пунктом в ТЗ, вы потратите немало времени чтобы понять - а где именно в коде нужны эти изменения внести.

А "приемочные тесты" - это вообще не про ТЗ (FSD), а про бизнес-требования (BRD). А это совсем другой документ, написанный совсем другим языком и не для разработчика, а для аналитика. И в этих тестах в код вообще никто не смотрит. Там прогоняют тестовые сценарии на предмет соответствия тому что подается на вход и что получается на выходе ожидаемым результатам (если на вход подаём это, на входе должны получить это). Ну плюс нагрузочное и интеграционное тестирование еще.

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

Многое, с чем я могу согласиться, например, есть в этом комментарии: https://habr.com/ru/articles/929600/#comment_28604372 — очень точно сказано и про приоритет читаемости, и про то, что в большинстве проектов производительность — не первое, о чём стоит беспокоиться.

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

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

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

в большинстве проектов производительность — не первое, о чём стоит беспокоиться

Ну это моя профессиональная деформация... Мне всю жизнь приходилось думать про эффективность. Ну так сложилось...

Разбиение метода на 2–3 вспомогательных почти никогда не станет узким местом.

В целом соглашусь, тут уже утрировал.

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

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

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

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

Так что комментарии все равно приходится писать. На 100% самодокументированый код не получается когда работа идет со сложными структурами данных и сложной бизнес-логикой.

Просто одно другое дополняет, а не отменяет.

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

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

Не соглашусь, выносить блоки кода в функции только чтобы не писать комментарий, не выглядит опрпвданным в 100% случаев

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

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

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

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

Плюс страдает читаемость. Если в функции заложена какая-то логика сложнее чем проверка флага или 2+2, а эта логика используется внутри другой логики, то чтобы понять что скрывается за "проверка_чего-то_там" - как оно проверяется, правильная ли там логика проверки, придется прыгать по коду туда-сюда.

Понятно, что если оно в 10-ти местах используется - тогда да. А ради одного места - таки нет.

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

Тогда стоит более точно указывать название аргументов: например, taxRateNotPercent. По мне, лучше словами описать.

Да, оба варианта возможны, тут нужно смотреть по контексту.

От себя дополню, что предложенная вами формулировка NotPercent может оказаться не вполне однозначной. Я бы предлжил fractionalTaxRate.

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

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

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

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

class GeoLine {
public:
  //! \brief возвращает длину линии в _метрах_, потенциально затратный вызов
  //! \warning в случае местной системы координат длина будет в единицах проекции
  double Length(void) const;
  //! \brief возвращает длину линии в _единицах проекции_
  double LengthUnits(void) const;
}

Или содержат примеры использования:

// \example LOG_TIME(Info) foo(); //выведет в лог время выполнения foo()
#define LOG_TIME(Lv) if(log::impl::timer<Lv> t; t)

Комментарии могут содержать напоминания - знание, которое надо не забыть учесть при изменении кода (даже если прямо сейчас это знание не порождает какого-то решения, про которое надо объяснять "почему"):

int foo(double* xBegin, double* xEnd, const double* yBegin){
  // yBegin имеет право совпадать с xBegin
  //...
}

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

/// Какой из этих трёх комментариев здесь уместнее? Или никакой?
// определяем, с какой стороны луча находится точка
// подставляем точку в уравнение прямой
// наша прямая aX+bY+c=0, подстановка (p.x,p.y) даёт знаковое расстояние
double dist = a*p.x + b*p.y + c;
if(dist > 0) { //справа по направлению обхода
  //...
} else {
  //...
}

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

Да, тоже хотел упомянуть, но не получилось так точно сформулировать.

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

Затруднительно - это еще мягко сказано. Писать имена переменных "параметр_который_не_может_быть_больше_5_и_меньше_1" - ну такое себе...

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

Именно. И эти объяснения опять таки в имя никак не внести. В целом это может быть краткое описание алгоритма которое сильно помогает понимать что именно делает код.

Как-то пришлось делать модуль для транслитерации. Особенность была в том, что "ЯКОВ" транслитерируется в "YAKOV", а "Яков" в "Yakov" Или "Я." в "Ya." Т.е. регистр второго знака в сочетании зависел от контекста.

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

Прекрасный пример. Давайте разбирать, почему комментарии всё ещё не нужны.

  //! \brief возвращает длину линии в _метрах_, потенциально затратный вызов
  //! \warning в случае местной системы координат длина будет в единицах проекции
  double Length(void) const;

возвращает длину линии в метрах

Значит, оно должно возвращать не double, а что-то в духе Meters или Unit<Meters, double>.

потенциально затратный вызов

Length переименовываем в ComputeLength, и затратность уже понятна по имени.

А вообще что такое «затратный вызов»? Там внутри тригонометрия используется, и поэтому оно тормозит, или оно в БД идёт и получает данные о данном пути, и поэтому тормозит (но на несколько других масштабах)? Комментарий об этом не говорит вообще.

И, кстати, в LengthUnits такого комментария нет. Я так понимаю, оно не затратное? Или забыли написать? Непонятно.

в случае местной системы координат

Ничего не понял. Где этот случай торчит в API? Откуда я как читающий этот код в первый раз знаю, где и какой случай подразумевается?

длина будет в единицах проекции

Это почти как stringly-typed code, только doubly-typed.

В плюсах лучше возвращать std::variant<Meters, ProjUnits>, например, и комментарий снова не нужен, и рефакторить-развивать такой код — одно удовольствие.

А в языках с нормальными системами типов мы вообще пишем что-то в духе

LengthType : CoordSystem → Type
LengthType Local = ProjUnits
LengthType _ = Meters

length : (line : GeoLine) → LengthType (coordSystemOf line)

-- или, альтернативно, если имеет смысл
-- индексировать сам GeoLine
-- координатной системой, в которой оно живёт:
length : GeoLine cs → LengthType cs

Значит, оно должно возвращать не double, а что-то в духе Meters или Unit<Meters, double>.

В принципе справедливо, и я предпочёл бы находиться в мире, где это так, но а) плохо применимо когда уже есть уйма кода с double, включая C-интерфейсы нескольких разных библиотек; б) я взял простой и компактный случай как иллюстрацию, можно взять менее компактный где система типов всё равно не поможет выразить смысл (ну, или описание типа будет чрезмерно громоздким).

Length переименовываем в ComputeLength, и затратность уже понятна по имени.

Не согласен. Вы решили одну проблему, но создали другую: теперь у объекта нет просто метода Length(), который в 90% случаев можно и нужно использовать как простой аксессор. Оговорка сделана оговоркой для оставшихся 10%.

А вообще что такое «затратный вызов»?

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

И, кстати, в LengthUnits такого комментария нет. Я так понимаю, оно не затратное? Или забыли написать?

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

Ничего не понял. Где этот случай торчит в API?

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

В плюсах лучше возвращать std::variant<Meters, ProjUnits>, например, и комментарий снова не нужен

Не очевидно что лучше. Потому что объектов прорва, а опция variant у них будет одна и та же. Её можно понять заранее во внешнем коде. (И это нелокальное условие, которое я не знаю как выразить типами.)

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

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

Это внешнее ограничение, не связанное напрямую с темой статьи (что имена важны и делают большинство комментариев ненужными) и моего коммента (что типы важны и делают большинство комментариев ненужными). ИМХО это как-то ближе к «без типов тяжело», чем к «комментарии нужны».

В конце концов, есть js вообще без типов, и там это не работает совсем. Что я делаю? Просто не пишу на js.

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

А это было бы интересно обсудить.

Не согласен. Вы решили одну проблему, но создали другую: теперь у объекта нет просто метода Length(), который в 90% случаев можно и нужно использовать как простой аксессор. Оговорка сделана оговоркой для оставшихся 10%.

Для меня это звучит почти как «код, который работает в 90% случаев». Как я узнаю, попадаю я вот здесь вот в 90% или в 10%? Как я узнаю это, проводя код ревью, где комментарии используемых методов не видно? Или вот у меня функция на три уровня выше тормозит, а она дёргает Length, что я не вижу в профайле, потому что заинлайнилось всё три раза — мне идти все комментарии ко всем функциям до каждого листа дерева вызовов теперь смотреть?

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

std::optional<Meters> GetLength() const;
Meters ComputeLength() const;

или сделать

Meters GetLength(ComputationWitness) const;

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

У данного типа - нигде (и нет, его индикатор в данном классе не является уместным), отсюда и предупреждение.

Окей, спрошу по-другому. Какую минимальную проверку мне надо написать перед вызовом Length(), чтобы знать, что он будет дешёвым (и вернуть 0, например, если это не так)?

Не очевидно что лучше. Потому что объектов прорва, а опция variant у них будет одна и та же.

Сорри, не могу разрешить неоднозначность. Объектов, работающих с длинами и их возвращающих, прорва, и если один из них, так получилось, засунул в variant метры, то и все остальные точно будут работать только с метрами и в эти странные юниты не вылезут? Если да, то это как раз значит, что [в мире выразительных типов] все эти штуки должны быть индексированы используемой координатной системой. Тогда вам не нужно никаких конверсий и проверок в рантайме, и всё проверяется компилятором в компилтайме.

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

А там, кстати, где перф на самом деле нужен до наносекунд (всякое хфт), я так делаю даже на плюсах. Просто всё становится темплейтом, и у вас будет

template<CoordSystem>
struct CoordSystemTraits
{
  using LengthType = Meters;
};

template<>
struct CoordSystemTraits<LocalCoordSystem>
{
  using LengthType = ProjUnits;
};

template<CoordSystem S>
using LengthOf = typename CoordSystemTraits<S>::LengthType;

template<CoordSystem S>
class GeoLine
{
public:
  LengthOf<S> GetLength() const;
};

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

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

Это да, но это не комментарии к конкретному месту в коде (именно потому что они нелокальны). Им место в README.md, в папочке docs/, или где-то ещё в подобных отдельных местах.

Спасибо за аргументированный комментарий. Со многими мыслями в нём я согласен.

Про напоминания — да, полностью поддерживаю. Это действительно хорошая и уместная роль комментариев: сохранить знание, которое сейчас вроде бы ни на что не влияет, но может оказаться критичным при изменениях. Такие вещи в код "не впишешь", и комментарий тут вполне уместен.

Что касается встроенной документации — я предпочитаю отделять её от обычных комментариев в коде. То, что оформляется через Javadoc, PHPDoc, Doxygen и т. п., — это всё-таки документация к интерфейсу, и она действительно должна описывать семантику, ограничения, контракты, примеры использования и прочее. У себя на проекте я как раз продвигаю такую практику, и она обязательна для публичных методов.

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

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

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

Про осмысленность нейминга вопросов вообще нет.

Но назначение комментариев не в том чтобы писать "тут вычисляем ставку налога" - это бессмысленный комментарий. Назначение комментария - кратко описать какую-то сложную и неочевидную логику, суть неочевидного алгоритма. Это очень облегчает понимание кода. Особенно, когда (а лично мне приходится очень много работать именно с таким кодом) на доработку (изменение бизнес-логики, оптимизация) приходит код, написанный 5 и более лет назад. А если я пишу что-то новое, свое, то всегда думаю о том, что через те же 3-5 лет кому-то другому возможно придется что-то в этом коде менять и разбираться в том, как это реализовано и почему реализовано именно так, а не иначе.

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

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

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

Пример по поводу соответствия реализации требованиям ТЗ (псевдокод).

=== ТЗ ===

Функция calculateFinalAmount(order) должна реализовывать следующий алгоритм:

  1. Подсчитать сумму всех товаров в заказе, исключая товары с пометкой "подарок".

  2. Применить персональную скидку клиента, если она указана.

  3. Применить промокод, если он присутствует и действителен.

  4. Если сумма после скидок превышает 10 000, применить дополнительную скидку 5%.

  5. Добавить налог на добавленную стоимость (НДС) в размере 20% от суммы после всех скидок.

  6. Если заказ содержит хотя бы один товар с категорией "алкоголь", применить акциз 10% к их стоимости до скидок.

  7. Вернуть итоговую сумму и расшифровку примененных скидок и налогов.

=== Реализация ===

def calculateFinalAmount(order)
    orderTotal = calculateOrderTotal(order: order)
        .addToOrderOperations()
    orderTotal = applyCustomerPersonalDiscount(sum: orderTotal)
        .addToOrderOperations()
    orderTotal = applyCoupon(sum: orderTotal, coupon: order.coupon)
        .addToOrderOperations()
    orderTotal = applyValueDiscount(sum: orderTotal, minValue: 10000, discountPerCent: 5)
        .addToOrderOperations()
    orderTotal = applyVat(sum: orderTotal, ratePerCent: 20)
        .addToOrderOperations()
    orderTotal = applyAlcoholExcise(sum: orderTotal, orderItemInitialPrices: order.items.filterAlcohol().getPrices(), ratePerCent: 10)
        .addToOrderOperations()

    return [
        orderTotal: orderTotal,
        orderOperations: getOrderOperations()
    ]

Все верно. Но тут нет сколь бы то ни было сложной логики. Все на уровне 2+2.

Увы, но не могу привести примеры из тех ТЗ с которыми работаю (во-первых, слишком длинно - там одна функция описывается н 3-5 страницах), во-вторых, там слишком много конкретики "не для публикации".

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

  • счет в статусе «Открыт»

  • счет открыт в день

  • маска счета входит в список разрешенных

  • исключить запрещенные маски счетов

  • ограничить выборку классом клиента ФЛ

  • ограничить  выборку условием пустого ИНН или не подвержденного

И это только одна выборка. Остальные не проще.

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

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

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

Но факт в том, что аналитик не способен сразу расписать все тонкости эффективного алгоритма - он не настолько хорошо погружен в технические особенности стека.

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

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

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

accountBuilder = findAccounts()
  .open()
  .createdAt(creationDates)
  .accountMask(allowedMasks)
  .excludeAccountMasks(prohibitedMasks)
  .individualsOnly()
  .innNotApproved()
// ...
accountBuilder.applyExtraConditions(preBuiltExtraConditions)
// ...
return accoundBuilder.getAll()

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

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

Задача решается, например, применением строителя условий

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

        exec sql declare curProcAccounts cursor for
                   select GF.GFCUS,
                          GF.GFCLC,
                          GF.GFCRF,
                          cast('F' as char(1))
                     from ABALPF ABAL

                     join SCPF SC
                      on (SC.SCNANC, SC.SCOAD) = (ABAL.ABALNBR, :dte)

                     join GFPF GF
                       on GF.GFCPNC = SC.SCAN

                    where ABAL.ABALMSGT          = 'FNSFMSG'
                      and ABAL.ABALOPEN          = 'Y'
                      and SC.SCCAD               = 0
                      and SC.SCAI80             <> 'Y'
                      and GET_CTP_TYPE(GF.GFCTP) = 'F'
                      and not exists (select EXCPT.EXCABAL
                                        from EXCPTPF EXCPT
                                       where EXCPT.EXCABAL  = SC.SCNANC
                                         and EXCPT.EXCGROUP = SC.SCSAC);

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

        dou lastRow;
          exec sql fetch curProcAccounts for :C_MAX_SQL_ROWS rows into :dsData;

          exsr srChkSQLError;

          lastRow = sqlGetRows(rowsRead);

          for row = 1 to rowsRead;
            // Если ИНН требует подтверждения и тип идентификации входит в список разрешенных,
            // заносим данные в список
            eval-corr dsListKey  = dsData(row);

            if chkCRFNotConf(dsData(row)) and chkUIDTPI(dsListKey);
              SKPL_InsertData(pDataList: %addr(dsListKey): %addr(dsListData): %size(cusDULt));
            endif;
          endfor;
        enddo;

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

Да, без номеров, но с максимально близко к тексту ТЗ. Чтобы было можно связать кодл с ТЗ.

Да, без номеров, но с максимально близко к тексту ТЗ. Чтобы было можно связать кодл с ТЗ.

Под этим утверждением подпишусь. Если уж нужно ссылаться на ТЗ, то не по номерам, а по формулировкам или заголовкам.

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

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

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

Все верно. SQL сразу отбирает тех у кого

  • счет в статусе «Открыт»

  • счет открыт в день

  • маска счета входит в список разрешенных

  • исключить запрещенные маски счетов

  • ограничить выборку классом клиента ФЛ

А

  • ограничить  выборку условием пустого ИНН или не подвержденного

Реализуется в chkCRFNotConf Вот только

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

не прокатит - в "других таблицах" могут быть миллионы записей. И не всегда их нужно читать. В том же chkCRFNotConf:

      //==============================================================================
      // проверка неподтвержденности ИНН
      //==============================================================================
      dcl-proc chkCRFNotConf ;
        dcl-pi *n ind;
          dsData  likeds(t_dsData);
        end-pi;

        dcl-f FINNST20LF disk(*ext)
                         usage(*input)
                         usropn
                         keyed
                         qualified
                         static;
        dcl-f GF01LF     disk(*ext)
                         usage(*input)
                         usropn
                         keyed
                         qualified
                         static;

        dcl-ds dsGFRec       likerec(GF01LF.GFPFR:         *all);
        dcl-ds dsGFKey       likerec(GF01LF.GFPFR:         *key);
        dcl-ds dsFINNST20Key likerec(FINNST20LF.FINNSTPFR: *key);
        dcl-s  result        ind;
        dcl-s  CRF           like(t_dsData.CRF);

        // Если ИНН не определен, нужно его получить из карточки
        if dsData.CRF = cNoCRF;
          if not %open(GF01LF);
            open GF01LF;
          endif;

          dsGFKey.GFCUS = dsData.CUS;
          dsGFKey.GFCLC = dsData.CLC;

          chain %kds(dsGFKey) GF01LF.GFPFR dsGFRec;

          if %found(GF01LF);
            CRF = dsGFRec.GFCRF;
          endif;
        else;
          CRF = dsData.CRF;
        endif;

        if CRF = *blanks;
          result = *on;
        else;
          if not %open(FINNST20LF);
            open FINNST20LF;
          endif;

          dsFINNST20Key.STCUS  = dsData.CUS;
          dsFINNST20Key.STCLC  = dsData.CLC;
          dsFINNST20Key.STINN  = CRF;  
          dsFINNST20Key.STHIST = 'A';
          dsFINNST20Key.STFROM = 'Y';
          

          setll %kds(dsFINNST20Key) FINNST20LF;
          result = not %equal(FINNST20LF);
        endif;
      
        return result;
      
        begsr *pssr;
          dump;
        endsr;
      end-proc;

Во-первых, он на вход может принимать уже прочитанный ИНН. Тогда его нужно просто проверить (он может быть пустым, подтвержденным или неподтвержденным). В данном случае он "бесплатно" приходит их SQL - там все равно эта таблица задействована. Но есть выборки где она не задействована, а цеплять ее только ради ИНН дорого - она достаточно большая - более 50млн записей. В этом случае дешевле прочитать из нее одну запись по индексу прямым доступом к БД (chain).

Если ИНН в итоге пустой - все понятно. Нужно отправлять запрос. А если нет - смотрим подтвержден он или нет. Это уже другая таблица FINNST. И по ней есть индекс FINNST20LF в который входит признак подтвержденности ИНН - STFROM = 'Y'. И вот тут нам не надо читать запись из таблицы. Нам достаточно знать есть такая запись в индексе или нет - setll позиционируется на запись с заданным или меньшим значением ключа (но не читает ее из таблицы). А %equal говорит нам о том, попали на точное значение, или нет. И чтение (read) тут не нужно - лишнее время. Это уже тонкости языка (аналитик их знать не обязан, а разработчик - должен знать).

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

А еще в ТЗ есть такой пункт

Дополнить выборку из пункта «Отбор клиентов для актуализации ИНН» уникальными записями из файла CLJMNPF где CLJDT = $Date и CLEJMNPF где CLJCUS = CLEJCUS и CLJDT = CLEJDT;

По каждой отобранной записи:

  • если есть запись в файле CLEJMNPF, то выполнить поиск записи в файле указанном в поле CLEJFILE, вызвав Get_File_Record передав на вход CLEJFILE:CLEJKYL:CLEJKYE

  • заполнить структуру customerDUL найденной записью RDKPF

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

И вот тут в ТЗ не написано что этот же клиент уже может быть в списке - попасть туда из других выборок. И логика тут не просто "добавить", а проверить - если есть в списке, то смотрим - если там уже заполнен ДУЛ, то добавляем на этого клиента еще одну запись. А если не заполнен, то в существующую запись списка заполняем ДУЛ.

А если его там еще нет - добавляем новую запись с заполненным ДУЛ.

Так что реальная логика оказывается сложнее того, что написано в ТЗ.

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

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

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

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

На самом деле порядок выполнения решается и проще

      // Этапы вполнения
      dcl-enum enStages qualified;
        stageAccounts   1;
        stageNames      2;
        stageUnconfGRF  3;
        stageChngUID    4;
        stageChngCBH    5;
        stageCLJ        6;
        stageCrdHldr    7;
        stageSend       8;
      end-enum;

И далее

        for-each stage in enStages;
          select stage;
            when-is enStages.stageAccounts ; // Отбор по счетам
              procAccounts(procDte: dsError);

            when-is enStages.stageNames    ; // Отбор по изменению Имени
              procNames(procDte: dsError);

            when-is enStages.stageUnconfGRF; // Отбор по неподтвержденному ИНН
              procUnconfCRF(finnstFrom: finnstTo);

            when-is enStages.stageChngUID  ; // Отбор по изменению идентификации
              procUID(finnstFrom: finnstTo);

            when-is enStages.stageChngCBH  ; // Отбор по изменению статуса самозанятого
              procCBH(procDte);

            when-is enStages.stageCLJ      ; // Отбор из витрины ЖМ
              procCLJ(procDte);

            when-is enStages.stageCrdHldr  ; // Отбор держателей карт из МПК
              procCardHolders(procDte);

            when-is enStages.stageSend     ; // Отсылка запросов
              sendRequests(dsError);
          endsl;

          // Если на каком-то из этапов вернулась ошибка - завершаем работу
          if dsError <> *blanks;
            leave;
          endif;
        endfor;

Порядок выполнения определяется порядком следования в перечислении. Даже не значениями (они могут быть любыми - цифры 1, 2, 3..., буквы 'A', 'B', 'C', ... или вообще любые символы...) а именно порядком т.к. for-each идет по порядку объявления.

Но если порядок имеет значение - это необходимо указать в комментарии. Чтобы кому-то не пришло в голову что-то поменять "потому что так красивее".

В данном случае sendRequests всегда должен идти последним - он работает уже с заполненным на предыдущих этапах списком.

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

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

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

"Как надо" - это упрощенный пример. Который часто сознательно или подсознательно подгоняется под иллюстрируемую концепцию.

А в жизни приходится ровно наоборот - выбирать из нескольких возможных вариантов наиболее подходящую под данную ситуацию концепцию. И вот тут уже вступает в силу "как не надо..."

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

// Вася, не забудь про налоги и скидки, иначе бухгалтерия сам знаешь что…

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

Если же в коде реализован сложный алгоритм - он должен быть в первую очередь описан в БЗ проекта. И там он (алгоритм) должен поддерживаться в актуальном состоянии вместе с кодом, но другими людьми - аналитиками.

В остальном - мысль простая: все должно быть в меру. Когда комментарий необходим - его надо написать. Такие ситуации действительно бывают: как минимум, когда пишут "почему" реализовали так, как есть. Иногда есть неочевидные вещи, которые тянутся из зависимостей. Иногда ты пишешь код, который изначально пойдет в техдолг, и комментарий нужен, что бы через 2-3 недели ты в целом не забыл, что тут происходит. Ну и т.п.

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

плохо, когда устаревает коментарий, но когда устаревает имя метода - еще хуже

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

Сюда можно включить, например, принципы SOLID. Буква O из этой аббревиатуры, напомню, — Open-Closed Principle. В контексте поднятой вами проблемы его можно понимать так, что не следует менять имеющееся поведение системы. Если кратко и грубо — не меняй старый метод — пиши новый. То есть, поведение метода не должно меняться никогда. Поэтому и его имя не устареет.

Если же в коде реализован сложный алгоритм - он должен быть в первую очередь описан в БЗ проекта. И там он (алгоритм) должен поддерживаться в актуальном состоянии вместе с кодом, но другими людьми - аналитиками.

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

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

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

Именно. У Макконнела в "Совершенном коде" эта тема раскрыта на 42 страницах (считал по оригиналу), у Мартина в "Чистом коде", как я уже упоминал, ей тоже уделено внимание.

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

Я всегда всем объясняю так: если ты пишешь комментарий, который отвечает на вопрос "как?" Или "что?", то вместо этого перепиши код, чтобы было понятно что он делает без комментариев.

А вот вопросы "зачем?", "почему?" и "для чего?" это именно то что кодом никак не выразить и требуется раскрывать комментариями.

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

Ещё хороший поинт - если тебе на ревью задают вопрос "почему ты сделал так" - писать комментарии вместо объяснения в тредах пулл-реквеста.

Ещё более важный вопрос "почему именно так, а не иначе?"

Sign up to leave a comment.

Articles