Управление качеством кода

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

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

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

Сплошные холивары.

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

Законы при разработке в 1С это "Система стандартов и методик разработки конфигураций для платформы 1С:Предприятие 8"

Негласные правила в определении качества: Когда программист смотрит на такой код - вырывается “Ах, что же это такое???”, только матерно. Сокращенно ЧБ. Если код постоянно вызывает ЧБ, то тут нужен стандарт, чтобы так больше не писали. Этот подход используется при добавлении диагностик в https://github.com/1c-syntax/bsl-language-server/issues

Уровни стандартов

Промышленные стандарты (из других языков). На них наткнетесь в интернетах и в книгах. Люто рекомендую прочитать Совершенный код и Чистый код
Стандарты 1С https://its.1c.ru/db/v8std. Доступ открыт и бесплатен.
Внутренние стандарты Компании. Сюда обычно входят правила префиксации. Правила доработки конфигурации на поддержке для упрощения обновления. Правила идентификации изменений - от любимых // Здесь был Уася по задаче 000666 до правила текста оформления коммита.
Личное. Я это называю почерком, когда автора кода легко определить по стилю, но при этом негатива это не вызывает. У меня это “дышащий стиль” с кучей пробелов и отступов, правилами именования, похожими на венгерскую нотацию и другими шаблонами.

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

Пример хорошего переопределения - префиксация имен у объектов при доработке конфигурации на поддержке. Стандарт 1С запрещает спец. префиксы. Но префиксы помогают при определении принадлежности объекта.

Пример плохого переопределения - идентификаторы изменений прям в коде с сохранением старого кода.

Почему стоит соблюдать стандарты.

Стандарт это ответ на решенную проблему

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

Все стандарты по запросам появились после анализа долгих запросов. Причем долгими в проде.

Экономия мыслетоплива

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

Удешевление рабочей силы

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

Унификация стиля

Соблюдение стандартов приводит код к единому стилю. А у единого стиля есть одна особенность - разработчик перестает различать где его код, а где коллеги. Разработчик одинаково хорошо начинает разбираться и в своем и в чужом коде. А это приводит к резкому уменьшению желания переписать все с нуля. Меньше энергии тратится на негатив, больше на креатив.

Приоритизация

Что разработчик берет в работу в первую очередь? Важное или срочное? Разработчик берет в первую очередь понятное! (ц) Максим Дорофеев.

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

Сопровождаемость

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

Ускорение разработки

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

Абстрактный придуманный пример в работе двух команд. Быстрой - которые фигачат код, игнорируя тех. долг, и качественной - у которой качество превыше всего.

	Быстрая команда		Качественная команда
Неделя	Задач	Комментарий	Задач	Комментарий
1	20	Ура! Новый проект. Фигачим.	2	Настраиваем окружение, разворачиваем инфраструктуру.
2	20	Все еще фигачим.	2	Продолжаем настраивать, обучаем использованию инструментария
3	18	После выката на прод кое-что отвалилось. Правили.	10	Начали что-то делать. Идет с трудом. Пишем тесты.
4	16	Очередное обновление и очередные проблемы в проде.	15	В проде были проблемы. Писали тесты, чтоб больше не повторилось.
5	15	Один разработчик на дежурстве постоянно мониторит и правит ошибки на проде.	18	Фигачим, вроде втянулись.
6	10	На этот раз прод упал очень сильно. Чинили 2 дня всей командой.	20	Успеваем решить все запланированные задачи и начали писать подробную документацию. Хотя нет, вон статья интересная про сбор ТЖ в ELK, надо попробовать у нас.
7	12	Делать задачи все сложнее. Уже мало кто помнит и понимает почему сделано так.	20
8	9	Уволился один из разработчиков.	22	Двоих застукали за опенсорсом в рабочее время, надо с ними что-то сделать.
9	7	Нашли замену. Тратим больше времени на объяснение, чем на разработку.	23	Начали появлятся задачи на доработки старого функционала. Легко и быстро их правим. Придумали что сделать с опенсорсерами. Отправили их на конференцию за обменом опытом.
10	5	Неделя чистоты. Нового ничего старались не делать - только чинили ошибки.	24	Привлекли студентов. Они пилят функционал в песочнице. Когда все тесты проходят и исправлены все ошибки от анализаторов - только тогда рассматриваем их на включение.
11	10	Много замечаний по выполненным задачам	20	Поймали критические ошибки до выкатки в прод. Поправили. Вроде все хорошо.
12	4	Опять упал прод.	25	Выполнили все задачи за 3 дня. Получили разрешение на разработку внутренних проектов.
13	0	Настолько все фигово, что нужно переписать все с нуля. При попытке реализовать новую задачу что-то обязательно отваливается из старого.	20	Скууучно. Статью написать может про управление качеством?

