Антипаттерны программирования в 1С

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

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

"Любой дурак может написать код, понятный компьютеру. Хороший программист пишет код, понятный человеку." - Мартин Фаулер.

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

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

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

Процедура Док_МСТ_для_БЛМ()

//...

КонецПроцедуры

Функция Преобр_НСТ_в_СПТР()

// ...

КонецФункции

Или так:

Процедура Док_МСТ_для_БЛМ(Отказ)

   Массив = Преобр_НСТ_в_СПТР();
   
   Для каждого элемент из Массив Цикл
       Если ПР_На_ВЗ_и_КВД(элемент) Тогда
            Сооб_ПИОВД(элемент, Отказ);
       Иначе
            Обр_ДляСТР(элемент);
       КонецЕсли;
   КонецЦикла;

КонецПроцедуры

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

Запомните! Ни в коем случае не пользуемся встроенным в конфигуратор или EDT функционалом "Рефакторинг" и не добавляем описание функции. Помните, что не стоит кликать кнопкой мыши на функции, вызывать контекстное меню и выбирать команду "Создать описание функции".

2. Меняем произвольно регистр букв в названиях переменных и функций, а также меняем раскладку русский или английский

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

ПользовательXml, пользовательxml, пользовательXML, ПользовАтельXML 

3. Необоснованное сокращение названий переменных

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

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

Функция ПолучитьАП()
	Пользователи = Новый Массив;
	мСоединения=ПолучитьСоединенияИнформационнойБазы();
	Для каждого ЭлементМассива Из мСоединения Цикл
		Пользователь = ЭлементМассива.Пользователь; 
		Пользователи.Добавить(Пользователь);
	КонецЦикла;
	Возврат Пользователи;
КонецФункции

Функция ПолучитьПБ()
	
	Пользователи = Новый Массив;
	Выборка = ПользователиИнформационнойБазы.ПолучитьПользователей(); 
	Для Каждого ЭлементМассива Из Выборка Цикл 
		ИмяПользователя = ЭлементМассива.Имя; 
		Пользователи.Добавить(ИмяПользователя);
	КонецЦикла;
	Возврат Пользователи;	
КонецФункции

И далее где-то будет следующий код: 

//..
// много кода
//..
// где-то тут получаем данные о пользователях

Пользователи = ....

//..
// много кода
//..

Для каждого ЭлементМассива из Пользователи Цикл // ? что тут ищем вверху по коду
	//..	
КонецЦикла;

//..

4. Использование одной и той же переменной для всех действий в коде и присвоение им ничего не значащих имен

Это вносит небольшой элемент непредсказуемости, особенно когда у вас есть условия.

Для каждого тмп из ТаблицаТовары Цикл
    Если тмп.Цена=0 Тогда
        Сообщить("Внимание в таблице не заполнена цена!");
    КонецЕсли;
КонецЦикла;

Если ТаблицаТовары.Количество()=1 Тогда
    тмп = "Всего одна строка";
КонецЕсли;

Если НЕ тмп="" Тогда
     Сообщить(тмп);
КонецЕсли;

Или так:

Процедура ПрограммноеСозданиеТаблицы(Таблица)
    Для Каждого тмп из Таблица Цикл
       // создаем группу
       Перем1 = СозатьПрограммноЭлементФормы(тмп, Тип("ОбычнаяГруппаФормы"));
       // создаем поле надписи внутри группы родителя
       Элемент1 = СозатьПрограммноЭлементФормы(тмп, Тип("ПолеФормы"), Тип("ПолеНадписи"), Перем1);
       // создаем декораци внутри группы родителя
       Элемент2 = СозатьПрограммноЭлементФормы(тмп, Тип("ПолеДекорации"),,Перем1);
       Элемент2.Заголовок = "декорация";
       // создаем кнопку внутри группы родителя за декорацией
       Элемент =  СозатьПрограммноКомандуФормы(тмп,Перем1,Элемент2);
    КонецЦикла;
КонецПроцедуры

5. Нет форматирования. Разработчик никогда не использует авто форматирование, не знает про комбинацию "Shift+Alt+F"

Авто-форматирование кода убивает любую индивидуальность кода. Стремитесь к неповторимости. Так и только так можно стать великими разработчиками, которых будут помнить и всегда вспоминать "хорошим" словом коллеги программисты.

