Как стать автором
Обновить

Комментарии 32

Коментарии писать вообще зло. Пишите так, что бы не приходилось писать комментарии и будет Вам счастье.
НЛО прилетело и опубликовало эту надпись здесь
Идеальный код, этот тот, которого нет. Без шуток. Его не нужно поддерживать, тестировать, документировать, он не создает зависимостей, не засоряет неймспейс, и в нем нет багов. Поэтому если какой-то код можно удалить ничего не ломая и не усложняя, то его нужно удалить.

«Совершенство достигается не тогда, когда уже нечего прибавить, но когда уже ничего нельзя отнять.»
НЛО прилетело и опубликовало эту надпись здесь
Как бы… «Почему/зачем» — единственный вопрос, на который должен ответить комментарий. Ибо на вопрос «как» по определению отвечает сам код.

Иногда приходится и "как" описывать, но крайне редко.
Обычно это касается случаев сложных алгоритмов, оптимизированных на производительность.
Например, в средней команде мало кто сможет сам разобраться, как работает сверхбыстрый алгоритм B* дерева, хранящегося в файле с постраничным кэшированием. А если там ещё и unsafe для ускорения складывания в/из буфера используется — комментарии must have. Несмотря на 100% покрытие тестами (не только модульными, но и интеграционниками).
И да, этот код был необходим. По требованиям от заказчика необходимого быстродействия достичь не удалось ни на MSSQL, ни на файловой БД(SQLite) ни на прочих вариантах.
Универсальные решения проиграли именно в силу универсальности.

Для описания алгоритмов делается документация, с таблицами, схемами, гувернандками и пасьянсом.

Не любят разработчики читать доки.
Плюс здесь комментарии "по месту".
А процедуры — большие, в угоду производительности (да, делали замеры — разбиение на методы даже с хинтом Aggressive Inlining даёт про Сашку по скорости на 10-15% в релизе)

Не про Сашку, а просадку… Автозамена :(

я бы сделал референс на Literate Programming Кнута, и суть даже в том что комменты там — для описания по-месту именно подходят, и чередуются с кодом.

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

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

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

Это же код! Чего вы боитесь? Переписали — протестировали, не получилось — откатили. Настоящий секрет: писать чистые функции без побочных эффектов. Тогда даже писать не надо будет в половине случаев, т.к. они сами будут генерироваться.
Рассмешил))) тесты, они ведь есть не на каждом проекте (если бизнес не даёт времени т.е. денег на сие и это сильно усложняет жизнь)
Есть мнение, что TDD не сильно замедляет разработку (если вообще замедляет). Потому что быстрее приходишь к верному решению и изначально пишешь тестируемый код, а его легче менять, он более чистый.
А практика писать в стиле «нам некогда останавливаться и менять квадратные колёса на круглые, у нас сроки» на самом деле замедляет тебя уже через час.
Есть мнение, что TDD не сильно замедляет разработку (если вообще замедляет). Потому что быстрее приходишь к верному решению и изначально пишешь тестируемый код, а его легче менять, он более чистый.

Есть возражения
1) не все можно легко протестировать, например в геймдеве, тем более если есть куча рендерного\визуального, что можно оценить глазами, AI не изобрели; хотя согласен что многие вычислительные штуки надо тестировать автоматически
2) кучу фичей, которые постоянно добавляются\удаляются\меняются — нерентабельно покрывать тестами т.к они поменяются буквально завтра и все тесты (кроме основного кода) выкинуть
3) если это стартап, дело даже не в том что сложно тестировать\нецелесообразно, а в том что по некоторому мнению (я его тоже придерживаюсь) тратиться не столько как вы выразились «не сильно замедляет разработку» а очень даже существенно лишнего времени, я бы сказал что раза в 1.5-2, и за это время может проект стать мало кому нужным, плюс фичи будут выкинуты (см пункт 2) и нужно быстрее (максимально быстро) выходить на рынок

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

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

А вариант писать код и тесты так, чтобы не надо было кучу времени тратить на написание и поддержку тестов, не рассматриваете? :)

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

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