На этом абстрактном примере, где топлю за “качественную” команду и критикую “быструю” заметны интересные вещи:

Хоть “качественная” команда и сделала больше уже на 9 неделю - за первые 9 недель они сделали существенно меньше. В реальности этот срок зависит от множества факторов и может быть как 4 недели, так и 4 года. Ускорение при качественной разработке - игра в долгую.
Первые 2 недели “качественная” команда раскачивалась и вообще не выдавала задач. Время на раскачку в реальности так же может сильно отличаться. Если процессы известны, инструментарий знаком, команда уже сработана - на раскачку может уйти и 1 день. А если команда это видит в первый раз - тут может и месяца не хватит. Не каждый бизнес готов ждать подготовки к старту проекта больше месяца.
Если у вас не больше 100 задач, какую команду выберите? А у какой команды ФОТ выше?
Регрессионная спираль смерти - когда невозможно делать новый функционал, т.к. все силы уходят на правку ошибок в существующем, и каждое исправление привносит новые ошибки - это не миф. Я это видел.
То, что команда делает больше задач за неделю, чем на проекте с нуля - тоже чистая правда - при настроенной инфраструктуре, хорошем покрытии тестами и проверках не сложно изменить даже ядро системы. А чаще выходит, что новые хотелки решаются 3мя строчками кода.

Будем считать, что следование стандартам прямо влияет на качество кода. И следовать стандартам это правильно.

Как проверять качество

Самоконтроль

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

Двойное чтение кода

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

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

Тесты

При разработке с использованием тестов код сам собой меняется. Становится “тестопригодным” и это благополучно сказывается на качестве.

Тесты и нужны для проверки качества решения.

Автоматические проверки

1С:Автоматизированная проверка конфигурации. Умеет проверять на соответствие кучу стандартов, но делает это долго, а результаты смотреть не удобно

1С:EDT. Встроенная проверка ищет потенциальные ошибки без привязки к стандартам. Проверяет типы, использование переменных и методов. Но воспользоваться этой проверкой можно только в EDT.

Платный плагин для SonarQube. Из плюсов - может почти все. Из минусов - стоит денег.

Опенсорсный плагин для SonarQube. А точнее BSL-LS, который умеет отдавать найденные предупреждения в SonarQube, VSC. Проверок пока немного, но он быстрый и быстроразвиваемый. (Да-да, и эта статья преследует цель привлечь побольше народу к участию в разработке - кодом, идеями, популяризацией)

За счет наработок разной степени костыльности результаты проверок АПК, EDT и BSL-LS можно получить в одном едином, красивом и специально предназначенном для этого месте - в SonarQube.

Как статические анализаторы кода улучшают жизнь.

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

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

Ловят такие нарушения стандартов, которые программист еще не знает. Незнание стандартов не освобождает от ответственности. Стандарты могли изменится, могли появится новые. Помнить актуальную версию невозможно. Робот может.

Беспристрастность. Находит нарушение - показывает. Никаких излишних придирок или снисхождения.

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

Писать код хорошо - хорошо, а статистические анализаторы упрощают и улучшают этот процесс. Бери и делай!

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

Кому стоит управлять качеством.

Алистэр Коуберн в статье “Каждому проекту своя методология“ привел матрицу сложности проекта.

По вертикали - критичность проекта. Что можно потерять в случае ошибок. Пользователи потеряют в комфорте (Comfort - C), немного денег (Discretionary money - D), много денег (Essential money) или жизнь (Life - L).

По горизонтали - размер команды. Больше людей - больше сложных связей.

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

