Хабрахабр

О линтерах, качестве кода, качестве вообще и управлении качеством

Бойтесь своих желаний, они могут исполниться.
Народная мудрость.

Я взорвал их машину у церкви сразу после венчания.
One Wish Grant, фильм Трасса 60. Одна пара пожелала пожениться и обрести вечное счастье.

image

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

Очень абстрактная часть о качестве

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

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

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

Мы проводим исследование и узнаём что запах возле наших прилавков не нравится посетителям. Представьте что у нас есть магазин фруктов и есть проблема, наши фрукты стали хуже продаваться, а у конкурента очереди прям. О мы нашли проблему, индекс удовлетворённости поксетителя запахом! А запах у лавок конкурента нравится. Сделали. Давайте её решим, есть же аромамаркетинг, просто поставим автоматические установки возле прилавков и получим прекрасный запах яблоневого сада. Только вот покупателей теперь ещё меньше. И индекс удовлетворённости покупателя запахом закономерно запахом попёр вверх.

Если взглянуть серьёзно то изначальная проблема могла быть совершенно разной:

  1. Наши конкуренты продают столь же качественные фрукты, но сделали аромамаркетинг раньше нас и привлекли посетителей именно запахом.
  2. Наши фрукты хороши, но фрукты конкурента действительно лучше чем наши (сорта, хранение, что угодно)
  3. Наши фрукты протухли. Они просто сгнили и воняют.
  4. Протухли фрукты с прошлого года которые стоят за витриной, а мы надеемся что ещё сможем их продать. А они оттуда воняют.
  5. У конкурента больше ассортимент
  6. Конкурент более красиво разложил свои фрукты, в целом такие-же как у нас.
  7. Там тупо дешевле
  8. Там продавец красивая, а у нас баба Маня вышла на подмену...

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

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

Ещё более гиперболизировано

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

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

Если что-то плохо пахнет, духи не помогут. Ну вы поняли.

В меру абстрактная часть о качестве кода

Среди свойств хорошего кода мы найдём (не сортируя по важности):

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

Это достигается в первую очередь при помощи самодисциплины и уровня квалификации разработчиков, а также, когда разработчиков много, при помощи соглашей об оформелнии кода(code style) и соглашений об архитектуре (MVC, MVVM, ECS, тысячи их). Качественный код появился гораздо раньше чем линтеры, какие либо соглашения и архитектурные паттерны.
Большинство правил линтеров являются чисто косметическими и решают задачу повышения читаемости кода за счёт однообразного применения небольших и локальных практик. Длина строк там, наименования переменных, const везде где нет модификации, иногда даже вводятся ограничения на цикломатическую сложность функций. Речь сейчас не о конкретных правилах, а о том что эти правила вцелом косметические, они помогают коду выглядеть лучше. Ключевое слово тут выглядеть.

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

Наш недостаточно созрел. Теперь проведём аналогию с помидорами. Что сделает программист? Автоматический линтер скажет нам: «Смотри надо вот тут и тут не нужный цвет». И в этом кроется основная идея моей критики линтеров. Очень часто покрасит. Сейчас я приведу конкретный пример, а потом сделаем вывод.

Конкретика

PixiJS 2 февраля 2018 года (год назад).

Суть в том, что ранее для рисования кривой использовалось константное количество точек, что очевидно не оптимально. Прилетает пул реквест. Алгоритм не rocket science, но точно не очевидный, опубликован в 2013 году и приведён со ссылкой на статью его автора (наблюдаются проблемы с https). Предложено использовать хитрый алгоритм для оценки длины кривой. Счастье что он вообще сохранился на личной страничке.

Там приведён код на С (16 строк):

float blen(v* p0, v* p1, v* p2)
{ v a,b; a.x = p0->x - 2*p1->x + p2->x; a.y = p0->y - 2*p1->y + p2->y; b.x = 2*p1->x - 2*p0->x; b.y = 2*p1->y - 2*p0->y; float A = 4*(a.x*a.x + a.y*a.y); float B = 4*(a.x*b.x + a.y*b.y); float C = b.x*b.x + b.y*b.y; float Sabc = 2*sqrt(A+B+C); float A_2 = sqrt(A); float A_32 = 2*A*A_2; float C_2 = 2*sqrt(C); float BA = B/A_2; return ( A_32*Sabc + A_2*B*(Sabc-C_2) + (4*C*A-B*B)*log( (2*A_2+BA+Sabc)/(BA+C_2) ) )/(4*A_32);
};

А в пул реквесте прислан следующий код (JS):

