Чек-листы для проведения Code Review

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

Введение

При внедрении Code Review, о котором я рассказывал ранее, мы за основу брали стандарт программирования, разработанного фирмой 1С. Для того чтобы с ним ознакомиться, необходимо прочитать соответствующий раздел на ИТС «Система стандартов и методик разработки конфигурация для платформы 1С:Предприятие 8» (https://its.1c.ru/db/v8std ). Обязательными к использованию являются все пункты. Это стандарт фирмы 1С. Все пункты важны, но мы выделили основные пункты и добавили свои пункты, все это переложили в чек-листы  для проведения Code Review (проверка кода), и это стал наш инструмент контроля соответствиям стандарта.

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

По каждому пункту чек-листов дается краткое пояснение. В своей деятельности Вы можете использовать предложенные чек-листы полностью или выделить из них нужную информацию даже без внедренной полноценной системы Code Review.

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

Чек-лист для проведения Code Review

1. Стандарт комментариев.
Более подробно описано в отдельном разделе.

2. Форматирование (Заглавные буквы, пробелы и т.д.).
Код должен легко читаться, и написан в одном стиле.

3. Название объектов (переменные, модули, функции и т.д.). Грамматические ошибки.
Код должен читаться без пояснений и не должен содержать грамматические ошибки.

4. Архитектура решения доработки.
Доработка не должна противоречить общей архитектуре системы.

5. Избыточность доработок.
Можно решить более простым методом; избыточные соединения в запросе и т.д.

6. Дублирование кода (ООП).
Общие процедуры и функции нужно выносить в общие модули.

7. Проверять деление на «0», проверка на Неопределено.
Добавлять проверки на данные значения, чтобы избежать красных ошибок.

Например:



8. Проверка на тип (где это необходимо).
Если одна переменная может быть, как объектом, там и структурой.

9. Обращение через точку. 
Избегать обращение через точку. Для объектов использовать стандартные функции БСП: (ОбщегоНазначения.ЗначениеРеквизитаОбъекта и т.д.).

Например:
Не правильно



Правильно:

10. Использовать Isnull в запросе.
При левом соединение в запросе, обязательно использовать Isnull.
Например:
Не правильно:



Правильно:



11. Не использовать запрос в цикле (кроме исключений).
По возможности выносить запросы из цикла.
Например:
Не правильно:



Правильно:



12. Использовать «Выразить» для реквизитов составного типа в запросе.
Разыменование ссылочных полей в запросе. Если реквизит в запросе имеет составной тип, то необходимо использовать функцию «Выразить», иначе будет неявные соединения со всеми типами реквизита.
Например:
Не правильно:



Правильно:



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



Правильно:



14.  Использование условий в запросе в секции ГДЕ, вместо параметров виртуальной таблицы.
Данные пункт значительно влияет на производительность. Необходимо отборы накладывать на параметры виртуальной таблицы, а не в секции ГДЕ запроса.
Например:
Не правильно:



Правильно:



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



Правильно:



16. Выгрузка результата запроса, где нет необходимости.
Задействуется больше ресурсов сервера.

17Разделить выполнение и выборку/выгрузку  результата запроса.
Правильно разделять выполнение и выборку/выгрузку результата запроса
Например:
Не правильно:


Правильно:


17. Избегать Полное (Внешнее) соединение в запросе.
Данное  ограничение накладывается СУБД PostgreSQL. Вместо Полного(Внешнего) соединения, нужно использовать секцию: Объединить.
Например:
Не правильно:



Правильно:


18. Не использование БСП (пишем то, что уже есть)
Изучаем БСП, используем уже готовые функции/процедуры БСП.
(например: ОбщегоНазначения.ЗначениеРеквизитаОбъекта; ОбщегоНазначения. ОписаниеТипаЧисло();ОбщегоНазначения.СообщитьПользователю()).

19. Не использовать метод «Сообщить».
Вместо метода «Сообщить» нужно использовать «СообщитьПользователю».

20. Использовать ТекущаяДатаСеанса() на &Сервере.
Нельзя использовать ТекущаяДата() на &Сервере, т.к. она может отличаться от текущий даты сеанса.

21. Не верные директивы компиляции
Например: обращаемся к серверной процедуре, но не используем «Объект» (использование на &НаСервере вместо &НаСервереБезКонтекста).
Например:
Не правильно:



Правильно:


22. Использовать инструкции препроцессору в модулях (Объекта, Менеджера, Сеанса).
Инструкции препроцессору: Сервер, ТолстыйКлиентОбычноеПриложение, ВнешнееСоединение и т.д.
Если их не использовать, то можно получить красные ошибки при запуске приложения, например в веб-клиенте.
Например:



23. Присутствует «мертвый» код, пустой обработчик.
Не должно быть закомментированного кода и пустых обработчиков.

24. Использовать стандартные области в модулях формы, объекта, менеджера, а также в общих модулях.
Легче читать и группировать код, например область: #Доработки.
Например:

25. Новые объекты должны быть обязательно включены в специальную подсистему «ДобавленныеОбъекты» и на них должны быть даны права.
Новые объекты должны быть включены в отдельную новую подсистему и на них должны быть даны права. Это позволяет легче ориентироваться в добавленных объектах.

26. Программное создание всех реквизитов формы, если дорабатываем типовую форму.
В таком случае легче проводить обновление. В новых конфигурациях все доработки форм можно выносить в один модуль: СобытияФормИС. ПриСозданииНаСервере, СобытияФормИС. ПриЧтенииНаСервере, СобытияФормИС. ПослеЗаписиНаСервере.

27. Не использовать проверку на тип «булево», через ИСТИНА или ЛОЖЬ.
Например:
Не правильно:



Правильно:



28. Использовать дату «регистра накопления – остатки», как «МоментВремени» (или «Граница»), а не «Дата».
Иначе может быть получены неверные данные.
Например:
Не правильно:



Правильно:



29.  Правильное использование: «транзакций» и «попыток».
Избегать вложенных попыток и транзакций.
Не должна возникать ошибка: В данной транзакции уже происходили ошибки.
Например:
Не правильно:



Правильно:

    


Например:
Не правильно:


Правильно:

    

29. Учитывать RLS при написании запросов.
При включении RLS должны работать все запросы.

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

Формат комментариев и префиксация новых объектов

Формат комментария должен иметь формат, представленный на рисунке 1: 


Рисунок 1 Формат комментариев.

Ниже приведено описание каждого элемента комментария (номер элемента на картинке соответствует номеру, выделенному жирным, в описании ниже). «Пример 1» для проектных работ, «Пример 2» для работ на сопровождении.

 // <Тип комментария> <Наименование организации> <ФИО разработчика> (<Дата внесения изменения в формате "dd.MM.yyyy">) <П|С>.<Номер> <Краткое пояснение изменения>

1 <Тип комментария> - указывается символ «+» для комментариев перед изменением, «-» для комментариев после изменения.

2 <Наименование организации> - наименование организации разработчика для его идентификации. Не более десяти символов.

3 <ФИО разработчика> - фамилия и инициалы разработчика, который вносит изменения в код. Формат: «Фамилия И.О.».

4 <Дата внесения изменения в формате "dd.MM.yyyy"> - следует обязательно придерживаться заданного формата даты. Дата изменения указывается в скобках.

5 <П|С> - если изменение вносится в рамках проекта, то следует указывать «П»; если в рамках сопровождения, то следует указывать «С». <Номер доработки в рамках проекта или сопровождения> - порядковый номер доработки, по которому можно однозначно найти постановку задачи по изменению. Для проектных работ .<Префикс проекта>.<Номер раздела | Номер технического задания>.<Номер доработки>. Для задач на сопровождение .<Номер доработки>.

6 <Краткое описание изменения> - необязательное поле. Указывается, если разработчик считает необходимым описать изменение кода. Не более 50 символов.

Создание новых объектов и префиксация

Первое правило – это добавление в конфигурацию объектов верхнего уровня (справочников, документов, регистров и т.д.). В начало наименования объекта нужно добавлять префикс (рисунок 2).


Рисунок 2 Создание нового объекта верхнего уровня.

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

  • Во-первых, это визуально выделяет добавленный элемент в конфигурации и в коде сразу видно, что это – наш добавленный объект.
  • Во-вторых, достигается уникальность имени, потому что поставщик может добавить свой объект с тем же именем. И если мы будем использовать свой префикс, то это гарантирует, что наше имя будет уникально. Синоним указываем без префикса. Синоним не должен значительно отличаться от имени объекта. К примеру, если имя объекта «Префикс_КадроваяИсторияСотрудников», то неверно указывать в качестве синонима «Работники организации».

Второе правило: комментарии ко всем добавленным объектам должны содержать специальную метку с именем разработчика и номером задачи. Эта метка аналогична открывающему комментарию из модуля, только без специальных символов – с ее помощью всегда можно понять, кто, когда и по какой задаче добавил этот объект.

Третье правило: новые объекты должны быть обязательно включены в специальную подсистему «Префикс_ДобавленныеОбъекты». Если такая подсистема отсутствует на момент начала добавления новых объектов конфигурации – ее необходимо создать. Включать в командный интерфейс не обязательно. Данная подсистема нужна для определения объектов конфигурации, разработанных нами, а также для возможного переноса объектов по подсистеме в иные конфигурации.

Первое правило распространяется и на реквизиты объектов, регистров, форм и названия элементов форм, если такие реквизиты добавляются в типовые объекты. За тем исключением, что в реквизитах форм можно указать только префикс в имени.

К примеру: Типовой справочник «Пользователи». В нем необходимо добавить реквизит, который бы содержал, действителен или нет пользователь (рисунок 3). Формат комментария описан выше.  При добавлении реквизитов формы так же необходимо указывать префикс. 


Рисунок 3 Добавление реквизита в типовой объект.

Чек-листы для проверки архитектурных решений кода

1. Использовать только в крайних случаях объект метаданных - “Константы”. 
Одна константа со временем может принимать несколько значений, в зависимости от разных сущностей. 
Правильное решение - это использовать регистр сведений в котором будет четыре измерения: “Аналитика” (составной тип) и одни ресурс: Значение (составной тип).

2. Поле на форме, которое подсвечено красной пунктирной линией, должно быть обязательно к заполнению.
Иначе у пользователей возникает непонимание по данному полю.
Например:




3. При добавлении нового объекта метаданных - “Команда”, “Отчет”, “Документ” и т.д.  не забывать про роли. 
Необходимо добавить новую роль или включить доступ объекта в одну или несколько  существующих ролей.

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

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



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

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

8. Проверка на тип переменной. 
Если переменная может принимать различные типы (например: структура или объект метаданных: “документ”), то перед обращением к реквизитам переменной нужно проверить тип. 
Например:
Не правильно:


Правильно:



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

10. Получение данных из периодического регистра сведений срезом последних не по всем измерениям регистра. 
В таком случае возможно получение дублированных записей (отличаются значениями неучтенных измерений).

11. Фиксировать по возможности все важные события в журнал регистрации (например: отправка почты (кому и во сколько ушло письмо); обмены с другими системами (во сколько и какие объекты отравлены/загружены)  и т.д.). 
Это поможет при анализе спорных ситуаций с пользователями.
Например:



12. Отказ от использования функций: НайтиПоКоду, НайтиПоНаименованию.
Т.к. со временем код и наименование объекта может быть изменено.

13. Необходимые проверки в объектах метаданных нужно выносить в модуль объекта, по возможности исключить их из модуля формы.
Т.к. возможно программная запись (проведение) объекта.

14. Осуществлять проверку объектов метаданных на реквизиты “ЭтоГруппа” и “ОбменДанными” (наиболее актуально для новых объектов).
Т.к. для этих реквизитов возможно по-другому должен выполняться основной функционал (логика) объектов. 

15. Проверка на наличие на форме реквизитов при программном добавлении.
Данная проверка избавит от критической ошибки при повторном добавлении реквизита на форму.
Например:   



16. Формировать отчеты в фоновом режиме.
Чтобы пользователи могли работать при построении отчета.
Например:



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

18. В типовом решении при синхронизации данных между базами нет возможности  отследить какие объекты были выгружены/ загружены. 
Данная информация будет необходима при анализе интеграционных процессов. Необходимо разработать регистр сведений для хранения информации по отправке/ получению данных или фиксировать данную информацию в журнал регистрации.

19 .По возможности не создавать реквизиты с типом “ХранилищеЗначений” для объектов метаданных (справочников, документов) активно использующихся при реализации основной бизнес-логики.
Т.к. это может значительно может повлиять на производительность. Поэтому, например, вопрос хранения тех же фотографий лучше решать не в основном справочнике, а в отдельном подчиненном справочнике или регистре сведений.

20. При написании запросов желательно:
20.1. Использовать пакетные запросы и временные таблицы вместо вложенных запросов.
20.2. Для объединения нескольких таблиц используйте объединить, а не левое соединение.
20.3. Для отладки сложных запросов используйте дамп запроса.
20.4. Индексировать  реквизиты/ устанавливать индекс для реквизитов, по которым используется отбор, поиск, соединение.

Заключение


Использование приведенных чек-листов (полностью или частично) повысит качество кода программистов, даже без внедренной системы Code Review. Основное правило, чтобы вся команда придерживалась одних стандартов.

Ретунский (2).png
Статью подготовил аналитик-эксперт по информационным системам франчайзинговой сети «ИнфоСофт» Ретунский Александр.
Статья опубликована на портале  ИнфоСтарт