Жизнь (L)	L3	L10	L30	L80	L150	L300	L600
Много денег (E)	E3	E10	E30	E80	E150	E300	E600
Немного денег (D)	D3	D10	D30	D80	D150	D300	D600
Комфорта (C)	C3	C10	C30	C80	C150	C300	C600
	1-4	6-20	20-40	50-100	100-200	200-500	500+

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

Желтые проекты D3 и C10 - управлять качеством не помешает, но профит будет минимальный. Проводите двойное чтение кода, гонять анализаторы, экспериментировать с тестами. Это обычно внутренние базы компании - думаю сюда входят 80% фикси.

Оранжевые проекты- разработка небольших коробочных продуктов, автоматизация средних предприятий. Управлять качеством уже рекомендуется. Затраты на это окупаются. Без автоматизированных инструментов можно прожить, но это уже вопрос эффективности команды. Здесь уже стоит разворачивать то, что называют “Промышленными стандартами разработки 1С” - тесты, покрытия, сонар, пайплайны, девопс, гиты.

Красные проекты - разработка больших коробочных продуктов, автоматизация крупных предприятий. Здесь нужны полноценные и серьёзные процессы. Без управление качеством тут нельзя. Обязательные автоматические контроли, тесты, покрытие близкое к 100%, код прочитан дважды, в том числе безопасниками. Тут все серьезно.

Адовые проекты - не уверен, что такие есть на 1С. Если у вас такой проект - напишите как вы там вообще живете.

Советы по отслеживанию качества

Проверяются только модули со снятыми замками

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

Важно не решить все проблемы, а не вносить новые

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

Правило бойскаута

Отдельно выделю - Оставляйте код чище, чем застали. Сделайте чуточку лучше тот код, который трогаете при кодировании текущей задачи.

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

Часто тех.долг хочется отметить пропуском. Ошибка в коде остается со всеми негативными свойствами. Но теперь найти ее становится еще сложнее.

Решить все проблемы не получится

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

Примеры проверок

Некоторые из них выстрелили. Причем в ногу и больно.

Синтаксическая конструкция вида "Если...Тогда...ИначеЕсли..." должна содержать ветвь "Иначе"

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

ТекущаяДата()

Сравнительно недавно появился стандарт на запрет использование метода ТекущаяДата() , а может и давно и я о нем не знал. Впервые о запрете узнал, когда это подсветил сонар (через импорт из АПК). Еще подумал “за 10 лет кодинга ни разу не выстрелило в ногу, фигня, а не стандарт”.

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

Превышена длина строки

В стандарте указано, что не рекомендуется превышать 120 символов в строке. На проектах, что проверял на качество - это лидирующая по количеству срабатываний проверка. Каждый переопределяет допустимое количество символов под себя. Кто-то выставляет 180, кто-то 200. Я предпочитаю 140.

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

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

Орфография

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

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

А еще мы огребли от “Палета”и “Паллета”. Однозначно правильного варианта написания нет, как подсказывает гугл. Но в коде путаница приводила к реальным ошибкам. Решили, что раз проверка орфографии ругается на “Палета” и не ругается, на “Паллета”, то правильно так, как не ругается. Соответственно на остальные места будет подсветка. Добавлять в исключения запретили.

Отсутствует комментарий к экспортной процедуре (функции)

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

Ну и момент документирования тут есть. Все любят хорошую документацию. Писать ее, правда, никто не любит.

В модуле должны быть определены стандартные области

Также к ошибкам не приводит. Но повышает удобство разработки. Не нужно думать где размещать те или иные методы. Уже все придумано. При поиске нужного события - так же понятно где искать.

С развитием кодогенерации стандартные области будут еще более полезны.

Установлено право "Интерактивное удаление"

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

Использование устаревшей процедуры

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

Недопустимый вызов служебной процедуры или функции другой подсистемы

А это уже об использовании приватных методов. Если публичные методы сперва делают устаревшими, чтобы было время на исправление. То приватные методы могут изменится в любой момент. Ну нет в 1С возможности сделать приватный метод на подсистему. Используете чужие приватные методы? Рискуете, что что-то отвалится в неожиданный момент.

Выводы

