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

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

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

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


прикольно... а как у вас?

(0) ну в общем он прав. Как минимум имеет смысл сделать ТипОбъекта = ТнпЗнч(ДокОбъект) и потом сравнивать переменную. Если сравнений реально много (больше пяти), то я могу сделать из них список значений и потом в условии просто проверить принадлежность значения переменной списку. Это более читабельно.

Насчёт пробела - это требования к комментариям от 1с. Так что тут всё нормально. Ещё и точку в конце не забудь

(1) С этим я согласен, но само условие из двух ИЛИ .... чем сложно?

(2) Ден, Чем мешается отсутствие пробела в комментарии, поясни...

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

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

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

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

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

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

и название функции НужныйТипДокумента() у тебя говнокодерское (хотя сонаркуб этого и не выявляет). Оно не говорит совершенно ни о чём. Чтобы понять цель функции нужно занырнуть внуть. Переименуй хотя бы на ЭтоДокументВыполняющихПриходДенежныхСреств(), такое название описывает выполняемую функцией проверку

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

(4) Есть правила, прописанные вендором. Их надо соблюдать.

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

Это что за говнокод??

(4) усложняют чтение комментария

Я бы сделал вот так

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

Думаю, на это оно не обругалось бы.

(8) это поясняющая переменная. Пока что неведомый для 1сников зверёк. Название переменной поясняет, что и для чего

Точнее

Если ТипыДляПроверки.НайтиПоЗначению(ТипЗнч (ДокОбъект))=Неопределено Тогда
Возврат Ложь;
Иначе
Возврат Истина;

КонецЕсли;

Т (8) С чего это то говонокод, если с функции булево возращать таким кодом

(11) Это не переменная. Это говнокод, который невозможно с первого взгляда прочитать

(14) как-минимум из-за говнокодерского названия функции. Название должно рассказывать, что делает данная функция

(15) с чего бы это вдруг? Т.е. для тебя открытие, что до первого знака равенства содержится имя переменной, которой присваивается значение?

(16) ок, я похож на говнокодера, как я должен был назвать функцию, чтобы тебе было понятно?

Опять срач на ровном месте. ТС, используй АПК для начала, там прямо из ошибки можно перейти на описание стандарта на ИТС, и там почитать, что к чему и как.

Вообще то переписав код получил его от СонарКуб без ошибок, но все равно не считаю что Сонара прав

(18) должно быть понятно не мне, а читающему код. Вот увидел некий Вася в коде

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

о чём ему говорит название НужныйТипДокумента? Да ни о чём. Такое название провоцирует залезть внутрь функции чтобы выяснить её содержимое. А будь у функции "говорящее" название, потребность заглядывать вовнутрь отпала бы сама собой

(17) Ещё раз. Чтобы нормально читать - это говнокод. Хочешь так писать - хотя бы поставь скобки. Тогда не так сильно бить будут.

(20) тем не менее, он прав. Ща нацитирую МакКонела

(18) Например, ЭтоНужныйТипДокумента. Минимум

Кстати, об АПК

Возьмите типовую и прогоните... )))

(19) В чем заключен расчет производительности от того что в Если засунул условие с двумя ИЛИ ?

Условие с двумя ИЛИ убивает производительность системы?

(25) Да нафига типовую проверять, даже если в типовых лажа, это не повод самому лажу гнать. Проверять свой внесенный код надо.

(26) При чём тут производительность, это замечание на соответствие читаемости кода понятиям 1С.

(22) что ты пристал? Я всего-лишь вытящил конструкцию ТС за Если. Мог бы расписать ещё "документальнее"


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

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

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

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

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

(27) сложные логические проверки убивают производительность читающего код

на сколько быстрее работает конструкция
Тип - ТипЗнч(ДокОбьект);

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



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

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

(33) Поизучал бы ты стандарты 1С на досуге, глупых вопросов бы поубавилось.

(33) ни на сколько, но так читабельность ещё больше падает

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

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

(34) Сам только по стандарту работаешь?

Блин, МакКонела так просто не втиснешь, слишком много он про сложность кода написал. Но если кому интересно, глава "19.6. Управляющие структуры и сложность" вот этой книги:
https://www.ozon.ru/context/detail/id/138437220/

(38) Совершенный код это приятно, но не в разделе 1С это давать ))

(30) Индуский код? Платят за строки?

(40) нет, поясняющий код. А экономия на строках кода верный признак говнокодерства

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