Однако, чтобы извлечь полезный вывод из того, о чем вы задумались, нужно еще стараться быть объективным и (само)критичным. (Само)критика — то, что как раз сортирует то, что допустимо/хорошо и недопустимо/плохо. Но чтобы это было объективным решением, нужно просто учиться, повышать свою квалификацию, получать лучшую базу для принятия этих решений. Это ли не то, что уже само по себе поможет писать лучший код?

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

И нет, возможность объяснить чьи-то (или свои) некачественные решения далеко не всегда делает их, таким образом, допустимыми. А обоснованное объективное указание на такое недопустимое решение, в свою очередь, не имеет ничего общего ни с «обсиранием», ни с чем-то негативным вообще — это полезный и необходимый элемент обратной связи. Хотя, очевидно, есть множество таких же недостаточно взрослых людей, которые эту разницу не видят и отрицают, считая, что можно и нужно давать только положительную обратную связь.
В статье нет ответа на вопрос в заголовке
Для решения таких проблем существуют системы типов и TDD.
Статья правильная.
А вот теперь гуру TDD и систем типов, и желающих писать код без комментариев, аргументируйте как тут (см мой пример) можно НЕ писать никаких комментариев, и потом как это будет просто поддерживать без-комментариев VS с-комментариями.
Тот, кому легко и все понятно в своих проектах, просто делает относительно простые и рутинные вещи, без обид. Поэтому им (не говорю конкретно кому) досатточно и чистый код, и никаких комментариев. И TDD здесь не причем — всю внутрянку тестить — и усложнять и увеличивать кол-во тестового кода (для покрытия оригинала) это будет высоко-вероятно в 2 раза больше кода (а тесты тоже код), чем комменты написать, хотя да такие вещи стоит тестировать автоматически, но дело в описании почему и зачем и что (в том числе), а не только «правильно ли это работает»

Пример из моего проекта
namespace Assets.SVG_Importer.SubpixelMorphologicalAA
{
    /// <summary>
    /// Рендер мгновенного снимка SMAA (Subpixel Morphological Anti-Aliasing)
    /// </summary>
    /// <remarks>
    /// Применимо в конце на итоговую текстуру, чтобы не остались артефакты блендинга
    /// (как при MSAA остаются) и не остался алиасинг после масок;
    /// В отличие от SSAA не требует повышенного размера текстур для рендера;
    /// Хуже работает на тонких объектах,
    /// поэтому SSAA отдельно или SSAA+SMAA - где-то лучше чем чистый SMAA;
    /// </remarks>
    /// <remarks>
    /// Оригинал порта для Unity: https://github.com/Chman/SMAA/blob/master/SMAA/Scripts/SMAA.cs
    /// 
    /// Особенности изменений в порте
    /// * чтобы не требовалось задавать лишнего (параметры вшиты в шейдер):
    /// UsePredication = false (без камеры и доп. текстуры)
    /// Quality = QualityPreset.Medium (кач-во достаточное, иначе из-за Diag и т.п. артефачит т.е. не все антиалиасит)
    /// DetectionMethod = EdgeDetectionMethod.Color (именно он, т.к. при Luma был баг с отсутствием АА на некоторых цветах)
    /// Hdr = HDRMode.Auto (текстуры создаются во вне, одинакового типа)
    /// * в шейдерах внесены изменения для поддержки GLES 2.0 (Android, WebGL 1.0)
    /// 
    /// Текстуры для алгоритма должны быть в ресурсах:
    /// AreaTex
    /// SearchTex
    /// </remarks>
	public class ImmediateSmaa
	{
        /// <summary>
        /// Переиспользуемая RT 1
        /// </summary>
        private readonly RenderTexture _reusableRt1;

        /// <summary>
        /// Переиспользуемая RT 2
        /// </summary>
        private readonly RenderTexture _reusableRt2;

        /// <summary>
        /// Материал фильтра SMAA
        /// </summary>
        private readonly Material _smaaMaterial;

