Обсудить: Правило - Несколько идущих подряд вложений "Если Условие" без исключений "ИначеЕсли" или "Иначе"

#1

Контекст обсуждения

Пример неверного кода:

Если ИспользуемаяВерсияПлатформыСтаршеИлиРавна("8.3.10") Тогда
    Если ТолькоИзмененные Тогда
        Если ПроверитьВозможностьОбновленияФайловВыгрузки(КаталогВыгрузки, ПутьКФайлуВерсийДляСравнения, ФорматВыгрузки) Тогда

            Какой-то Код();            

        КонецЕсли;
    КонецЕсли;
КонецЕслИ;

более правильно сделать единое условие и одно Если

Если ИспользуемаяВерсияПлатформыСтаршеИлиРавна("8.3.10") 
     И ТолькоИзмененные
     И ПроверитьВозможностьОбновленияФайловВыгрузки(КаталогВыгрузки, ПутьКФайлуВерсийДляСравнения, ФорматВыгрузки) Тогда

            Какой-то Код();            

КонецЕслИ;

Сценарий поведения

  • Правило срабатывает на несколько вложений “Если”
  • Не срабатывает при наличии “ИначеЕсли” или “Иначе”
  • Учесть случай, когда на верхних уровнях нет “ИначеЕсли” или “Иначе”, а на нижних есть
    • Например, для случая ниже правило должно сработать только для первых 2 Если, для последнего Если правило не должно сработать
        Если ИспользуемаяВерсияПлатформыСтаршеИлиРавна("8.3.10") Тогда
            Если ТолькоИзмененные Тогда // здесь работает
                Если ПроверитьВозможностьОбновленияФайловВыгрузки(КаталогВыгрузки, ПутьКФайлуВерсийДляСравнения, ФорматВыгрузки) Тогда // а здесь уже работает

                    Какой-то Код();            

                Иначе
                    Другой Код();            
                КонецЕсли;
            КонецЕсли;
        КонецЕслИ;

PS в стат-анализаторах для разных языков такое правило.
Например, IDEA для Java предлагает такое.

@all Что скажете?

1 Симпатия
#2

Увидел в свежем пулл-реквесте в библиотеке v8runner и вспомнилось/захотелось :slight_smile:

#3

Полагаю, механизм оценки когнитивной сложности в Сонаре уже сейчас может выявлять ловить такое, не?

А также другие техники по упрощению условий: https://refactoring.guru/ru/refactoring/techniques/simplifying-conditional-expressions

1 Симпатия
#4

И следом “булево должно быть первым”.

1 Симпатия
#5

Э-э-э-э, они же все булево!

#6

Оно уже посчитано, а остальные требуют выполнение функции

#7

А что полноценного механизма анализа графа исполнения на предмет его упрощения в сонаре не предусмотрено?

#8

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

1 Симпатия
#9

Ага, хорошее предложение.

“В условии лучше в начало поставить простую переменную с типом Булево, а не сложные выражения” - потенциальное ускорение выполнения

#10

Несколько интересных скриншотов срабатывания правила

image
image.png910×501 30.2 KB

image
image.png894×383 22.9 KB

image
image.png920×453 30.8 KB

image
image.png1004×449 38.4 KB

image
image.png966×297 21.1 KB

image
image.png1004×449 38.4 KB

image
image.png870×274 18.7 KB

#11

Чот сомнительное правило, если честно.
Здесь предостерегают от последовательных логических операторов, а вы их наоборот рекомендуете делать. https://www.sonarsource.com/docs/CognitiveComplexity.pdf

Sequences of logical operators
For similar reasons, Cognitive Complexity does not increment for each binary logical
operator. Instead, it assesses a fundamental increment for each sequence of binary logical
operators. For instance, consider the following pairs:

a && b
a && b && c && d
a || b
a || b || c || d

Understanding the second line in each pair isn’t that much harder than understanding the
first. On the other hand, there is a marked difference in the effort to understand the following
two lines:

a && b && c && d
a || b && c || d
#12

Все верно.
но использование кучи условий вида “Если Первое Тогда Если Второе Тогда Если Третье Тогда” еще хуже.

условия вида “a && b && c && d” уже можно отрефакторить.
ИМХО это уже полегче сделать, чем разбираться при наличии кучи Если.

я об этом также подумал и уже зафиксировал на нашем внутреннем трекере создание нового правила “Слишком сложное условие”

Если в условии (Если, ИначеЕсли и Пока) слишком сложное выражение, которое сложно читать и понимать, тогда выдавать замечание.

Виды сложных выражений:

  • более 3 подусловий (сделать несложно)
  • например, Если ПервоеУсловие И Второе И Третье И Четвертое Тогда
  • слишком много вложенных выражений (тут сложновато)
    • например, Если Функция1(Функция2(Функция3(Парам1, Парам2, Парам3), Парам4), Парам5) = Значение Тогда
  • текст выражения-условий слишком длинный, например, более 80 или 120 символов

    • например,

      Если ПараметрПолноеИмяОбъектаМетаданных <> Неопределено
      И ПользовательскаяНастройкаКД.Параметр = ПараметрПолноеИмяОбъектаМетаданных.Параметр
      И ПараметрПолноеИмяОбъектаМетаданных.Значение <> ПользовательскаяНастройкаКД.Значение Тогда

можем также обсудить.

1 Симпатия
#13

Вот эту ссылку тоже рассмотрите https://refactoring.guru/ru/refactoring/techniques/simplifying-conditional-expressions

#14

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

#15

Эту ссылку уже приложили к описанию правила, которое видно для пользователя на сервере Сонара :slight_smile:

1 Симпатия
#16

@artbear А напомни, плз, у нас (1С и oscript) условия лениво вычисляются?
т.е.

МояСтруктура = Неопределено;
Если ТипЗнч(МояСтруктура) = Тип("Структура") И МояСтруктура.Свойство("МоеСвойство") Тогда
//МойКод
КонецЕсли;

не выдаст ошибки?

#17

Лениво.
10смв

#18

А вот и срабатывания

image
image.png1035×730 49.4 KB

еще

image
image.png961×433 32 KB