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

SONARQUBE У кого тоже есть такая проверка?

↓ [Волшебник, 19.10.20 - 09:27]
SONARQUBE У кого тоже есть такая проверка?
Я
   osa1C
 
18.10.20 - 09:37
Доброго всем времени суток!
Ветка как обсуждение, как такового вопроса нет...
Столкнулся в одной крупной организации с проверкой кода Сонар Кубом. Вроде как проверка на гавнокод... Но иногда доходит до глупости вида
//Добавлено 02.10.2020 [отАвтор} по задаче 3243565-ИР - это комментарий в коде. Ругается на отсутствие ПРОБЕЛА после //. Смешно! Но это мелочь. Наткнулся и на другие проблемы.
   Например:
НужныйТипДокумента =  Получить(ДокОбъект);

Функция НужныйТипДокумента (ДокОбъект)

Если ТнпЗнч(ДокОбъект) = Тип("ДокументОбъект.ПриходныйКассовыйОрдер")
    Или ТнпЗнч(ДокОбъект) = Тип("ДокументОбъект.ПоступлениеБезналичныхДС")
    Или ТнпЗнч(ДокОбъект) = Тип("ДокументОбъект.ВзаимозачетДенежныхСредств") Тогда
    Возврат Истина;
КонецЕсли;
Возврат Ложь;
КонецЕсли;

выдает ошибку "Слишком сложное условие в Если.... внесите в переменную, а также вынесите условия в отдельную функцию"


прикольно... а как у вас?
   NorthWind
 
1 - 18.10.20 - 09:42
(0) ну в общем он прав. Как минимум имеет смысл сделать ТипОбъекта = ТнпЗнч(ДокОбъект) и потом сравнивать переменную. Если сравнений реально много (больше пяти), то я могу сделать из них список значений и потом в условии просто проверить принадлежность значения переменной списку. Это более читабельно.
   ДенисЧ
 
2 - 18.10.20 - 09:50
Насчёт пробела - это требования к комментариям от 1с. Так что тут всё нормально. Ещё и точку в конце не забудь
   osa1C
 
3 - 18.10.20 - 09:51
(1) С этим я согласен, но само условие из двух ИЛИ .... чем сложно?
   osa1C
 
4 - 18.10.20 - 09:53
(2) Ден, Чем мешается отсутствие пробела в комментарии, поясни...
   Конструктор1С
 
5 - 18.10.20 - 09:55
>>выдает ошибку "Слишком сложное условие в Если.... внесите в переменную, а также вынесите условия в отдельную функцию"

Он всё правильно говорит. Это проверка на говнокодерство. Слишком сложные логические проверки усложняют восприятие кода
Предлагает тебе вынести логическое условие за конструкцию Если. Либо в переменную, либо в отдельную функцию

ТнпЗнч(ДокОбъект) = Тип("ДокументОбъект.ПриходныйКассовыйОрдер")
    Или ТнпЗнч(ДокОбъект) = Тип("ДокументОбъект.ПоступлениеБезналичныхДС")
    Или ТнпЗнч(ДокОбъект) = Тип("ДокументОбъект.ВзаимозачетДенежныхСредств")

вот это чтобы не торчало внутри Если. Можно например так

ТипДокумента = ТнпЗнч(ДокОбъект);
ЭтоПоступлениеДенех = ТипДокумента  = Тип("ДокументОбъект.ПриходныйКассовыйОрдер")
    Или ТипДокумента = Тип("ДокументОбъект.ПоступлениеБезналичныхДС")
    Или ТипДокумента = Тип("ДокументОбъект.ВзаимозачетДенежныхСредств");

Если ЭтоПоступлениеДенех Тогда
    Возврат Истина;
Иначе
    Возврат Ложь;
КонецЕсли;

и название функции НужныйТипДокумента() у тебя говнокодерское (хотя сонаркуб этого и не выявляет). Оно не говорит совершенно ни о чём. Чтобы понять цель функции нужно занырнуть внуть. Переименуй хотя бы на ЭтоДокументВыполняющихПриходДенежныхСреств(), такое название описывает выполняемую функцией проверку
   NorthWind
 
6 - 18.10.20 - 09:56
(3) там, во-первых, не из двух, а из трех, а во-вторых, внутри условия еще вызовы функций.
Такие штуки довольно бывает муторно читать "свежему" человеку, если используются какие-то более редкие функции чем ТипЗнч (). Вот на это оно у вас и обругалось.
   ДенисЧ
 
