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

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

1 Симпатия

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

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

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

2 Симпатий

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

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

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

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

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

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

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

2 Симпатий

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

1 Симпатия

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

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

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

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

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

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

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

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

2 Симпатий

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

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

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

image

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

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

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

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

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

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

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