        /// <summary>
        /// This texture allows to obtain the area for a certain pattern and distances
        /// to the left and to right of the line
        /// </summary>
        private static readonly Texture2D AreaTex = Resources.Load<Texture2D>("AreaTex");

        /// <summary>
        /// This texture allows to know how many pixels we must advance
        /// in the last step of our line search algorithm, with a single fetch
        /// </summary>
        private static readonly Texture2D SearchTex = Resources.Load<Texture2D>("SearchTex");

        /// <summary>
        /// Pass IDs..
        /// </summary>
        private const int PassEdgeDetection = 2; // Color (именно он, т.к. при Luma был баг с отсутствием АА на некоторых цветах)
        private const int PassBlendWeights = 4;
        private const int PassNeighborhoodBlending = 5;

        /// <param name="reusableRt1">Переиспользуемая RT (non-sRGB sampler)
        /// (размер всех текстур равен: исходной, для результата и промежуточных)</param>
        /// <param name="reusableRt2">Переиспользуемая RT (non-sRGB sampler)</param>
        public ImmediateSmaa(RenderTexture reusableRt1, RenderTexture reusableRt2)
        {
            _reusableRt1 = reusableRt1;
            _reusableRt2 = reusableRt2;

            Shader shader = Shader.Find("Hidden/Subpixel Morphological Antialiasing");
            if (!shader.isSupported)
            {
                throw new Exception("ImmediateSmaa: SMAA shader is not supported");
            }

            _smaaMaterial = new Material(shader);

            // (подразумевается в настройках текстуры - linear color space, non-sRGB sampler)
            _smaaMaterial.SetTexture("_AreaTex", AreaTex);
            _smaaMaterial.SetTexture("_SearchTex", SearchTex);

            int width = reusableRt1.width;
            int height = reusableRt1.height;

            _smaaMaterial.SetVector("_Metrics", new Vector4(1f / (float)width, 1f / (float)height, width, height));
        }

        /// <summary>
        /// Рендеринг в переиспользуемую RT, с фильтром
        /// </summary>
        /// <param name="source">Исходная RT (gamma color space, non-sRGB sampler)</param>
        /// <returns>Переиспользуемая RT с результатом (gamma color space, non-sRGB sampler)</returns>
        public RenderTexture Render(RenderTexture source)
        {
            GraphicsUtils.AssureRenderTextureIsCreatedNotLost(_reusableRt1);
            GraphicsUtils.AssureRenderTextureIsCreatedNotLost(_reusableRt2);

            GraphicsUtils.ClearRenderTexture(_reusableRt1);
            GraphicsUtils.ClearRenderTexture(_reusableRt2);

            // Edge Detection (source - gamma color space for color/luma edge detection)
            Graphics.Blit(source, _reusableRt1, _smaaMaterial, PassEdgeDetection);

            // Blend Weights
            Graphics.Blit(_reusableRt1, _reusableRt2, _smaaMaterial, PassBlendWeights);

            // todo debug
            //TextureSaveUtils.SaveRenderTextureToFile(_reusableRt1, "Q:\\SmaaStep1_Debug.png");
            //TextureSaveUtils.SaveRenderTextureToFile(_reusableRt2, "Q:\\SmaaStep2_Debug.png");

            /**
             * All texture reads and buffer writes must be non-sRGB, with the exception
             *     of the input read and the output write in
             *     'SMAANeighborhoodBlending' (and only in this pass!). If sRGB reads in
             *     this last pass are not possible, the technique will work anyway, but
             *     will perform antialiasing in gamma space.
             *     
             * Note: source Gamma correction производится внутри измененного шейдера,
             * чтобы в linear color space производился антиалиасинг; на выходе шейдера Gamma space
             */

            // Neighborhood Blending
            _smaaMaterial.SetTexture("_BlendTex", _reusableRt2);
            Graphics.Blit(source, _reusableRt1, _smaaMaterial, PassNeighborhoodBlending);

            // todo debug
            //TextureSaveUtils.SaveRenderTextureToFile(source, "Q:\\PreSmaa_Debug.png");
            //TextureSaveUtils.SaveRenderTextureToFile(_reusableRt1, "Q:\\AfterSmaa_Debug.png");

            return _reusableRt1;
        }