7 - 18.10.20 - 09:59
(4) Есть правила, прописанные вендором. Их надо соблюдать.
   ДенисЧ
 
8 - 18.10.20 - 10:00
(5) "ЭтоПоступлениеДенех = ТипДокумента  = Тип("ДокументОбъект.ПриходныйКассовыйОрдер")
    Или ТипДокумента = Тип("ДокументОбъект.ПоступлениеБезналичныхДС")"

Это что за говнокод??
   Конструктор1С
 
9 - 18.10.20 - 10:00
(4) усложняют чтение комментария
   NorthWind
 
10 - 18.10.20 - 10:00
Я бы сделал вот так

    // Ниже перечисление типов для проверки. При необходимости могут быть легко

    // добавлены новые

    ТипыДляПроверки = Новый СписокЗначений ();
    ТипыДляПроверки.Добавить (Тип ("ДокументОбъект.ПриходныйКассовыйОрдер"));
    ТипыДляПроверки.Добавить (Тип ("ДокументОбъект.ПоступлениеБезналичныхДС"));    
    ТипыДляПроверки.Добавить (Тип ("ДокументОбъект.ВзаимозачетДенежныхСредств"));    
    
    Возврат ТипыДляПроверки.НайтиПоЗначению(ТипЗнч (ДокОбъект));

Думаю, на это оно не обругалось бы.
   Конструктор1С
 
11 - 18.10.20 - 10:01
(8) это поясняющая переменная. Пока что неведомый для 1сников зверёк. Название переменной поясняет, что и для чего
   NorthWind
 
12 - 18.10.20 - 10:01
Точнее
Если ТипыДляПроверки.НайтиПоЗначению(ТипЗнч (ДокОбъект))=Неопределено Тогда
Возврат Ложь;
Иначе
Возврат Истина;
   NorthWind
 
13 - 18.10.20 - 10:02
КонецЕсли;
   osa1C
 
14 - 18.10.20 - 10:02
Т (8) С чего это то говонокод, если с функции булево возращать таким кодом
   ДенисЧ
 
15 - 18.10.20 - 10:03
(11) Это не переменная. Это говнокод, который невозможно с первого взгляда прочитать
   Конструктор1С
 
16 - 18.10.20 - 10:03
(14) как-минимум из-за говнокодерского названия функции. Название должно рассказывать, что делает данная функция
   Конструктор1С
 
17 - 18.10.20 - 10:04
(15) с чего бы это вдруг? Т.е. для тебя открытие, что до первого знака равенства содержится имя переменной, которой присваивается значение?
   osa1C
 
18 - 18.10.20 - 10:06
(16) ок, я похож на говнокодера, как я должен был назвать функцию, чтобы тебе было понятно?
   Стаканов
 
19 - 18.10.20 - 10:08
Опять срач на ровном месте. ТС, используй АПК для начала, там прямо из ошибки можно перейти на описание стандарта на ИТС, и там почитать, что к чему и как.
   osa1C
 
20 - 18.10.20 - 10:09
Вообще то переписав код получил его от СонарКуб без ошибок, но все равно не считаю что Сонара прав
   Конструктор1С
 
21 - 18.10.20 - 10:10
(18) должно быть понятно не мне, а читающему код. Вот увидел некий Вася в коде

...
НужныйТипДокумента(ДокОбъект)
...

о чём ему говорит название НужныйТипДокумента? Да ни о чём. Такое название провоцирует залезть внутрь функции чтобы выяснить её содержимое. А будь у функции "говорящее" название, потребность заглядывать вовнутрь отпала бы сама собой
   ДенисЧ
 
22 - 18.10.20 - 10:10
(17) Ещё раз. Чтобы нормально читать - это говнокод. Хочешь так писать - хотя бы поставь скобки. Тогда не так сильно бить будут.
   Конструктор1С
 
23 - 18.10.20 - 10:10
(20) тем не менее, он прав. Ща нацитирую МакКонела
   ДенисЧ
 
24 - 18.10.20 - 10:10
(18) Например, ЭтоНужныйТипДокумента. Минимум
   ДенисЧ
 
25 - 18.10.20 - 10:11
Кстати, об АПК

Возьмите типовую и прогоните... )))
   osa1C
 
26 - 18.10.20 - 10:11
(19) В чем заключен расчет производительности от того что в Если засунул условие с двумя ИЛИ ?
   osa1C
 
