Почему я снова комментирую код приложений на Ruby/Rails

Здравствуйте, я — разработчик программного обеспечения на Ruby / Rails и я комментирую свой (а с недавних пор ещё и чужой) код. Голос из зала, вероятно, крикнул бы «Привет, разработчик!»

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

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

Основным тезисом моего краткого рассказа будет: «О сложности кода может судить только его автор».

Итак, прежде всего нужно уточнить, когда же именно работает правило «Вместо комментирования измени код так, чтобы комментирование стало излишним». Оно работает всего лишь в нескольких случаях:
  • Когда код написан неквалифицированно: слишком длинно (возможно, разработчик плохо знает стандартную библиотеку и писал много лишнего кода сам) или слишком сложно (perl-style, однострочные цепочки функций длиной более 5-ти функций подряд)
  • Когда код включает в себя редкоиспользуемые или нехарактерные для данного ЯП методы и приёмы

И всё! В этих случаях, действительно, лучше такой код переделать. Переделать, убедится, что код стал короче, яснее, понятнее для коллег и… наконец-то написать комментарии! И вот я уже добрался до того места, где и буду пояснять свои идеи.

Пояснение для Идеи № 1: «О сложности кода адекватно может судить только его автор».

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

Что же будет, если один или двое программистов уйдут из команды, в другой проект, в другую студию или просто в отпуск? Вероятно, на их место будут взяты один и два других разработчика, которым дадут код, дадут задачи и… и всё. Предварительно, HR-менеджер спросит у них, умеют ли они «разбираться в чужом коде» и «работать в команде», а они, конечно же, ответят, что умеют. И вновь прибывшие окажутся один на один с кодом и коллегами, которые быть может и рады отвечать на вопросы «а почему здесь так...» или «а зачем там это...», но это отвлекает от работы. Поэтому разработчик вынужден «разбиться с проектом». Количество рабочего времени, потраченного на «выполнение» своим мозгом чужого кода с целью пройти по нему и понять, что он делает — ужасает. Поясню на примере.

Представим себе функцию, вроде такой:

def update
   list = current_user.lists.update(params[:id], params[:list])
end


С точки зрения правила о простоте кода, здесь просто рай. Комментировать тут нечего, одна строка, совершенно никаких сложностей. Однако, представим себе, что params формируется не простой формой на странице, а с помощью кода на Javascript в Backbone.js, что lists — это на самом не lists, а оставшееся от прошлой команды название и модель эта теперь везде в тестах называется Things, что на before_save этой модели навешена функция, которая берёт некие поля и создаёт отложенное задание, которое парсит, согласно данным этих полей некие URL'ы (возможно, без контроля ошибок HTTP-протокола), чтобы потом сохранить полученные ответы в другой таблице. А иногда вызывает exception'ы и шлёт сообщения о них… куда-то… на сайт сбора ошибок, куда программисту дадут доступ, как только он сам напомнит. А совсем иной контроллер (API, например) отдаст значения этих полей ajax'ом в Backbone'овскую View. Кстати, можно ещё уточнить, что данная функция должна рендерить шаблон, сначала пропуская его через парсер RABL, формируя там JSON, совершенно непохожий на изначальное содержимое list, который Things. Ну и для полного комплекта уточним, что работать это должно на хостинге с какими-нибудь ограничениями, к примеру, без доступа к cron'у. И с качестве СУБД используется NoSQL.

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

Есть и другая часто встречающаяся проблема.