        /// <summary>
        /// Очистка RT
        /// </summary>
        public void Free()
        {
            _reusableRt1.Release();
            _reusableRt2.Release();
        }
	}
}


Еще Пример из моего проекта

/// <summary>
    /// Построитель контуров, т.е. массивов X и Y,
    /// образованных виртуальной нарезкой больших массивов;
    /// Используется для создания всех контуров одного графического элемента
    /// ---
    /// С возможностью заранее не знать требуемый размер, и добавлять столько элементов контура, сколько нужно,
    /// в процессе интерполяции (иначе там был бы нужен еще один временный буфер),
    /// но только до момента финализации контура - после этого становится известен конец текущего массива
    /// и начало следующего - с этого момента добавление в первый контур запрещено,
    /// и общий массив используется уже для добавления в следующий контур;
    /// Это ограничение использования позволяет достичь не-фиксированных массивов и использовать память эффективней
    /// ---
    /// Оптимизация List[Vector2], для уменьшения GC, для использования в пуле (ThreadLocal)
    /// ---
    /// Отличается от BuilderOfSlicedArrayVector2 возможностью не знать размер заранее,
    /// поэтому полученные Handle другого типа (не HandleOfSlicedArrayVector2)
    /// </summary>
	public class BuilderOfContours
	{
        /// <summary>
        /// Общие массивы
        /// </summary>
        private readonly float[] _totalX = new float[CollectionsConfig.VerticesLimitInGraphicElement];
        private readonly float[] _totalY = new float[CollectionsConfig.VerticesLimitInGraphicElement];

        /// <summary>
        /// Аллокатор для HandleOfContour
        /// (в этом классе т.к. сам Builder управляет созданием и освобождением Handle)
        /// </summary>
        private readonly PooledAllocator<HandleOfContour> _handleOfContourAllocator = new PooledAllocator<HandleOfContour>(() => new HandleOfContour());

        /// <summary>
        /// Список выделенных handle для освобождения,
        /// и он же - список контуров для возврата вовне
        /// </summary>
        private readonly List<HandleOfContour> _contours = new List<HandleOfContour>(CollectionsConfig.ContoursLimitInGraphicElement);

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


Во втором же примере комментарии в основном:


  • необоснованно ограничивают область применения кода ("Используется для создания всех контуров одного графического элемента" — а что если кто-то захочет этот код переиспользовать не для графического элемента, а для чего-то ещё? Расстрел на месте? Позорная грамота?),
  • описывают какие именно от конкретной реализации ожидания ("С возможностью заранее не знать требуемый размер, и добавлять столько элементов контура, сколько нужно, в процессе интерполяции (иначе там был бы нужен еще один временный буфер), но только до момента финализации контура — после этого становится известен конец текущего массива, и начало следующего — с этого момента добавление в первый контур запрещено, и общий массив используется уже для добавления в следующий контур", "Оптимизация List[Vector2], для уменьшения GC, для использования в пуле (ThreadLocal)"). Если эти ожидания так важны, то хорошо бы было не комментарии писать, а добавить соответствующие тесты, например: "а правда ли оптимизация позволяет уменьшить GC?")?
  • Добавляют информацию, которую хорошо бы проверять ("Аллокатор для HandleOfContour (в этом классе т.к. сам Builder управляет созданием и освобождением Handle)" — а что, нельзя интерфейс класса сделать так, что бы эти Handle в принципе нельзя было удалить где-то вовне?)

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

Да тут что с комментариями, что без — ничего не понятно потому, что код вырван из контекста.

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

необоснованно ограничивают область применения кода («Используется для создания всех контуров одного графического элемента» — а что если кто-то захочет этот код переиспользовать не для графического элемента, а для чего-то ещё? Расстрел на месте? Позорная грамота?),

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