27 - 18.10.20 - 10:12
Условие с двумя ИЛИ убивает производительность системы?
   Стаканов
 
28 - 18.10.20 - 10:13
(25) Да нафига типовую проверять, даже если в типовых лажа, это не повод самому лажу гнать. Проверять свой внесенный код надо.
   Стаканов
 
29 - 18.10.20 - 10:14
(26) При чём тут производительность, это замечание на соответствие читаемости кода понятиям 1С.
   Конструктор1С
 
30 - 18.10.20 - 10:14
(22) что ты пристал? Я всего-лишь вытящил конструкцию ТС за Если. Мог бы расписать ещё "документальнее"


ТипДокумента = ТнпЗнч(ДокОбъект);

ЭтоПоступлениеНалички = (ТипДокумента  = Тип("ДокументОбъект.ПриходныйКассовыйОрдер"));
ЭтоПоступлениеБезнала = (ТипДокумента = Тип("ДокументОбъект.ПоступлениеБезналичныхДС"));
ЭтоВзаимозачетДенег = (ТипДокумента = Тип("ДокументОбъект.ВзаимозачетДенежныхСредств"));

ЭтоПоступлениеДенех = (ЭтоПоступлениеНалички Или ЭтоПоступлениеБезнала Или ЭтоВзаимозачетДенег);

Если (ЭтоПоступлениеДенех) Тогда
    Возврат Истина;
Иначе
    Возврат Ложь;
КонецЕсли;
 
 Рекламное место пустует
   Конструктор1С
 
31 - 18.10.20 - 10:16
(24) добавление "Это" всего-лишь поясняет, что функция выполняет логическую проверку. При этом функция продолжает быть "черной коробочкой", в которую полезе каждый читающий код программист
   Конструктор1С
 
32 - 18.10.20 - 10:17
(27) сложные логические проверки убивают производительность читающего код
   osa1C
 
33 - 18.10.20 - 10:17
на сколько быстрее работает конструкция
 Тип - ТипЗнч(ДокОбьект);

НужныйТип - ПолучитьНужныйТипДокумента(Тип)



Функция НужныйТипДокумента (Тип)

Возврат = Тип("ДокументОбъект.ПриходныйКассовыйОрдер")
    Или ТнпЗнч(ДокОбъект) = Тип("ДокументОбъект.ПоступлениеБезналичныхДС")
    Или ТнпЗнч(ДокОбъект) = Тип("ДокументОбъект.ВзаимозачетДенежныхСредств") 
КонецФункции;
   Стаканов
 
34 - 18.10.20 - 10:18
(33) Поизучал бы ты стандарты 1С на досуге, глупых вопросов бы поубавилось.
   Конструктор1С
 
35 - 18.10.20 - 10:18
(33) ни на сколько, но так читабельность ещё больше падает
   osa1C
 
36 - 18.10.20 - 10:19
+ (33) замена*** 
Функция НужныйТипДокумента (Тип)

Возврат = Тип("ДокументОбъект.ПриходныйКассовыйОрдер")
    Или Тип("ДокументОбъект.ПоступлениеБезналичныхДС")
    Или Тип("ДокументОбъект.ВзаимозачетДенежныхСредств") 
КонецФункции;
   osa1C
 
37 - 18.10.20 - 10:21
(34) Сам только по стандарту работаешь?
   Конструктор1С
 
38 - 18.10.20 - 10:23
Блин, МакКонела так просто не втиснешь, слишком много он про сложность кода написал. Но если кому интересно, глава "19.6. Управляющие структуры и сложность" вот этой книги:
https://www.ozon.ru/context/detail/id/138437220/
   osa1C
 
39 - 18.10.20 - 10:27
(38) Совершенный код это приятно, но не в разделе 1С это давать ))
   Aleksey
 
40 - 18.10.20 - 10:29
(30) Индуский код? Платят за строки?
   Конструктор1С
 
41 - 18.10.20 - 10:32
(40) нет, поясняющий код. А экономия на строках кода верный признак говнокодерства
   NorthWind
 
42 - 18.10.20 - 10:32
(20) конкретно в вашем случае там действительно ничего совсем уж страшного нет. Но вообще стоит принять, что для улучшения читаемости условия должны быть как можно проще. Длинные условия - это плохо, условия, внутри которых много вызовов функций - тоже не есть хорошо. Только и всего. Не стоит подходить к этому как к догме, нужно просто осознать с точки зрения человека, который придет "со стороны" и возьмется читать код.
   Стаканов
 