(37) Да, весь код проверяю АПК, пока проверка не пройдёт, в продуктив не добавляю.

(42) Т.е. вместо 2 "или" использовать 4 "если" с простым условием и которые возводят с переменными флага внутри проверки?
А ешще лучше вызывать отдельный функции из ГМ передавая вест контекст

(44) по-разному можно. Можно (30), можно (10)-(12).

(44) почитай (38), много нового для себя откроешь. Не собираюсь читать лекцию на тему "что делает код сложным для восприятия и как его сделать простым"

(43) крутяк. у себя никак руки не дойдут внедрить... такое пишут люди - волосы дыбом встают ) но если нет провала по производительности - пока пропускаю

Когда упрощаешь код, неминуемо увеличивается количество строк кода. Это происходит из-за:
- разбиения сложных методов на более простые
- разбиения сложных конструкций на более простые
- разбиения сложных логических проверок на более простые
- некоторого удлинения имен переменных и методов (например, вместо КонтрПодходит() используется КонтрагентЯвляетсяРезидентомСНалоговымиЛьготами())

(47) АПК применять можно хотя-бы для понимания, насколько всё плохо, и направления на путь истинный тех, у кого очень плохо :)

Тоже планируем внедрять статический анализ кода. Только вместо АПК смотрим на SONAR. Тупо намного быстрее (хотя я его не гонял после последних оптимизаций 1С работы над строками).

(50) Сонар будет когда-нибудь, да, но надо идти от простого к сложному, а то так ничего и не будет. Для использования АПК практически и никаких усилий не надо.

(50) АПК <> Сонар
В Сонаре есть не все, что есть в АПК.
И наоборот тоже верно.

(52) Статический анализ это вообще просто гигиена разработки :)

(43) Оно конечно правильно, но не просаживаться же до нелогичности пробела

(48) Согласен, и повышается его нечитабильность

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

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

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

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

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

(54) Так почитай стандарты, там написано, зачем нужен пробел в начале комментария.

(59) Не написано
7.3. Большие комментарии или комментарии к фрагменту кода пишутся перед комментируемым кодом в отдельной строке. Текст выравнивается по левой границе комментируемого фрагмента. Между символами комментария "//" и текстом комментария должен быть пробел.
Пруф https://its.1c.ru/db/v8std/content/456/hdoc

Про комментарии...
Обратите внимание - в том же vscode при комментировании по горячей кнопке тоже ставится пробел...
Общепринято в мире, наверное

(55) не-а. При перямых руках эффект обратный - с упрощением кода повышается его читаемость

(60) Ну вот зачем ты так, я же человека стандарты читать хотел научить :)) Но если подумать, то таки в п.3 написано, зачем :)

(57) "разбивка на дополнительные переменные,на самом деле,только усложняет код"

Это если переменные обозваны абы как и находятся абы где. Нормальный код читается сверху вниз и один раз (ибо понимается с первого раза). А сложное логическое условие заставляет останавливаться и перечитывать его несколько раз

(61) и с точки зрения читабельности этот пробел понятен

(57) "лучше всего идет через описание типов и его метод СодержитТип"

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

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

(67) Когда комментарий начинается не с пробела, скорее всего его нужно убрать из кода :))

(58) в (63) uоворилось, что там есть ответ ЗАЧЕМ, это надо сделать вроде как ) а не указание что это надо сделать.

(67) //лишние *символы //в $$начале %/слова №/затрудняют &чтение /*текста

(69) Чо ты такой серьёзный? Если не понял, это же шутка была в (63) :)) А если понял, тогда к чему вопрос?

(70)разве? Не заметил.

(72) чё, в самом деле что ли?

(71)Триггирюсь на фразу "если подумать" ищу подвох каждый раз))))
Раз уж все такие умные, вот пример отделаю блоки текста https://take.ms/bbcEV комментарии очевидные, но они позволяют сразу перейти в нужной области вывода документа, то есть служат якорями а не комментариями. Это ОК или фу фу фу?

(74) Ну так если пробел поставить перед текстом комментария, что страшного случиться? Я вот уже на автомате пробел ставлю, и в шаблонах текста во всех пробел забил.

(74) серьёзного ничего не произойдёт. Но на чтение комментария без пробелов уйдёт больше времени. Потратил времени маленько здесь, маленько тут, маленько там, и вот уже время разработки значительно выросло

(74) это уже не комментарии получаются, а читающие будут считать их за комментарии

(77)Так и что делать? в хорошем коде не должно быть такого?