//...
МассивСтрок = Новый Массив;
Если ЗначениеЗаполнено(Организация) Тогда
Для Каждого тмп из ТаблицаТовары Цикл
    Если НЕ тмп.Организация=Организация Тогда
МассивСтрок.Добавить(Новый Структура("стр",тмп));
КонецЕсли;
    КонецЦикла;
КонецЕсли;
//...

6. Кодирование в одну строчку кода

Длина не важна, главное, чтобы текст помещался на экране и желательно в одну строчку. Кто-то будет отлаживать код? Зачем? Он же идеальный)

Попытка Объект.Записать(); тмп = истина; Исключение Сообщить(ОписаниеОшибки())  КонецПопытки;

Или так:

Если НЕ ЗначениеЗаполнено(Организация) Тогда Организация=ПолучитьОрганизациюПоУмолчанию(); КонецЕсли;

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

ТаблицаДанных = ПолучитьТаблицуДанных(ИсходнаяТаблицаУсловий, Параметры, Организация, Партнер, ИспользоватьДоговоры, Итина, СписокЗначений);

7. Ненужная усложненность, однострочные вложенные сокращения и оператор «?» вместо использования условий

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

Запрос = Новый Запрос(ПолучитьТекстЗапроса());
ИспользоватьКонтроль = ?(ЗначениеЗаполнено(Партнер),?(ЗначениеЗаполнено(Договор),?(Договор.Контроль=Истина,Истина,Ложь),Ложь),Истина)
Запрос.УстановитьПараметр("ИспользоватьКонтроль", ИспользоватьКонтроль);

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

 МассивСтрок = Новый Массив;

Для каждого стр из ТаблицаТовары Цикл

    Если стр.Количество<>0 Тогда
        // добавим в массив строки по условию...
        МассивСтрок.Добавить(Новый Структура("стр,ключ",стр, Новый Структура(Новый УникальныйИдентификатор()))));
    КонецЕсли;

КонецЦила;

8. Большое количество параметров в функции 

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

Не стоит использовать структуру для свертки параметров или использовать хранилище данных.

Функция ПолучитьДанныеПоЗадаче(МассивВходныхДанных, Организация, Контрагент, Соглашение=Неопределено, Договор=Неопределено, Валюта=Неопределено, ТипОтношений=Неопределено .... )
//...
// что-то делает
//...
КонецФункции

9. Использование вложенных вызовов большой глубины или слоеный пирог

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

//...
// вызываем для получения параметров функции, в которых также вызываем вложенные
// хорошо, что еще не в одну строчку записали)
РезультатОбработки = ПолучитьДанныеПоЗадаче(
          МассивВходныхДанных,
          ПолучитьОрганизациюИзДокумента(ДокСсылка, ПолучитьОрганизациюПоУмолчанию()),
          ПолучитьКонтрагентаИзДокумента(ДокСсылка),
          ПолучитьСоглашение(ДокСсылка,ПолучитьСоглашениеПоУмолчанию(ПолучитьОрганизациюПоУмолчанию())));
//...

10. Повсеместное дублирование кода 

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

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

11. Использование стиля "Рваный запрос" или "Рваная строка"

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