43 - 18.10.20 - 10:35
(37) Да, весь код проверяю АПК, пока проверка не пройдёт, в продуктив не добавляю.
   Aleksey
 
44 - 18.10.20 - 10:43
(42) Т.е. вместо 2 "или" использовать 4 "если" с простым условием и которые возводят с переменными флага внутри проверки?
А ешще лучше вызывать отдельный функции из ГМ передавая вест контекст
   NorthWind
 
45 - 18.10.20 - 10:47
(44) по-разному можно. Можно (30), можно (10)-(12).
   Конструктор1С
 
46 - 18.10.20 - 10:47
(44) почитай (38), много нового для себя откроешь. Не собираюсь читать лекцию на тему "что делает код сложным для восприятия и как его сделать простым"
   Надо работать
 
47 - 18.10.20 - 10:51
(43) крутяк. у себя никак руки не дойдут внедрить... такое пишут люди - волосы дыбом встают ) но если нет провала по производительности - пока пропускаю
   Конструктор1С
 
48 - 18.10.20 - 10:54
Когда упрощаешь код, неминуемо увеличивается количество строк кода. Это происходит из-за:
- разбиения сложных методов на более простые
- разбиения сложных конструкций на более простые
- разбиения сложных логических проверок на более простые
- некоторого удлинения имен переменных и методов (например, вместо КонтрПодходит() используется КонтрагентЯвляетсяРезидентомСНалоговымиЛьготами())
   Стаканов
 
49 - 18.10.20 - 13:27
(47) АПК применять можно хотя-бы для понимания, насколько всё плохо, и направления на путь истинный тех, у кого очень плохо :)
   1CnikPetya
 
50 - 18.10.20 - 16:16
Тоже планируем внедрять статический анализ кода. Только вместо АПК смотрим на SONAR. Тупо намного быстрее (хотя я его не гонял после последних оптимизаций 1С работы над строками).
   Стаканов
 
51 - 18.10.20 - 16:18
(50) Сонар будет когда-нибудь, да, но надо идти от простого к сложному, а то так ничего и не будет. Для использования АПК практически и никаких усилий не надо.
   palsergeich
 
52 - 18.10.20 - 17:52
(50) АПК <> Сонар
В Сонаре есть не все, что есть в АПК.
И наоборот тоже верно.
   Стаканов
 
53 - 18.10.20 - 17:58
(52) Статический анализ это вообще просто гигиена разработки :)
   osa1C
 
54 - 18.10.20 - 21:57
(43) Оно конечно правильно, но не просаживаться же до нелогичности пробела
   osa1C
 
55 - 18.10.20 - 21:59
(48) Согласен, и повышается его нечитабильность
   1CnikPetya
 
56 - 18.10.20 - 23:00
(54) Когда ты пишешь один - на это можно забить. А вот когда на проекте 2 десятка разработчиков, то единый стиль кода намного упрощает процесс ревью кода и последующее чтение. А начиная с пробела может пойти вразброс все: выравнивание табами, именования функций и переменных, оформление запросов и модулей и прочее-прочее. Поэтому проще зафиксировать единые правила и далее следовать им без отклонений.

(52) Давно смотрел на SONAR, но и то не очень глубоко изучал. Предполагаю, что у АПК есть возможность проверки метаданных, верно? Роли, формы, подсистемы, в то время как SONAR ориентирован именно на код?

(55) От грамотной декомпозиции всегда обратный эффект. Портянка на 10 экранов кажется понятной и логичной лишь первые пару недель после написания. А так, "заставь дурака молиться..."
   Сияющий в темноте
 
57 - 18.10.20 - 23:24
Если условие сложное,то пишите в комментариях,что оно делает,разбивка на дополнительные переменные,на самом деле,только усложняет код,так как вместо условия нужно возвращаться наверх и смотреть,а что там переменной присвоили.
проверка на типы лучше всего идет через описание типов и его метод СодержитТип,тогда ничего не нужно объяснять вообще.
   Web00001
 
58 - 19.10.20 - 02:39
>>разбивка на дополнительные переменные,на самом деле,только усложняет код,так как вместо условия нужно возвращаться наверх и смотреть,а что там переменной присвоили.
Если в имени переменной написано, что ей присовили и ты это видел когда читал, зачем возвращаться?
   Стаканов
 