В проекте N функциональность M обычно реализуется с помощью XYZ. Не зная кода полностью или не прочитав документации к проекту, описывающей структуру и используемые XZY, разработчик не может знать, X, Y или Z используется ли уже или нет. И если нет — почему не используется. Не существует программиста, который может сразу же начать работать и создавать новое в незнакомом ему проекте, это миф о «крутости разработчика». Лучшее, к чему может привести подобная ситуация — разработчик сделает свою задачу так, как умеет, отдаст на code review и получит код обратно с комментарием «Зачем сделал так, ведь у нас для этого принято использовать [список каких-то технологий]» или «Зачем сделал так, ведь можно было так: [описание сущностей, выяснить существование которых самостоятельно невозможно (например, наличие триггеров или функций в БД на production-сервере]»

Так что же я предлагаю, уже давно пора спросить читателю. Прежде всего, делайте с кодом, над которым работаете, несколько простых вещей:
  • Описывайте то, как работает этот код. Обязательно, хоть и кратко, в начале каждого файла, содержащего класс, описывайте, что он реализует. Перед методами класса кратко описывайте, что метод получает и что должен создать в конце своей работы, коротко описывайте цепочку, по которой пройдут данные. Так же в файлах-модулях. Комментируйте вычисляемые автоматически значения и назначения создаваемых миграциями полей. В случае длинной цепочки вызовов функций (длиннее трёх) — описывайте цепочку, что вызывает что, хотя бы просто последовательность.
  • Ведите в каждом файле блок TODO, в одном и том же месте, хоть сразу после объявления класса или модуля.
  • Ведите в каждом файле блок IN_WORK (назовите как угодно Вам), в котором пишите о том, какие места в данный момент подвергаются переделке/рефакторингу/устарели и вместо них нужно использовать BLABLA из класса XYZ.
  • Обязательно описывайте все факты, наличие которых нельзя вывести из имеющейся документации!


Для последнего пункта приведу пример: если ваш проект несовместим, например, с MySQL, а разработчик потратил полдня, пытаясь развернуть проект именно на этой СУБД, не вздумайте сказать ему «Мы же не поддерживаем MySQL!». В ответ вы услышите в лучшем случае «А почему я об этом не знал?». И если вы скажете этому парню фразу «А почему ты не спросил?» — можете быть уверены. что в вашем проекте уже существуют большие проблемы с документированием и это уже наносит вам ущерб. Ведь никто из нас не знает точного списка всего, что он не знает!

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

Подробнее
Реклама
Комментарии 36
  • +1
    У меня такое ощущение, что пост про «наболевшее» не лучший выход, и стоило бы воспользоваться помощью таких замечательных инструментов, как Google Groups. Я намекаю на обсуждение данной проблемы в Ror2Ru (=

    Есть так же мнение, что читаемости коду добавляет следование DDD, но к сожалению данная методология только набирает обороты (хотя очень сильно).
    • +1
      Вероятно, Вы правы насчёт «поста о наболевшем», однако, разве не так улучшается мир? Некто пользуется несовершенным инструментом, у него после долгого использования «наболело», он громко заявляет об этом и начинает искать варианты улучшений. И находит. Или другие обращают внимание на проблему и находят.
      • +2
        Ну я к тому, что «наболевшее» можно обсудить в сообществе, и узнать, кто к каким практикам пришел. Можно конечно и на хабре, но когда есть тематическое сообщество — почему нет? (=
      • 0
        Я думаю вы представляете себе те сложности, с которыми старые матёрые говнокодеры (некоторым из них, правда нет и 30) с кучей пафоса и сигарой в зубах рассуждают об концепциях старой школы и отвергают даже попытки упоминания о DDD, методах с экран и правилами комментирования. Через 5-10 минут разговор перерастает в дремучий холивар о том, что паттерны — зло и «я все пишу правильно не зная ни одно паттерна уже 100500 лет» (напоминает вопрос о 5% самых умных людей). А потом с этим УГ, которое везде суются в виде говнокода ActiveRecord с кучей логики и остального лапшекода вперемешку с алгоритмами приходится разбираться и что-то ещё там фиксить.

        В литературе (Code Complete Second Edition by Steve McConnell, Chapter 32) рекомендуется писать комментарии так:

        1. Документация состоит из комментариев в листингах и внешней информации(документации).
        2. Стоит писать документы для классов, методов и членов классов.
        3. Другой вид(внутренний) документации — сам код. Очень важен хороший стиль программирования, а не только комментарии.
        4. Циклы выполняют только 1 функцию, нормальный вариант следует внутри if, а не else,
        5. булевы выражения упрощаются доп. булевыми функциями.
        6. Полезны резюмирующие комментарии, комментарии цели (писать зачем мы делали то или иное действие)
        7. Стоит применять комментарии которые несложно редактировать и поддерживать.
        8. Один комментарий на 10 операторов — оптимально.
        9. Нужны комментарии отдельных строк (если они сложны для понимания )
        10. Обязательно нужны комментарии строк, где ранее была ошибка
        11. Стоит избегать комментария в конце строк, кроме случаев объявления переменных, исправления ошибок.
        12. Лучше описывать абзацы кода.
        13. Комментарий должен отвечать на вопросы как или почему (if (accountFlag == 0))
        14. Комментарий может играть роль заголовка или роль пояснения сюрприза(повышения производительности за счёт чего-то).

        Вывод. Комментарии — must have. Комментируют метод, выходные переменные, результаты, граничные значения, неоднозначные операторы и их наборы, простую алгоритмическую логику внутри метода. Краткость — сестра таланта. Не нужно писать поэм.
        • 0
          Я хоть и старый матерый говнокодер, но паттерны и DDD использую, методы и функции короткие, *doc не ленюсь писать (хотя бы чтобы IDE подсказки адекватные давала), тесты пишу (обычно до кода) и т. п. Единственное не понимаю, что плохого в ActiveRecord с кучей логики, если уж начали его использовать (мне больше DataMapper & co нравится, но частенько альтернативы нет). Не, можно, конечно, на каждый класс типа User с персистентным состоянием и методами для этого вводить класс типа UserHandler без состояния (или временным) и инжектить туда User, но нужно ли?
          • +1
            Всё, что Вы перечислили — прекрасно. Я срочно в гугл искать эту великую книгу.
        • 0
          Комментирование само по себе вроде и хорошо и правильно, но помимо всего требует неимоверной дисциплины от всей команды.

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

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


            Нет, такой сценарий почти нереален, если делается Code Review. Коммит просто будет отправлен на доработку. Само собой, мы предполагаем наличие адекватного review'ера, остальные случаи рассматривать надо совсем отдельно.
            • +1
              Да ладно, даже если предположить, что у нас идеальный ревьюер, который никогда не делает глупых ошибков (хотя все люди их делают), то может легко остаться какой-нибудь комментарий в заголовке модуля, где упоминается функционал, или в другом методе, который пользуется этим методом — он просто не попадет в контекст ревью в тако случае
              • 0
                Code review — это отдельно взятая ситуация. Но не всегда она имеет место быть. Как и наличие адекватного review'ра. Review'ер может быть адекватным, но иметь какой-то уровень доверия к тому или иному разработчику, чтобы проверять документацию. Ну в общем, допущений в реальной жизни слишком много, ИМХО.
                • +1
                  Очень даже реален :) В code-review в основном читают только те строчки, которые поменялись. Бывает что-то вокруг, чтобы понять контекст. И очень редко раскрывают спрятанные блоки (обычно прячется всё, что за функциями, а значит и шапка-комментарий тоже). И получается, если комментарий не правили, то он не подсветится и скорее всего его и не посмотрят.
                  • 0
                    Если так принято делать в команде — то вероятность ревьюером это забыть такая же, как и не заметить ошибку в коде.
                    • 0
                      Не знаю как где, но на моей практике код ревьювят не для того, чтобы найти ошибки. CI, юнит тесты и QA делают это в разы эффективнее.

                      Ревьювят в целом для трёх вещей:
                      — попридираться к пробелам, отступам и именам переменных. Т.е. стиль кода, читаемость.
                      — убедиться, что решение было сделано правильно, а не воткнуто костылём. Т.е. архитектура, технический долг.
                      — потроллить автора вопросами «а что если?». Т.е. ЧСВ и поиск исключительных ситуаций.

                      Первое и второе нигде, кроме как во время ревью не сделать. А третье, это не совсем тот поиск ошибок, который делает CI и юнит тесты, т.е. не поиск регрессий, и даже не проверка, что фикс действительно работает, а больше тренировка мозга программиста, чтобы привыкал думать.
                      • 0
                        Под ошибкой я и подразумевал второе. Ну и различные косяки обычно тоже отлавливаются. Если в команде принято коментить и на ревью придёт изменение без изменений коментов — это примерно первый случай.
              • +7
                … что lists — это на самом не lists, а оставшееся от прошлой команды название и модель эта теперь везде в тестах называется Things

                Начнем с того, что переименуем везде lists в things — вот и начали рефакторинг в сторону упрощения кода. И дальше поехали, упрощать, отделять и абстрагировать :)
                • +1
                  На меня в этом плане серьезно повлиял TDD — вот уж где пришлось овладеть принципом «low cohesion, high coupling». Как только встает задача «протестировать одну фичу из всей этой макаронины» — сразу начинаешь все разделять на разные сущности, код логически становится гораздо проще. Меньше shared state, больше явного взаимодействия.
                  • 0
                    «low cohesion, high coupling»

                    а не наоборот?
                    • 0
                      Ха, вы меня поймали, и правда что-то фигню сморозил :)
                • +2
                  Не хотел сначала читать статью, т.к. она про руби, а руби я не знаю, но прочитал и не разочарован — ваши советы можно применить к любой технологии!
                  • +2
                    Описывайте то, как работает этот код. Обязательно, хоть и кратко, в начале каждого файла, содержащего класс, описывайте, что он реализует. Перед методами класса кратко описывайте, что метод получает и что должен создать в конце своей работы, коротко описывайте цепочку, по которой пройдут данные. Так же в файлах-модулях. Комментируйте вычисляемые автоматически значения и назначения создаваемых миграциями полей. В случае длинной цепочки вызовов функций (длиннее трёх) — описывайте цепочку, что вызывает что, хотя бы просто последовательность.


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

                    Я бы озаглавил статью: «Поскольку я всё ещё не пишу тесты, я снова комментирую код приложений на Ruby/Rails» :) Без обид.
                    • +2
                      Возьмите документацию по Qt и представите как бы она выглядела в виде тестов. Стало бы вам проще разобраться с нуля в библиотеке?
                      • 0
                        1. Пример прекрасной документации библиотеки Rspec, написанной исключительно на тестах: www.relishapp.com/rspec/rspec-expectations/docs. Вообще, покопайтесь на Relish — там вся документация построена на тестах.
                        2. Когда вы пишете публичное API неважно веб-сервиса или библиотеки типа QT, то да, конечно, оно должно быть хорошо задокументировано. Но речь идёт о документации кода, который живёт и постоянно меняется. Речь идёт о документировании методов, которые пишет внутренний разработчик, который, априори, должен разбираться как тут всё устроено. Тесты для этой цели подходят прекрасно, иногда гораздо лучше комментариев, ибо ты видишь живой код, видишь, как он применяется и как он работает, с какими данными на входе и с какими на выходе.
                        • +1
                          В общем случае тесты не решение. Как показано в книге Мейера Object-Oriented Software Construction: описание дизайна не яляется очевидным следствием описания требований. Все таки есть два независимых типа документации «Что должно делать?» и «Как реализовано?» (requirements, design). Тесты отлично годятся для первого. Писать тесты для реализации в прнципе возможно, но это просто не выгодно: дублирование реализации, плохая читабельнось и так далее.
                          • +2
                            Есть ещё иногда очень важный вопрос «Почему так реализовано».
                            • 0
                              Да, обычно это non function requirement, то есть часть описания требований.
                      • +1
                        «Поскольку я всё ещё не пишу тесты, я снова комментирую код приложений на Ruby/Rails» :) Без обид.


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

                        P.S. Перечитал и понял, что как-то неясно выразился. Постараюсь в будущем развить концепцию своих идей и изложить её яснее :)
                        • 0
                          Я не люблю test_unit, но даже тесты, написанные и его помощью дают прекрасное представление о том, что делает код. Пример: github.com/rails/rails/blob/master/activesupport/test/number_helper_test.rb

                          Речь, конечно же, о коде, который находится в эволюции и постоянно изменяется.
                        • +4
                          В последнее время часто встречаю утверждения, что комментарии и тесты взаимозаменяемы. В некоторых случаях это так, в некоторых нет. Область применения комментариев гораздо шире, чем описание аргументов и результата. В то же время идеальный тест следует алгоритму «подготовить аргументы (состояние) -> выполнить действие -> проверить результат (состояние)». Тест не скажет о том, почему был выбран конкретный способ решения, какие у него могут быть побочные эффекты, какие существуют альтернативы. А если приходится менять незнакомый код, то подобные вещи важны. Что не отменяет важности тестов.
                        • 0
                          Добавив комментарии, вам понадобится подерживать и их, помимо кода.
                          • 0
                            или слишком сложно (perl-style, однострочные цепочки функций длиной более 5-ти функций подряд)

                            perl-style

                            perl-style 7-летней давности perldoc.perl.org/5.8.8/perlstyle.html
                            perl-style сейчас perldoc.perl.org/perlstyle.html

                            не вижу там ничего такого.
                            • 0
                              Я с вашими аргументами полностью согласен, только не согласен, что тонкости взаимодействия модулей (а вот этот метод с триггером), общие решения (не поддерживаем MySQL) и так далее стоит описывать в коментариях. Это же как раз случай для написания нормальной документации! Ну представьте, вместо того, что бы внезапно наткнуться в коде на важный коментарий, гораздо лучше прочитать все в одном месте заранее, да еще и с картинками, ссылками, нормальным форматированием.

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

                                Если новичка кто-то кинул в проект не объяснив «как тут принято» — это не проблема документации.
                                Если новичок сделал очевидно типовую задачу так, «как пришло в голову», а не спросил у коллег — так он и документацию читать не будет.

                                Я понимаю, что отсутствие доков вовсе — это крайность, но вы тут описываете другую крайность. И за псевдо-экономию времени новичка вы будете платить реальными часами всех участников проекта. Конечно, если в проекте, условно, 3 человека и в среднем один человек в месяц «ротируется», то о документации стоит задуматься, но в этом случае документация — не основная проблема :)
                                • +1
                                  Комментарии и документация — это часть коммуникаций.
                                  • 0
                                    Есть такая штука: communication bandwidth. Безусловно, комментарии — это часть коммуникаций. Но пресловутый bandwidth у них в разы меньше.
                                • +5
                                  Ожидал увидеть пример хорошо написанного кода, требующего комментарий, а увидел пример обычного плохого.
                                  1. модель не переименована
                                  2. коллбэков следует избегать
                                  3. обращениям к внешним интерфейсам место в контроллерах

                                  Плюс хорошее именование функций, переменных, и вопрос о комментировании пропадает сам собой.

                                  Комментарии порой нужны, но пример для этого лучше привести другой.
                                  • 0
                                    А вот такая ситуация: матерый быдлокодер (возможно с командой) ваяет проект (не веб-студия а, допустим, предприятие). Начинает борзеть (забивать на просьбы пользователей, требовать выше ЗП, отказываться внедрять доп. фичи и т.п.). Умный начальник решает — «давай-ка я введу в бригаду пару новичков из института — поубавить спесь с разрабов, да уменьшить зависимость от существующих работников».

                                    Приходит новичок, а там… Ни единого комментария.

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