Определитесь, нужно ли качество на проекте. Точнее качество то точно нужно, но готовы ли за него заплатить временем или деньгами? “Технология очень быстрого результата” (она же фигакфигак и в продакшен, не путать с “Технологией Быстрого Результата”) не так уж и плоха.
Правильнее на управление качеством (Continuous Code Inspection если по заумному) смотреть как на инструмент. Инструмент применяется под конкретные задачи, процессы, методологии.
С течением времени порог на управление качеством будет снижаться. Верю, что наступит время, когда это будет обязательным инструментов при разработке с низкой ценой использования.
Даже если нет нужных проектов, или там это очень сложно развернуть - инвестирование в знания окупятся. При смене платформы, проекта, работы.

-----------------------------------

Как реально управлять качеством на реальных проектах в реале расскажет Олег на INFOSTART EVENT 2019.

SonarQube BSL АПК acc качество cci

+183 –

См. также

"Чистый код в 1С" или как прокачать свой код? Пошаговая инструкция, часть №1

Рефакторинг и качество кода Программист Стажер Платформа 1С v8.3 Конфигурации 1cv8 Бесплатно (free)

В последнее время термин «чистый код» стал очень популярным. Появились даже курсы по данной тематике. Так что же это такое?

16.09.2024 13953 markbraer 64

Обратная связь, атомарность и масштабируемость

Рефакторинг и качество кода Программист Бесплатно (free)

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

10.09.2024 916 acces969 4

Быстрый фронт в базе размером 8.8 терабайт – наши стандарты при разработке компонентов системы

Рефакторинг и качество кода Бесплатно (free)

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

28.08.2024 1162 Chernazem 3

Использование принципов SOLID в разработке на 1С

Рефакторинг и качество кода Программист Бесплатно (free)

SOLID – принципы проектирования программных структур (модулей). Акроним S.O.L.I.D. образован из первой буквы пяти принципов. Эти принципы делают код более гибким, упрощают разработку. Принято считать, что принципы SOLID применимы только в объектно-ориентированном программировании. Но их можно успешно использовать и в 1С. Расскажем о том, как разобраться в принципах SOLID и начать применять их при работе в 1С.

22.08.2024 10193 alex_sayan 41

Паттерн "Стратегия". Пример реализации

Рефакторинг и качество кода Программист Стажер Платформа 1С v8.3 Конфигурации 1cv8 Бесплатно (free)

Рассмотрим основные принципы шаблона проектирования "Стратегия" на простом примере.

25.06.2024 4196 MadRave 34

Пример проведения Code-review #1

Рефакторинг и качество кода Программист Платформа 1С v8.3 Абонемент ($m)

В статье расскажу и покажу процесс проведения Code-review на примере обработки с GitHub.

1 стартмани

04.06.2024 6275 mrXoxot 55

Результаты ревью кода 1500+ решений каталога Инфостарт: наиболее частые ошибки разработчиков в коде

Рефакторинг и качество кода Платформа 1С v8.3 Бесплатно (free)

Поделюсь своим опытом аудита кода авторских продуктов с Infostart.ru как одним из элементов применения DevOps-практик внутри Инфостарт. Будет настоящий код, боевые скриншоты, внутренние мемы от команды ИТ-лаборатории Инфостарт и прочее мясо – все, что любят разработчики.

10.04.2024 13347 artbear 85

108

Ниндзя-код

Рефакторинг и качество кода Программист Платформа 1С v8.3 Россия Бесплатно (free)

Предлагаю вашему вниманию советы мастеров древности. Программисты прошлого использовали их, чтобы заострить разум тех, кто после них будет поддерживать код. Гуру разработки при найме старательно ищут их применение в тестовых заданиях. Новички иногда используют их ещё лучше, чем матёрые ниндзя. Прочитайте их и решите, кто вы: ниндзя, новичок или, может быть, гуру? (Адаптация статьи "Ниндзя-код" из учебника JavaScript)

01.04.2024 4234 DrAku1a 15

Комментарии

Подписаться на ответы Инфостарт бот

Свернуть все

1. Evil Beaver 8243 22.07.19 11:26 Сейчас в теме

Охрененно! Аффтар, пешы есчо, хорошо же получается!

Ответить

2. mrXoxot 3064 22.07.19 11:34 Сейчас в теме

Очень здорово!
Пусть все получиться!