59 - 19.10.20 - 07:44
(54) Так почитай стандарты, там написано, зачем нужен пробел в начале комментария.
   palsergeich
 
60 - 19.10.20 - 08:05
(59) Не написано
7.3. Большие комментарии или комментарии к фрагменту кода пишутся перед комментируемым кодом в отдельной строке. Текст выравнивается по левой границе комментируемого фрагмента. Между символами комментария "//" и текстом комментария должен быть пробел.
Пруф https://its.1c.ru/db/v8std/content/456/hdoc
 
 Рекламное место пустует
   ДенисЧ
 
61 - 19.10.20 - 08:07
Про комментарии...
Обратите внимание - в том же vscode при комментировании по горячей кнопке тоже ставится пробел...
Общепринято в мире, наверное
   Конструктор1С
 
62 - 19.10.20 - 09:04
(55) не-а. При перямых руках эффект обратный - с упрощением кода повышается его читаемость
   Стаканов
 
63 - 19.10.20 - 09:07
(60) Ну вот зачем ты так, я же человека стандарты читать хотел научить :)) Но если подумать, то таки в п.3 написано, зачем :)
   Конструктор1С
 
64 - 19.10.20 - 09:09
(57) "разбивка на дополнительные переменные,на самом деле,только усложняет код"

Это если переменные обозваны абы как и находятся абы где. Нормальный код читается сверху вниз и один раз (ибо понимается с первого раза). А сложное логическое условие заставляет останавливаться и перечитывать его несколько раз
   vi0
 
65 - 19.10.20 - 09:10
(61) и с точки зрения читабельности этот пробел понятен
   Конструктор1С
 
66 - 19.10.20 - 09:20
(57) "лучше всего идет через описание типов и его метод СодержитТип"

сомнительно, ибо такая проверка:
- заставляет скролить вверх, чтобы перечитать содержащиеся в описании типы
- не говорит ничего о том, почему выбраны именно эти типы
   Web00001
 
67 - 19.10.20 - 09:34
(63)Извините туповат. Думал, думал не придумал. Не смог найти в п.3 про то, зачем нужен пробел. п.3 рассказывает про то, что нельзя оставлять отладочные комментарии, или комментарии имеющие отношение к процессу разработки, а про пробел ни слова.
   Стаканов
 
68 - 19.10.20 - 09:41
(67) Когда комментарий начинается не с пробела, скорее всего его нужно убрать из кода :))
   Web00001
 
69 - 19.10.20 - 09:57
(58) в (63) uоворилось, что там есть ответ ЗАЧЕМ, это надо сделать вроде как ) а не указание что это надо сделать.
   Конструктор1С
 
70 - 19.10.20 - 10:12
(67) //лишние *символы //в $$начале %/слова №/затрудняют &чтение /*текста
   Стаканов
 
71 - 19.10.20 - 10:18
(69) Чо ты такой серьёзный? Если не понял, это же шутка была в (63) :)) А если понял, тогда к чему вопрос?
   Web00001
 
72 - 19.10.20 - 10:26
(70)разве? Не заметил.
   Конструктор1С
 
73 - 19.10.20 - 10:27
(72) чё, в самом деле что ли?
   Web00001
 
74 - 19.10.20 - 10:32
(71)Триггирюсь на фразу "если подумать" ищу подвох каждый раз))))
Раз уж все такие умные, вот пример отделаю блоки текста https://take.ms/bbcEV комментарии очевидные, но они позволяют сразу перейти в нужной области вывода документа, то есть служат якорями а не комментариями. Это ОК или фу фу фу?
   Стаканов
 
75 - 19.10.20 - 10:50
(74) Ну так если пробел поставить перед текстом комментария, что страшного случиться? Я вот уже на автомате пробел ставлю, и в шаблонах текста во всех пробел забил.
   Конструктор1С
 
76 - 19.10.20 - 13:18
(74) серьёзного ничего не произойдёт. Но на чтение комментария без пробелов уйдёт больше времени. Потратил времени маленько здесь, маленько тут, маленько там, и вот уже время разработки значительно выросло
   vi0
 
77 - 20.10.20 - 05:42
(74) это уже не комментарии получаются, а читающие будут считать их за комментарии
   Web00001
 
78 - 21.10.20 - 06:34
(77)Так и что делать? в хорошем коде не должно быть такого?


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