понедельник, 20 мая 2013 г.

О комментариях

"Не стоит относиться к комментариям как к "абсолютному добру". На самом деле, комментарии в лучшем случае являются неизбежным злом. Если бы языки программирования были достаточно выразительными или если бы мы умели искусно пользоваться этими языками для выражения своих намерений, то потребность в комментариях резко снизилась бы, а может быть и вовсе сошла "на нет".
Роберт Мартин "Чистый код"

"Комментарии должны сообщать о коде что-то такое, что он не может сообщить он сам – в виде заключения или в виде намерений".
Стив Макконнелл "Совершенный код"

clip_image002

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

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

И вот, почему.

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

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

Как? Что? Почему?

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

Использование любых "нестандартных" коллекций или алгоритмов требует пояснения. Разумно объяснить, почему используется LinkedList вместо List или SortedList вместо Dictionary. Вероятно, в первом случае, мы собираемся добавлять элементы в середину коллекции, а во втором случае мы определили, что логарифмическая сложность бинарного дерева дает более высокую производительность, по сравнению с константной сложностью хеш-таблицы.

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

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

clip_image004

Автор кода: - Вот здесь я использую ленивое вычисление на основе yield return, а вот тут вызываю ToList внутри блока try/catch. Поскольку ToList триггерит исполнение всего LINQ запроса, то именно здесь нужно обрабатывать FormatException, с которым может упасть метод Parse, вызываемый в запросе.

Ревьюер: - Ок, но раз это так важно, то почему бы не описать эту “важность” в виде пары комментариев? А то ведь твой коллега, не понимая этой важности легко сможет вынести обработку ошибок в строку формирования запроса, поломав всю существующую логику!

Комментарии должны объяснять намерения программиста на более высоком уровне абстракции, чем код и прояснять причины текущего решения!

Вот пара простых советов по поводу комментариев:

  • Комментарий не должен говорить, как мы решаем задачу. Вместо этого лучше сделать сам код более ясным.
  • Комментарий может прояснять "что мы делаем" на более высоком уровне абстракции (*).
  • Хороший комментарий говорит, не просто что мы делаем, а поясняет почему мы делаем именно так.
  • Если что-то важно в вашем коде, то сделайте эту важность максимально очевидной! Лучше с помощью самого кода, на крайний случай – с помощью комментариев.

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

В следующий раз, рассмотрим простой пример эволюции кода и комментариев.

