Вход | Регистрация
 

Рефакторинг кода в 1С

Рефакторинг кода в 1С
Я
   НоваяВолна
 
12.01.22 - 11:34
Надоел робот, который не воспринимает код, вроде и простой и говорит ИСПРАВЬ! Хоть и часы по рефакторингу у нас оплачиваются как и обычные рабочие часы, но лезть в готовый код, переделывать и по новой тестировать уже сданную работу как то не комильфо. И ладно ещё были бы серьезные ошибки, а то ругается на такую мелочь, как //Комментарий - отствие пробела между // и Комментарий )))). А теперь к сути. как бы вы написали конструкцию вида:

        Если ВидНеисправности = Справочники.ВидНеисправности .НайтиПоКоду("10464")
            ИЛИ  ВидНеисправности = Справочники.ВидНеисправности .НайтиПоКоду("10998")
            ИЛИ  ВидНеисправности = Справочники.ВидНеисправности .НайтиПоКоду("10999")
            ИЛИ  ВидНеисправности = Справочники.ВидНеисправности .НайтиПоКоду("10929")
            ИЛИ  ВидНеисправности = Справочники.ВидНеисправности .НайтиПоКоду("10467") Тогда
                ......
       КонецЕсли;

ругается что слишком сложная конструкция из ИЛИ
   dubolom
 
1 - 12.01.22 - 11:37
МассивВН = Новый Массив;
МассивВН.Добавить(Справочники.ВидыНеисправности.НайтиПоКоду("ХорошийКод"));
МассивВН.Добавить(Справочники.ВидыНеисправности.НайтиПоКоду("ОченьХорошийКод"));
МассивВН.Добавить(Справочники.ВидыНеисправности.НайтиПоКоду("ВащеОтстойныйКод"));
Если МассивВН.Найти(ВидНеисправности)<>Неопределено Тогда
    ...
КонецЕсли;
   pechkin
 
2 - 12.01.22 - 11:38
(1) Лучше соотвествие
   ДенисЧ
 
3 - 12.01.22 - 11:38
Справочники.ВидНеисправности.Поломка
и т.п.
   pechkin
 
4 - 12.01.22 - 11:39
а правильно нужно галку какую поставить в справочник
   arsik
 
5 - 12.01.22 - 11:40
(2) Массив быстрее
   Василий Алибабаевич
 
6 - 12.01.22 - 11:40
(0)
МассивКодовНеисправностей = Новый Массив;
МассивКодовНеисправностей.Добавить("10464");
МассивКодовНеисправностей.Добавить("...");
МассивКодовНеисправностей.Добавить("...");
МассивКодовНеисправностей.Добавить("...");

Если МассивКодовНеисправностей.Найти(ВидНеисправности.Код) <> Неопределено Тогда
   pechkin
 
7 - 12.01.22 - 11:41
(5) в массиве поиск - это перебор, а в соотвествии - хэшмап
   RomaH
 
8 - 12.01.22 - 11:41
Справочники.ВидНеисправности.Поломка();

Функция Поломка() Экспорт
Возврат Справочники.ВидНеисправности.ПолучитьСсылку(новый УникальныйИдентификатор(""));
КонецФункции
   dubolom
 
9 - 12.01.22 - 11:41
(7) Хосподя, у него пять элементов.
   Василий Алибабаевич
 
10 - 12.01.22 - 11:41
+ (6) Или можно строку с разделителями. Но строка ИМХО не так наглядно. И будет тяжелее в сопровождении.
   arsik
 
11 - 12.01.22 - 11:44
(7) На создание и заполнение соответствия время потеряешь. Если Соответствие заполнить 1 раз и вызывать поиск 100 тогда возможно будет быстрее.
   pechkin
 
12 - 12.01.22 - 11:45
(11) все равно правильный вариант это (4)
   mistеr
 
13 - 12.01.22 - 11:47
(0) Я бы заменил на функцию. А список кодов на массив ссылок с кэшированием.
   dubolom
 
14 - 12.01.22 - 11:48
(13) Тогда уж запилить отдельный регистр сведений со списком требуемых видов неисправности, чтобы каждый раз в код не лезть.
   Uberschall
 
15 - 12.01.22 - 11:52
(7) это так, но чтобы это знать, надо быть "одной ногой" в "нормальном" ЯП. т.е. по сути знать несколько ЯП и все -равно заниматься 1С)))
   mistеr
 