3. artbear 1563 22.07.19 11:56 Сейчас в теме

(0) >Все любят хорошую документация

Опечатка.

4. artbear 1563 22.07.19 12:16 Сейчас в теме

(0) Отличная статья. Радует, что все больше разработчиков/команд обращают внимание на качество своей работы - статический анализ кода, автоматические тесты, CI, DevOps и вот это вот все.

5. artbear 1563 22.07.19 12:24 Сейчас в теме

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

Вот тут на самом деле непросто.
Да, такое решение упрощает жизнь, скорость анализа, количество замечаний и т.п.

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

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

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

Ответить

6. Scorpion4eg 434 22.07.19 14:43 Сейчас в теме

Жаль что никто кроме EDT напрямую с внешними обработками не работает.
АПК от слова совсем. Sonar более менее пережевывает... А ведь во внешних печатных формах можно столько всего на плохокодить.

7. olegtymko 914 22.07.19 14:45 Сейчас в теме

(6) Паковать обработки / отчеты в cf/cfe и прогонять в EDT, АПК и заливать все в SonarQube. Тогда весь плохокод будет на лицо.

9. artbear 1563 22.07.19 15:51 Сейчас в теме

(7) Костыль ИМХО. но ради качества на что не пойдешь :)

8. artbear 1563 22.07.19 15:50 Сейчас в теме

(6) Почему "никто" ? наш платный плагин для SonarQube (от Пули) прекрасно работает.

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

давно проверяю инструмент Ванесса-АДД, построенный как раз на внешних обработках.

10. theshadowco 253 22.07.19 16:20 Сейчас в теме

(8) токо в формате edt увы работает неахти

11. Scorpion4eg 434 22.07.19 16:42 Сейчас в теме

(8) Не спорю, классно работает! Но когда мы его тестировали была проблема с "Возможно неиспользуемая процедура" для обработчиков элементов формы. Решили уже?

12. Fox-trot 163 22.07.19 17:33 Сейчас в теме

(11) достаточно добавить операнд "Экспорт", хотя костыль конечно

13. artbear 1563 22.07.19 17:51 Сейчас в теме

(11) Да, эта проблема решена в конце прошлого года или начале этого

14. vlad.frost 186 22.07.19 23:49 Сейчас в теме

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

SonarQube, кстати, позволяет этому правилу следовать с чуть меньшими рисками:
во-первых, можно заранее, перед выполнением задачи, посмотреть, какие есть в коде нарушения стандартов;
во-вторых, после всех правок убедиться, что все нарушения-таки исправлены.

15. user614660_dajigin 23.07.19 06:47 Сейчас в теме

Все таки есть сомнения по поводу "Иначе". Очень много конструкций таких типов:

Если КакойТоПризнак = ПервоеЗначение Тогда
ВыполнитьДополнительныеДействия1;
ИначеЕсли КакойТоПризнак = ВтороеЗначение Тогда
ВыполнитьДополнительныеДействия2;
КонецЕсли;

...Продолжаем выполнение по плану: действия, которые должны выполняться для ПервоеЗначение, ВтороеЗначение и все последующие....

Что в этом случае? Пустой "Иначе" делать? Какой в нем смысл? Исключения выкидывать нельзя, т.к. "непопадание" в условия это не ошибка.

16. Labotamy 23.07.19 06:52 Сейчас в теме

(15)В этих случаях диагностика ничего вам не скажет. Диагностика срабатывает на Если ИначеЕсли без Иначе.

17. user614660_dajigin 23.07.19 06:53 Сейчас в теме

Уже понял косяк)) Исправил вопрос))

18. Labotamy 23.07.19 06:58 Сейчас в теме

(17)Посмотрите комментарии к статье Олега. Там весёлый холиварчик на эту тему вышел.

19. user614660_dajigin 23.07.19 07:07 Сейчас в теме

(18) Ага, спасибо, нашел)

20. artbear 1563 23.07.19 12:05 Сейчас в теме

(15) Да, эта диагностика холиварна и дает много срабатываний.

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

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

Поэтому мы для своего платного плагина (от Пули) все еще думаем над необходимостью введения этого правила.

Создавать правило, которое, скорее всего, чаще будет отключено, не хочется.