Функция ПолучитьРезультатОбработкиДанных(Ключ, Данные) Экспорт
	
	//...
	
	Запрос = Новый Запрос;
	// получаем данные из таблицы остатков товаров организаций
	Запрос.Текст = "ВЫБРАТЬ
	|	Т.АналитикаУчетаНоменклатуры КАК АналитикаУчетаНоменклатуры,
	|	Т.Организация КАК Организация,
	|	Т.ВидЗапасов КАК ВидЗапасов,
	|	Т.НомерГТД КАК НомерГТД,
	|	Т.КоличествоОстаток КАК КоличествоОстаток
	|ИЗ
	|	РегистрНакопления.ТоварыОрганизаций.Остатки( ";
	// если требуется отбор периода
	Если ИспользоватьПериод=Истина Тогда
		Запрос.Текст = Запрос.Текст + " &Период ";
	КонецЕсли;
	Запрос.Текст = Запрос.Текст + " , ";
	// если требуется отбор по организации
	Если ИспользоватьПериод=Истина Тогда
		Запрос.Текст = Запрос.Текст + " Организация = &Организация ";
	КонецЕсли;
	Запрос.Текст = Запрос.Текст + ") КАК Т ";
	// если требуются только положительные остатки
	Если ИгнорироватьОтрицательныеОстатки=Истина Тогда
		Запрос.Текст = Запрос.Текст + " ГДЕ Т.КоличествоОстаток>0";
	КонецЕсли;	
	
	//...
	
КонецФункции

В этом случае не стоит как-либо продумывать структуру запроса, использовать "Схему Запроса" или специальные функции из БСП для установки параметров динамических списков (из общего модуля "ОбщегоНазначенияКлиентСервер" функции "УстановитьЭлементОтбораДинамическогоСписка") и т.п.

Также, если вам надо составить какое-либо сообщение для пользователя пишите именно так как есть соединяя все в одной строчке:

ТекстСобщения = "В строке №" + НомерСтроки + " не заполнена колонка " + ИмяКолонки.

И не в коем случае не пользуйтесь функцией "СтрШаблон" - предавая шаблонизированную строку ("В строке №%1 не заполнена колонка %2") и параметры. 

12. Возврат разных типов данных для функций

Примите за правило возвращать разные типы в результате функции. Внесите немного загадочности и всегда возвращайте в результате функции различные типы данных. Это заставит других программистов держаться "бодрячком" и всегда ожидать подвоха.

Функция ПолучитьРезультатОбработкиДанных(Ключ, Данные)
	
	РезультатФункции = Неопределено;
	
	Если Ключ=42 Тогда
		// вернем массив
		РезультатФункции = Новый Массив;
		Для каждого стр из Данные Цикл
			РезультатФункции.Добавить(стр);
		КонецЦикла;
	ИначеЕсли Ключ=Неопределено Тогда
		// вернем тип первого элемента данных
		РезультатФункции = Данные[0];
	Иначе
		// вернем строку
		РезультатФункции = "Не поддерживается обработка!";
	КонецЕсли;
	
	Возврат РезультатФункции;
	
КонецФункции

13. Использование магических цифр

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

ДеньКонтроля = ТекущаяДата()+60*60*24;

Или так:

Если Количество=42 Тогда
     Цена=Цена*2;
КонецЕсли;

14. Использование жесткого кодирования

Всегда где можете вставляйте данные об окружении в программный код. Например различные пути к файлам, пути к обработкам, пути к ресурсам. Этот антипаттерн всегда надо использовать совместно с "магическими числами".

&НаКлиенте
Процедура ОбработкаКоманды(ПараметрКоманды, ПараметрыВыполненияКоманды)

    //Помещаем обработку во временном хранилище
    АдресХранилища = "";
    Результат = ПоместитьФайл(АдресХранилища, "C:\Temp\ВнешняяОбработка.epf", , Ложь);           
    ИмяОбработки = ПодключитьВнешнююОбработку(АдресХранилища);
    
    // Откроем форму подключенной внешней обработки
    ОткрытьФорму("ВнешняяОбработка."+ ИмяОбработки +".Форма");

КонецПроцедуры

&НаСервере
Функция ПодключитьВнешнююОбработку(АдресХранилища)

    Возврат ВнешниеОбработки.Подключить(АдресХранилища);

КонецФункции 

15. Всегда все делаем свое или изобретаем велосипеды 

Помните! Ни в коем случае не используем готовые функции или подсистемы БСП. Даже если эта подсистема включена по умолчанию во всех типовых конфигурациях. Кто знает какие там баги и костыли? Существует большая вероятность, что создание библиотеки стандартных подсистем - это хитрый и коварный заговор в корпорации зла (Фирма "1С"), целью которого является упрощение и ускорение разработки и доработки конфигураций под Платформу 1С.

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

Эта методика позволит максимально запутать Ваш программный код и тогда кроме вас поддерживать его уже никто не сможет.

16. Синдром 80-го уровня или чрезмерная сложность

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

"Для полноценной конфигурации достаточно 1-го справочника, 1-го документа и 1-го регистра" - автор неизвестен.

17. Не будем ничего удалять, а вдруг пригодиться

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

P.S. А если серьезно, то:

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

Ссылки на полезные статьи:

• По следам код-ревью
• Как завести у себя в команде код-ревью. Отвечаем на вопросы
• Качество кода: слабое связывание и высокая сопряженность (Low coupling and High Cohesion)
• Качество кода: Поведенческие паттерны проектирования
• Типичные ошибки, некоторые вопросы качества и эффективности работы при разработке в 1С
• Обзор полезных методов БСП 3.1.4
• Работа со схемой запроса