описывают какие именно от конкретной реализации ожидания…
Если эти ожидания так важны, то хорошо бы было не комментарии писать, а добавить соответствующие тесты, например: «а правда ли оптимизация позволяет уменьшить GC?»)?

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

Добавляют информацию, которую хорошо бы проверять («Аллокатор для HandleOfContour (в этом классе т.к. сам Builder управляет созданием и освобождением Handle)» — а что, нельзя интерфейс класса сделать так, что бы эти Handle в принципе нельзя было удалить где-то вовне?)

* во 1х это C# managed-код, и можно вообще не удалять ничего, и все отдастся для GC, но с проблемами и тут — оптимизация
* во 2х — интерфейс у этого класса есть — там есть и FreeAll для очистки всех объектов (а там показано в комменте что по-одному они не очищаются), и тут в комменте описано время жизни объекта: объект HandleOfContour не самодостаточный, он — прокси, а коллекция лежит внутри BuilderOfContours, и поэтому освобождать все должен — Builder

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

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

Хороший код, это как хорошая картина. Так и вижу заголовок: "Как нарисовать картину, чтобы зрители не матерились" :)

А как писать код на который через месяц я сам не буду ругаться?)
Никак. Просто, стиснув зубы, не матерясь, отрефакторить и успокоиться.
Важен сам факт появления комментария «здесь я бросил искать нормальное решение и оставил этот костыль, потому что...»
Так вот, вот это «потому что» — уже не важно. Зрелый программист отлично знает, что зарывание костылей всегда будет вести к проблемам. Дело не в идеальном коде, дело в том, что если в конкретный момент времени не нашлось времени на то, чтобы сделать нормально, дальше лучше тоже, как правило, не бывает.
здесь я бросил искать нормальное решение и оставил этот костыль, потому что

«нормальное» это по каким критериям? если формально… вот есть требования (критерии), и адекватность решения — сложный вопрос.
для вас в том числе — приведу
ССЫЛКУ
на свой коммент (и далее см ветку)

по 1му-примеру моему — это «нормальное» решение для задачи Интеграции
во 2му-примеру моему — это «нормальное» решение для задачи Оптимизации
… комменты тут помогают, а решение НЕ-костыль

p.s. если рассматривать «средний случай использования комментариев неопытным\незрелым программистом, не разбирающимся как правильно и делающим костыль потому что не-знает\нет-времени» то соглашусь, что коммент там хуже, чем сразу-нормальное-решение-вместо-костыля, однако не находите что такой случай не равен всем-возможным-применениям-комментов-для-аргументации-решения?
Я могу привести еще примеры:
* было сравнено несколько либ, и выбрана конкретная, и в комменте написано «почему» (критерии — скорость, простота, фичи, баги)
* была собственная разработка\исследование, и несколько вариантов перебрали, решили что «лучше так, а не иначе» (иначе — не то что надо из: производительность, артефакты\баги, сложность кода)
Всегда не любил эту фразу «Хороший код не нуждается в комментариях»

Возможно это и так, но почему то большенство ее воспринимают так «Если я не пишу комментарии значит мой код хороший»

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

А ещё комментарии позволяют:
1) не разбираться в коде, даже если он красивый и легкий, а по одному предложению понять для чего этот метод нужен и просто его использовать
2) понять что должен делать метод, потому что метод может работать не правильно(вспомним школьников). А как по коду понять что бизнес логика не правильная?
Так что гуру пишите пожалуйста комменты, что бы школьники повторяли за вами
Ценность кода гуру на порядки выше чем у кода школьников, поэтому нельзя поганить первое в угоду второму. Ну и если школьники таки читают хороший код, но все равно пишут плохой, то почему комментарии должны что-то менять? Выходит что они просто не обучаюся при чтении кода.

Хорошее название метода позволяет то же самое :) А если такое название метода это символов 100, то, скорее всего, не в отсутствии комментариев проблема непонимания того, что он делает.

Зарегистрируйтесь на Хабре, чтобы оставить комментарий

Публикации

Истории