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

В следующем релизе будет новое интересное правило

Дублируются условия в разных ветках оператора “Если”.

Часто это является последствиями неверного или незаконченного “копипаста”.

Примеры неверных условий

Если Перем1 = Перем2 Тогда
    А = 1;
ИначеЕсли Перем1=Перем2 Тогда
    Б = 1;
КонецЕсли;
Если УсловиеВыполнено() Тогда
    А = 1;
ИначеЕсли Функция2(Значение1, Значение2) Тогда // ошибка
    Б = 1;
ИначеЕсли Функция2(Значение1, Значение2) Тогда // ошибка
    В = 1;
КонецЕсли;

или более сложный вариант дублирования

Если ПервоеУсловиеВыполнено() Тогда
    А = 1;
ИначеЕсли Перем1 = Перем2 Тогда // ошибка
    Б = 1;
ИначеЕсли ТретьеУсловиеВыполнено() Тогда
    В = 1;
ИначеЕсли Перем1 = Перем2 Тогда // ошибка
    Г = 1;
КонецЕсли;

Примеры срабатывания из типовых конфигураций

2 Симпатий

И еще одно правило начинает свою работу

Ух как 1С-ники любят копипаст

СППР - Справочник Задачи, форма ГрупповоеСозданиеЗадач

В методе аж 3 варианта (Если, ИначеЕсли и неявное Иначе) и везде одинаковый результат :slight_smile:

Выше результат по новому правилу “Одинаковый код во взаимоисключаемых ветках ветвления кода”

Или вот такой вот вариант :slight_smile:

Для оператора ?(Условие, Выражение1, Выражение2)

1 Симпатия

Или намного более сложный пример, который глазами нелегко заметить

1 Симпатия

Еще веселенькие условия, приводящие к одному результату :slight_smile:

Еще большие совпадающие куски кода

и еще

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

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 есть.