21. olegtymko 914 24.07.19 03:17 Сейчас в теме

(20) Опять же это очень холиварная тема.
На стадии разработки эта проверка очень много нервов и жизней может спасти. Но и в проде поможет отловить определенную массу косяков.
К примеру:
Дан какой-нибудь произвольный обмен с партнером через FTP в их кастомном формате. В коде есть условие, которое определяет что делать с документом

ДД = Новый ДвоичныеДанные(); // какие-то двоичные данные
ВидФайла = "ARRAIV"; // вид файла, который прочитали из именя файла

Если ВидФайла = "PICK" Тогда
	ЗагрузитьКакРасходнуюНакладную(ДД);
ИначеЕсли ВидФайла = "ORDER" Тогда
	ЗагрузитьКакЗаказ(ДД);
ИначеЕсли Видфайла = "STOCK" Тогда
	ЗагрузитьКакРеестрОстатков(ДД);
КонецЕсли;

Показать

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

Если ВидФайла = "PICK" Тогда
	ЗагрузитьКакРасходнуюНакладную(ДД);
ИначеЕсли ВидФайла = "ORDER" Тогда
	ЗагрузитьКакЗаказ(ДД);
ИначеЕсли Видфайла = "STOCK" Тогда
	ЗагрузитьКакРеестрОстатков(ДД);
Иначе
	// или исключение (при условии что выше по уровню стоит попытка и отлавливает все исключения)
	ВызватьИсключение(СтрШаблон("Вид файла %1 не поддерживается"));
	// или соообщение в какой нибудь лог
	Логирование.Ошибка("Обмен.СамыйЛучшийПарнер", СтрШаблон("Вид файла %1 не поддерживается"));
КонецЕсли;

Показать

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

24. Scorpion4eg 434 03.11.19 14:25 Сейчас в теме

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

26. Ndochp 103 13.11.19 11:55 Сейчас в теме

(24) Ну вот смотри, у тебя есть что-то, что работает с десятком видов документов и в 8 в реквизите то, что тебе надо, в девятом префикс "ДД.", в десятом - суффикс ".Ик"
вот ты и пишешь:

Если ВидФайла = "PICK" Тогда
    ПараметрСтрокой = Сред(ПараметрСтрокой, 4);
ИначеЕсли ВидФайла = "ORDER" Тогда
    ПараметрСтрокой = Лев(ПараметрСтрокой, СТрДлина(ПараметрСтрокой) - 3);
КонецЕсли;

27. Scorpion4eg 434 13.11.19 15:14 Сейчас в теме

(26) Это понятно. Но что ты будешь делать, если ВНЕЗАПНО (а это всегда внезапно) придет новый ВидФайла? Сколько времени потребуется на поиск ошибки?

А так добавляешь строку:
Иначе
ВызватьИсключение "Неожиданное значение переменной ВидФайла" + ВидФайла;
КонецЕсли;

28. Ndochp 103 13.11.19 21:22 Сейчас в теме

(27)У меня для 8 видов все хорошо, для двух - проблемы. Скорее всего с одиннадцатым все будет тоже хорошо. А если не будет, то я об этом не забуду.
Или вы предлагаете сделать еще один блок с видами файла 1..8? А если я точно знаю, что у меня 2 исключения, но не уверен, сколько основных?
Типа уникального перемещения между складами - единственный документ с двумя складами. Вы знаете все документы в конфигурации с реквизитом "Склад"?

29. Labotamy 14.11.19 08:42 Сейчас в теме

(28) А все коллеги знают и помнят эти "нюансы"? И те кто будут после, тоже?

30. Ndochp 103 18.11.19 15:31 Сейчас в теме

А куда деваться? или предлагаете перед каждым использованием реквизита "Склад" в экспортных процедурах модулей складской подсистемы делать проверку на тип документа и выбрасывать исключение для новых типов?

22. kalimehtar 22 29.07.19 08:12 Сейчас в теме

(15)

Пустой "Иначе" делать? Какой в нем смысл?

Смысл в том, что ты явно указал любому читающему, что ветка Иначе пропущена не случайно. Также как в проверках Си "while (c = getchar()) " считается ошибкой, а "while ((c = getchar()))" нормальный код. Вторая пара скобок указывает, что присваивание там осознанно, а не перепутали со сравнением "==".

