|
|
|
|
| ||
| 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 | |||
| Конструктор1С 46 - 18.10.20 - 10:47 | |||
| Надо работать 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 | |||
| Конструктор1С 70 - 19.10.20 - 10:12 | (67) //лишние *символы //в $$начале %/слова №/затрудняют &чтение /*текста | ||
| Стаканов 71 - 19.10.20 - 10:18 | |||
| 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)Так и что делать? в хорошем коде не должно быть такого? |
|
|
Список тем форума |
Рекламное место пустует
Рекламное место пустует
TurboConf — расширение возможностей Конфигуратора 1С
ВНИМАНИЕ! Если вы потеряли окно ввода сообщения, нажмите Ctrl-F5 или Ctrl-R или кнопку "Обновить" в браузере.