Новые правила "Одинаковые условия в последовательности вида Если (....) ИначеЕсли (....) ИначеЕсли (....)" и "Одинаковый код во взаимоисключаемых ветках ветвления кода"

sonar-bsl-plugin

#9

Слушай. А ведь реально глазами фиг увидишь.


#10

Третья стрелка явно лишняя.


#11

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


#12

одно дело, когда вы осознанно добавляете такую денормализацию с целью улучшения читабельности кода. помечаете его как won’t fix (с комментарием) и идете дальше.
другое дело, когда это результат копи-паста и это вероятный баг, а не просто code smell.


#13

Баг - это когда работает не правильно, а не когда повтор кода.
Когда повтор кода - или говногод или так задумано.

Как вы себе представляете код, который переводит стейт-машину, допустим, из 5 условий, полностью раскрывающих СДНФ в 10 возможных состояний. Очевидно, что там неизбежны повторы. И очевидно, что уход от СДНФ приведет к меньшей читабельности кода (а иногда к большему количеству ошибок). И опять же, писать напротив каждого не уникального присвоения нового состояния комментарий, что оно не уникально, будет мешать чтению кода.

Мое мнение - именно эта часть правила слишком техногенна.

PS. Про стейт машину не надо начинать холивар про матрицы состояний :wink:


#14

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

машина состояний - это все же очень специфичный кейс для среднестатистического алгоритма 1с-ника.

я предлагаю вообще не холиварить :slight_smile: неугодное правило всегда можно отключить в собственной настройке профиля качества.


#15

Регистр правил - вполне себе хороший паттерн, попривыкли свитчить.


#16

Абсолютно согласен.
Копипаст часто приводит к различным ошибкам.
И дублирование кода самая мелкая из этих проблем.


#17

Новые красивые срабатывания, которые глазами не увидишь :slight_smile:


#18

Аж 5 раз код дублируется!


#19

Или еще немалый блок кода


#20

В чем конкретно тут major проблема? Одинаковые версии тут не для всех веток используются, что на мой взгляд вполне приемлемо. Хотелось бы увидеть ваш вариант рефакторинга :slight_smile:

Ладно бы это не 1С был, а что-то вроде C++, где switch + fallthrough есть.


#21

Т.е.дублирование 4-х не самых простых строчек кода в соседних условиях тебя не напрягает?


#22

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


#23

Кстати, интересно, почему сработало 5 раз, а не 6. Вроде в ветке с ИмяКласса = "ЖурналыДокументов" тоже есть стандартные реквизиты.


#24

@Vladislav_Moroz есть и по 6 вхождений :slight_smile:

ты же про этот скриншот, верно? или другой?

image


#25

Да, речь про этот скриншот. Хотя, я согласен с @Infactum, что вызовы методов не должны считаться копипастой.


#26

два, три, десять, двадцать одинаковых дублей вы не считаете проблемой ? :frowning:

копипаст в коде - зло практически всегда, и не нужно его оправдывать.

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

Такая статическая проверка очень полезна во многих языках, в т.ч. и в 1С.


#27

сюда же 2, 3, 10, 20 одинаковых вызовов методов.


#28

Никита выше уже писал про такое