25. Ndochp 103 13.11.19 11:49 Сейчас в теме

(22) Только АФАИР влетаешь в диагностику " в блоке нет кода"

23. Tavalik 3409 29.07.19 16:36 Сейчас в теме

Крутая статья. Спасибо.

31. FrancuzbyGmailcom 29.11.19 11:28 Сейчас в теме

Кажется это "содрано" из книги "Совершенный код" ;)

32. Labotamy 29.11.19 15:02 Сейчас в теме

(31) Вероятней из "чистый".

33. Stepa86 1531 29.11.19 15:03 Сейчас в теме

(32) Про стандарты 1С у Макконнелла все же лучше написано

34. mikukrnet 182 16.04.21 14:56 Сейчас в теме

"Синтаксическая конструкция вида "Если...Тогда...ИначеЕсли..." должна содержать ветвь "Иначе""

А можно кейс, где это выстрелило?

35. olegtymko 914 19.04.21 08:47 Сейчас в теме

(34) При переходе на ставку НДС20

36. biimmap 2019 24.04.21 17:44 Сейчас в теме

Хорошая статья. тоже ЗА повышение качества кода.

Единственное с чем не соглашусь - это с комментированием старого кода. НА мой взгляд это обязательно нужно делать!
Потом при обновлении как понять, что там было? Почему так написали? Или даже просто требуется доработать то же самое место!

И вот насчет Если ... Иначе... Непонятно почему всегда должна быть ветка иначе? куча моментов когда часть кода выполняется только в определенных сценариях. Здесь неявное Иначе... Т.е. Иначе выполняем код написанный ниже.

Что касается автотестов... у меня обычно 2 кейса в работе:
1. Сотрудник соблюдает все стандарты и проверять там нечего, в том числе мой собственный код.
2. Сотрудник плевать хотел на стандарты, оно и так работает. Поэтому не понимаю зачем это надо за дверями коробочных решений.

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

37. kalyaka 1105 25.04.21 16:06 Сейчас в теме

(36)

Единственное с чем не соглашусь - это с комментированием старого кода. НА мой взгляд это обязательно нужно делать!

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

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

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

38. biimmap 2019 25.04.21 16:17 Сейчас в теме

(37) здесь вопрос в том, как его комментируют. Есть люди. которые комментируют процедуру целиком или запрос целиком и пишут новый.... Вот в этой ситуации это реально мусор!

Мой код, если меняю типовой выглядит точечно, пару строк старого, пару строк нового.

Рефакторинг делают обычно в самописных решениях. Или в тиражных! Здесь полностью согласен, не имеет значения что было написано раньше, для этого есть история хранилища.

Поэтому ещё раз сделаю акцент на доработке ТИПОВЫХ модулей и объектов. Конкретно я занимаюсь ЗУПом. Всё что НЕ типовое, никаких комментариев быть не должно, здесь согласен. Но в типовых без знания того "как было до" будет невозможно выполнить обновление, не вникая в суть задачи!

39. kalyaka 1105 25.04.21 20:34 Сейчас в теме

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

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

40. biimmap 2019 25.04.21 22:41 Сейчас в теме

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

Расширения это срань полная! Вот кто их придумал, тот пусть и использует. И вот в них как раз отсутствует возможность сравнить с чем-нибудь...

Оставьте свое сообщение

E-mail:

Автор:

Антон Степанов (Stepa86)

Рейтинг: 1531

Для получения уведомлений о новых публикациях автора подключите телеграм бот: Инфостарт бот

Публикация:

№ 1096770

Создание 22.07.19 13:49

Обновление 22.07.19 13:49

Статистика:

Просмотры 25893

Загрузки 0

Рейтинг 183

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

Характеристики:

Код открыт Да

Рубрики Рефакторинг и качество кода

Кому Программист ,
Руководитель проекта

Тип файла Нет файла

Платформа Платформа 1С v8.3

Конфигурация Не имеет значения

Операционная система Не имеет значения

Страна Не имеет значения

Отрасль Не имеет значения

Налоги Не имеет значения

Вид учета Не имеет значения

Доступ к файлу Бесплатно (free)