16 - 12.01.22 - 11:52
(14) Возможно, если он часто меняется.
   Garykom
 
17 - 12.01.22 - 11:53
(0)
Коды = Новый Структура("10464, 10998, ...");
Если Коды.Свойство(ВидНеисправности.Код) Тогда
// ...

КонецЕсли;

   dubolom
 
18 - 12.01.22 - 11:55
(12) Смотря по размеру справочника.
Если он большой, а в коде, как сейчас, вариантов 5 - то правильнее (14)
   Garykom
 
19 - 12.01.22 - 11:55
(0) У тебя ошибка не в сложном условии а в куче лишних дерганий БД через НайтиПоКоду
   mistеr
 
20 - 12.01.22 - 11:55
(17) Хороший вариант, если вместо Коды будет говорящее имя.
   RomanYS
 
21 - 12.01.22 - 11:55
(17) низзя такие ключи в структуре.
   Garykom
 
22 - 12.01.22 - 11:55
(21) ну сделай массив или соответствие
   НоваяВолна
 
23 - 12.01.22 - 11:58
(4) галку? интересно, но не понятно. выборка по справочнику потому и идет по нескольким кодам, потому как Виды Неисправностей, а точнее объекты ремонта разные. Грубо говоря если случились перечисленные неисправности, то сломался к примеру холодильник, а если другие, то радиоприемник, третьи - телевизор и т.д.
   Garykom
 
24 - 12.01.22 - 11:59
(21) хотя влом
Коды = Новый Структура("К10464, К10998, ...");
Если Коды.Свойство("К"+ВидНеисправности.Код) Тогда
// ...

КонецЕсли;
   Garykom
 
25 - 12.01.22 - 11:59
(24)+ или вместо "К" лучше|можно "_" ?
   dubolom
 
26 - 12.01.22 - 12:00
(23) Тогда не галку, а перечисление "ПоломаннаяХрень". И соответствующий реквизит в справочнике/регистре сведений.
   dubolom
 
27 - 12.01.22 - 12:01
(24) Если бы я был такой же злой, как средний мистянин, написал бы, что это - образец говнокода.
   RomanYS
 
28 - 12.01.22 - 12:02
(24) имхо лучше так 
Массив = СтрРазделить("10464, 10998, ..."); 

(25) а если код будет с пробелом или ещё каким символом запрещенным в ключах?
   Garykom
 
29 - 12.01.22 - 12:02
(28) Согласен
   Garykom
 
30 - 12.01.22 - 12:03
(27) Говнокод это в (0) У меня ближе к https://ru.wikipedia.org/wiki/Brainfuck - излишняя оптимизация
 
 
   Garykom
 
31 - 12.01.22 - 12:05
Суть что если у ТС работа с кодами то нехрен дергать базу
Коды из строки в некую структуру и ищи в ней
   VladZ
 
32 - 12.01.22 - 12:08
(0) А можно объединить эти Виды неисправностей общим признаком?
Тогда можно добавить реквизит в справочник и отбирать по этому признаку.

Плюсы: при добавлении нового элемента справочника с подобным признаком нет необходимости переписывать код.
Минусы: требуется дополнительный реквизит (если база на поддержке - нужно продумать варианты).
   НоваяВолна
 
33 - 12.01.22 - 12:13
(32) в том то и дело что нет. По идее создается документ в котором группой является Неисправность, а подгруппы Виды неисправностей. При этом Объект ремонта для каждой подгруппы свой. Объекты - это отдельный справочник
   VladZ
 
34 - 12.01.22 - 12:21
(33) Из личного опыта: кусок кода в сабже - это показатель неэффективной архитектуры.
А чтобы дать эффективный ответ на вопрос в сабже - нужно видеть архитектуру системы в целом.

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

Как говорят китайцы: "Живи, сохраняя покой. Придет весна и цветы распустятся сами"...
   dubolom
 
35 - 12.01.22 - 12:21
(33) Ну соответствие же "многие к одному" типичное. (26) чем не устраивает?
   ildary
 
36 - 12.01.22 - 12:25
(0) Для глупых проверок типа отсутствие пробела между // и Комментарий - лучше включить запрет их обнаружения - спецкомментарий в начале кода.
   mikecool
 