/** * Calculate length of quadratic curve * @see * for the detailed explanation of math behind this. * * @private * @param {number} fromX - x-coordinate of curve start point * @param {number} fromY - y-coordinate of curve start point * @param {number} cpX - x-coordinate of curve control point * @param {number} cpY - y-coordinate of curve control point * @param {number} toX - x-coordinate of curve end point * @param {number} toY - y-coordinate of curve end point * @return {number} Length of quadratic curve */ _quadraticCurveLength(fromX, fromY, cpX, cpY, toX, toY) { const ax = fromX - ((2.0 * cpX) + toX); const ay = fromY - ((2.0 * cpY) + toY); const bx = 2.0 * ((cpX - 2.0) * fromX); const by = 2.0 * ((cpY - 2.0) * fromY); const a = 4.0 * ((ax * ax) + (ay * ay)); const b = 4.0 * ((ax * bx) + (ay * by)); const c = (bx * bx) + (by * by); const s = 2.0 * Math.sqrt(a + b + c); const a2 = Math.sqrt(a); const a32 = 2.0 * a * a2; const c2 = 2.0 * Math.sqrt(c); const ba = b / a2; return ( (a32 * s) + (a2 * b * (s - c2)) + ( ((4.0 * c * a) - (b * b)) * Math.log(((2.0 * a2) + ba + s) / (ba + c2)) ) ) / (4.0 * a32); }

Код оформлен в полном соответствии с настройками линтера. Указаны описания всех параметров, ссылка на изначальный алгоритм, куча констов, в соответствии с требованием линтера «no-mixed-operators»: 1 расставлены скобочки. Даже для производительности апи сделано не объектным, а с отдельными параметрами, так действительно обычно лучше в JS.
Есть одна проблема. Этот код делает полную херню. (Попытка калькировать на русском выражение fucked up, которое вполне используется в западных публикациях для выражения степени проблемы и вроде как уместно).

Вот что сказал линтер глядя на этот код без скобок

c:\rep\pixi\pixi.js\src\core\graphics\Graphics.js
258:26 warning Unexpected mix of '-' and '*' no-mixed-operators
258:32 warning Unexpected mix of '-' and '*' no-mixed-operators
259:26 warning Unexpected mix of '-' and '*' no-mixed-operators
259:32 warning Unexpected mix of '-' and '*' no-mixed-operators
260:24 warning Unexpected mix of '*' and '-' no-mixed-operators
260:30 warning Unexpected mix of '*' and '-' no-mixed-operators
260:30 warning Unexpected mix of '-' and '*' no-mixed-operators
260:36 warning Unexpected mix of '-' and '*' no-mixed-operators
261:24 warning Unexpected mix of '*' and '-' no-mixed-operators
261:30 warning Unexpected mix of '*' and '-' no-mixed-operators
261:30 warning Unexpected mix of '-' and '*' no-mixed-operators
261:36 warning Unexpected mix of '-' and '*' no-mixed-operators

Возвращается огромная длина, и на неё выделяется много точек, хорошо, что там было ограничение сверху, по нему оно и работало. Раньше этот режим был отключен по дефолту, но потом включился для всех (из-за другого бага кстати). Фикс уже вмержили кстати. Я не связывался с автором коммита и не спрашивал его, почему он решил расставить скобки, но чувствую что он запустил линтер, файл конфигурации которого уже есть в PixiJS. Это линтер ему сказал, твой код плох, потому что в нём не хватает скобок, добавь скобки. Опция «no-mixed-operators» говорит что вы не имеете права написать

2*2+2*2

потому что это может привести к плохой читаемости. Эту опцию кто-то создал, потом кто-то включил в проект, что говорит о том, что многие люди считают её полезной.

Вывод

Я не хочу сказать что линтеры зло, но вот такое применение их я считаю злом. Мы (в смысле человечество) смогли автоматизировать обнаружение только малой части признаков хорошего кода, в основном это косметика типа расставленных скобок. Линтеры хороши как инструменты анализа качества кода, но как только мы возводим соблюдение требований линтера в рамки обязательного требования мы получаем это самое соблюдение требований. Ничего кроме соблюдения требований мы не приобретаем. Это как поставить камеру на конвер с помидорами и отправлять на покраску все которые недостаточно красные. Пока мы не давали разработчику инструмент оценки качества внешнего вида кода он мог прислать плохой код, и мы могли это увидеть. Теперь плохой код будет лучше замаскирован. Он будет мимикрировать под хороший, потому что все внешние признаки хорошего кода на нём есть. И мы потеряем линтер как инструмент оценки, ведь весь код соответствует. У нас был инструмент оценки, а теперь его нет, зато код со скобочками, правда не там иногда, но это детали. Итого линтеры считаю классным инструментом, но только если соблюдение требований не становится целью.

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

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

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

Теги
Показать больше

Похожие статьи

Добавить комментарий

Ваш e-mail не будет опубликован. Обязательные поля помечены *

Кнопка «Наверх»
Закрыть