30 комментариев:

  1. Для разъяснения сути надо давать подходящие имена классам\ф-ям\переменным, а вместо пояснений к логике писать тесты ее проверяющие.

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

    ОтветитьУдалить
  3. "Как" не нужны. "Что" описывается именем метода и класса. Высокоуровневое "что" нарушает идею о том, что класс не должен знать, что делает вызывающий код.

    Остается "почему" в самом коде и диаграммы вне кода.

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

    ОтветитьУдалить
  4. Да, спасибо этому блогу за знакомство с контрактами =)

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

    Для полной гармонии поправьте, пожалуйста, опечатки:

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

    "Использование любых "нестандартных" коллекций или алгоритмов требуют пояснения." -> "Использование любых "нестандартных" коллекций или алгоритмов требует пояснения." (Использование ... требует пояснения)

    "с которым можем упасть метод Parse, вызываемый в запросе." -> "с которым может упасть метод Parse, вызываемый в запросе."

    ОтветитьУдалить
  5. >Для разъяснения сути надо давать подходящие имена классам\ф-ям\переменным, а вместо пояснений к логике писать тесты ее проверяющие.

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

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

    ОтветитьУдалить
  6. @Анатолий: Спасибо за дополнения. И, если не сложно, присылайте плз. пожелания по очепяткам в личку;)

    ОтветитьУдалить
  7. @Constantine:

    > И сколько времени потратишь, для того чтобы выяснить почему тут так, а не иначе?

    Сколько бы не было об этом написано, ничего, кроме вашего опыта в этом деле не поможет:)

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

    ОтветитьУдалить
  8. >>почему используется LinkedList вместо List

    Тут уж действительно без комментария не обойтись. LinkedList - самая неоправданно популяризированная структура в С#. Частота упоминаний в литературе прямо пропорциональна убогости реальных возможностей.

    ОтветитьУдалить
  9. @brat-kolya: ну, у нее другая сложность вставки в середину и другая сложность доступа к произвольному элементу.

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

    Просто должен быть хинт, что это действительно оправдано.

    ОтветитьУдалить
  10. @Konstantin:
    > Высокоуровневое "что" нарушает идею о том, что класс не должен знать, что делает вызывающий код.

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

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

    ОтветитьУдалить
  11. >> у нее другая сложность вставки в середину и другая сложность доступа к произвольному элементу.

    Согласен, в теории все именно так.

    ОтветитьУдалить
  12. >> Согласен, в теории все именно так.
    А на практике как? Разве иначе?

    ОтветитьУдалить
  13. >>А на практике как? Разве иначе?

    Как ни странно - да, иначе. Был неприятно удивлен несколько лет назад при попытке использовать LinkedList в реальном приложеннии. Обычный List намного удобнее и по быстродействию не уступает. Просто проверить на тестовом примере. Наверное, есть область, где применение LinkedList оправданно - но она очень узкая, практически незаметная.

    Извините за офф-топик - мои коментарии имеют прямого отношения к теме статьи.

    ОтветитьУдалить
  14. Про комментарии:

    http://img9.joyreactor.cc/pics/post/%D0%9A%D0%BE%D0%BC%D0%B8%D0%BA%D1%81%D1%8B-geek-it-%D0%BF%D1%80%D0%BE%D0%B3%D1%80%D0%B0%D0%BC%D0%BC%D0%B8%D1%81%D1%82%D1%8B-94269.jpeg

    ОтветитьУдалить
  15. @brat-kolya: да, область, где LinkedList полезен и правда маленькая, например, вставка в середину. Если вы сравните именно эту операцию для List и LInkedList, то там разница для больших n (количества элементов списков) будет существенная.

    @Constantine: LOL
    Хотя это скорее про чужой код в целом, а не только про комментарии:))

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

    ОтветитьУдалить
  17. > Есть случаи, когда нам действительно нужна быстрая вставка в середину и тогда LinkedList очень полезен.
    по-моему совсем не повод для написания комментария. это скорее общий уровень владения языком в комманде. в моей текущей комманде такие "неочевидные" вещи скорее пойдут в документ "coding guidelines" и короткую презентацию для команд на "design meeting", который проводится раз в неделю для всех разработчиков.
    Ну и короткая заметка про читаемый код: http://www.simpleprogrammer.com/2013/04/14/what-makes-code-readable-not-what-you-think/

    ОтветитьУдалить
  18. Честно говоря не совсем понял, что охватывается термином диаграммы, но в статье упущен важный момент - документация API, например, в формате doxy.

    ОтветитьУдалить
  19. @Andy: под термином "диаграммы" охватываются ключевые UML-диаграммы, ИМХО.

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

    Мне кажется то, что подходит для API может быть перебором для кода приложения.

    ОтветитьУдалить
  20. @Sergey Teplyakov Кажется я понял, что ты имеешь в виду под высокоуровневыми комментариями.
    Такой пример подойдет:

    // Маршируем на плацу, причем принято начинать с левой
    Цикл(от_забора_до_обеда)
    {
    Шаг левой;
    Шаг правой;
    }


    Я же ругал другой пример
    // Как полковник прикажет, идем маршировать
    void Walk()
    { ... }
    Такой комментарий указывает на место конкретного вызова метода, и непонятно, можно ли его использовать вне того контекста.

    А вот такой комментарий снова хорошо, ибо он описывает предусловия:
    // Перед вызовом убедитесь, что плацу.
    void Walk()
    { ... }

    ОтветитьУдалить
  21. сорри за опечатки.
    ... что вы на плацу

    ОтветитьУдалить
  22. Вот не соглашусь с тем, что комментарии, описывающие как решается задача - не нужны. Да, в подавляющем большинстве случаев нормального именования переменных и методов достаточно. Однако, попробуйте как-то на досуге так поименовать переменные внутри quicksort'а, чтобы смысл алгоритма и его корректность стали видны человеку, который с ним не знаком. И метод в десяток строчек влезет, и переменные названы хорошо - а все равно вряд ли поймут.

    Итого: когда это самое "как" неочевидно из контекста и кода - комментарии насчет него не просто нужны. Они обязательны!

    ОтветитьУдалить
  23. Почти кэповская статья, хотя судя по комментариям и возражениям, наверное просто я думаю похоже) Так, как обычно все верно Серега написал. Ещё бы добавил, что чем краше с каждой версией становится c#, тем больше я думаю нам придется писать комментарии. Как один из баян примером - работа с разными linq провайдерами, когда частенько приходится писать комментарии решарперу)

    ОтветитьУдалить
  24. Можно ли настроить микрофон, по фазам луны, воспользовавшишсь средневековым арабским трактатом и не включая его?
    Всегда нужна обратная связь. Для чего пишут комментарии? Для коллег или для себя через год. Так что можно сначала писать как получиться, а потом на основании обратной связи редактировать. Только так приблизимся к совершенству.

    ОтветитьУдалить
  25. @Евгений: боюсь, что если вы при разработке фичи не добавите комментарий, то через год вы этого уже сделать не сможете, поскольку тонкости этой фичи будут забыты.
    Это все равно, что тесты писать через год, когда баг вылезет. Так это дело вряд ли взлетит;)

    ОтветитьУдалить
  26. @Sergey Teplyakov: Я не предлагаю не писать вообще :) Я предлагаю не придумывать правила написания, а сформировать их пользуясь обратной связью. Эти правила вообще будут отличаться для разных комманд, людей, проектов...
    В принципе, нужно тем подробней, чем больше людей потом будут смотреть код. Оптимальную "величину подробности" опять можно выяснить только посредством обратной связи.

    ОтветитьУдалить
  27. @Евгений: вопрос в том, какие критерии использовать для "первой итерации" написания комментариев.

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

    ОтветитьУдалить
  28. @Ivan:
    >> Итого: когда это самое "как" неочевидно из контекста и кода - комментарии насчет него не просто нужны. Они обязательны!

    Мне кажется, что "как" и "что" меняются в зависимости от уровня абстракции.

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

    ОтветитьУдалить
  29. Вот что ещё вспомнил, часто забывают, что комментарии кода не только в коде, но и в VCS (системе контроля версий). Некоторые вещи в commit message'ах расписывают так, что коду и не снилось. Так что полезно иногда делать что-то типа

    git show $(git annotate __FILE__ | grep '__LINE__)' | cut -f1)

    ОтветитьУдалить
  30. @Andy Shevchenko Да, комменты в Source control это очень полезно! У данной стратегии очень хорошие сайд эффекты. Когда не можешь описать что в Чекине, значит ты не задачей занимался, а всем понемножку.

    ОтветитьУдалить