37 - 12.01.22 - 13:00
(0) за такой код линейкой бить надо
   mikecool
 
38 - 12.01.22 - 13:00
+37 чугунной
   Asmody
 
39 - 12.01.22 - 13:02
(38)+ по голове
   DrShad
 
40 - 12.01.22 - 13:02
(39) сначала по пальцам
   ДенисЧ
 
41 - 12.01.22 - 13:03
(40) Лучше по яйцам.
   DrShad
 
42 - 12.01.22 - 13:04
(41) +100500 чтобы не размножались
   mikecool
 
43 - 12.01.22 - 13:04
перепись садистов ))
   Asmody
 
44 - 12.01.22 - 13:05
(36) ну, вообще-то, ненужные проверки отключаются в настройках. (полагаю, что у ТС модный нынче сонаркуб с bsl-плагином). только такие настройки обсуждаются в команде, а не по принципу "я хочу, я не хочу".
порядок в коде должен быть
   Asmody
 
45 - 12.01.22 - 13:06
(43) ты первый начал
   DrShad
 
46 - 12.01.22 - 13:07
(43) садист в данном случае это ТС
прикинь тебе в наследство достанется такая БД?
   mikecool
 
47 - 12.01.22 - 13:08
(45) ага ))
как то на проекте в тестовую(даже не в прод) проскочил аналог кода из (0), прог забыл убрать - так буча была поднята со стороны заказчика, что чуть ли не проект сворачиваем... )))
   mikecool
 
48 - 12.01.22 - 13:09
(44) дык такие конструкции однозначно проверки не должны пропускать, ибо овнокод )
   Asmody
 
49 - 12.01.22 - 13:11
(47) За НайтиПоКоду() и НайтиПоНаименованию() надо бить вот этой книжкой https://v8.1c.ru/metod/books/42696.htm
   Dmitrii
 
50 - 12.01.22 - 13:14
Соглашусь с (34). Проблема не в коде, а в архитектуре.
Либо переделать по человечески. И тогда надо пересматривать архитектуру.
Либо оставить пока как есть, отложив до тех времён, когда будет возможность сделать нормально.

(47) >> в тестовую проскочил аналог кода из (0), прог забыл убрать - так буча была поднята со стороны заказчика.
Тут всё таки обратная ситуация. *авнокод уже в проде. Любая оптимизация поиска или проверки по коду в справочнике даст только лишь оптимизированный *авнокод. Надо подход пересматривать, а не менять один *авнокод на другой - более оптимальный. ИМХО.
   DimVad
 
51 - 12.01.22 - 13:19
(17) Может вместо "Если Коды.Свойство(ВидНеисправности.Код) Тогда"
что-то типа :

Код_ = ОбщегоНазначения.ЗначениеРеквизитаОбъекта(ВидНеисправности, "Код");
Если Коды.Свойство(Код_) Тогда
   ildary
 
52 - 12.01.22 - 13:37
(44) Да, но нет: в коде надежнее - завтра эту конфигурацию откроют удаленщики без сонаркуба, но с вручную подключенным BSL (Turboconf, phoenixbsl) - и им уже не надо заморачиваться.
   Asmody
 
53 - 12.01.22 - 13:40
(52) Если практикуется сонаркуб, то код удаленщиков так же обязан через него проходить.
   Гений 1С
 
54 - 12.01.22 - 13:47
(0) хороший робот, ободряю. А робот ободряет разработчиков.
   ildary
 
55 - 12.01.22 - 13:50
(53) Да, но удаленщики проверяют свой код до сонаркуба.
   novichok79
 
56 - 12.01.22 - 14:42
да, тяжко в 1С без нормальных линтеров.
ну шо, хешмап, можно даже фиксированный чтоб наверняка и инициализация его 1 раз.
   Asmody
 
57 - 12.01.22 - 14:51
(55) это проблемы удаленщиков. ну или дайте им свой .bsl-language-server.json
   mikecool
 
58 - 12.01.22 - 14:51
(50) "Либо оставить пока как есть, отложив до тех времён, когда будет возможность сделать нормально. " Такое время не наступает почти никогда


Список тем форума
 
ВНИМАНИЕ! Если вы потеряли окно ввода сообщения, нажмите Ctrl-F5 или Ctrl-R или кнопку "Обновить" в браузере.