1с как уменьшить когнитивную сложность
Наш linter указывает максимальную когнитивную сложность 15, поэтому я должен уменьшить это по стандартам, которые мы соблюдали.
Может ли кто-нибудь предложить альтернативное решение этой части кода? Или оставить его таким, как это приемлемо, несмотря на слишком высокую сложность?
Я знаю, что это может быть личное мнение, но я ищу подлинные решения или ответы от людей, у которых были подобные ситуации раньше.
EDIT: я не могу получить доступ к большому количеству библиотек и пакетов из машины dev, над которой я работаю. У меня есть доступ к некоторым (слишком много, чтобы список), поэтому, пожалуйста, обратите внимание на это, прежде чем предлагать использовать один.
Вы можете найти рекурсивное решение. Это, возможно, менее читаемо, но имеет гораздо меньший уровень вложенности, что снижает степень сложности:
Первоначальный вызов выглядит следующим образом:
Вот решение на основе итератора.
может использоваться для создания кросс-продуктов по запросу
теперь с двойным интерфейсом Iterable/Iterator можно альтернативно использовать как
Решение Google Guava
Как только ваши данные упакованы в List> , вы можете использовать n-ary Cartesian Product, сохраняющий порядок элементов (лексикографический), реализованный в Google Guava.
Pure Java half-solution
Вот быстрое решение с жестко закодированными значениями для 3 списков, которые потенциально могут быть обобщены без чрезмерного количества штрафа за сложность. В основном он делает некоторые аккуратные (ака трудно следовать) вычислениям индекса.
Чистое Java-решение - прекрасный пример того, что для того, чтобы сохранить метрику счастливой, мы фактически увеличили сложность и удобство обслуживания.
Весь этот расчет индекса является довольно ужасным, и немногие идут, чтобы получить право. Это, скорее всего, будет стоить штраф в общем решении, так как потребуется итерация. Другие решения, которые я нашел в Интернете (в том числе рекурсивные и функциональные), не яснее, чем пучок вложенных циклов.
Придумайте здесь Декартовы подпрограммы продукта будут более сложными (даже если они будут оценивать более низкую сложность), чтобы понять.
Программное обеспечение должно основываться на абстракциях, а использование открытой, хорошо продуманной сторонней зависимости делает всю проблему ухоженной.
Запустил SonarQube, спасибо Олег Тымко и его статье https://infostart.ru/1c/articles/1089670/
Все сработало, все показало. Но два момента не могу даже понять.
1. Критическая ошибка Общий модуль недопустимого типа (CommonModuleInvalidType). Модуля два: серверный и клиентский, ругается на оба. Как надо оформить модуль, чтобы ругаться перестало? На ИТС прочитал, все вроде так, но тем не менее "Недопустимый тип". Как-то напрягает.
2. Когнитивная сложность (CognitiveComplexity). Это не ошибка, а Дефект кода, но так же критический.
Уменьшите когнитивную сложность "ПередЗаписью" с 64 до 15. Это она хочет чтобы я одну процедуру разбил на кучу мелких? Да, там много проверок, но если их тупо дробить, то лучше не станет, как мне кажется. С этим вообще кто-то заморачивается или забивает? В типовых очень любят дробить. Пока до сути доберешься, вспотеешь. Или так и надо?
2 вопрос - это так, недоумение. А вот 1 реально напрягает.
Может кто чего подскажет умного.
Со вторым вопросом реальный напряг. Ладно бы он ругался на сложные вложенные условия. Но он же тупо ругается даже на количество последовательных "Если. "
Реально есть люди, для которых сложно понять 15 последовательных одиночных Если?
Причем, по нормальным правилам записи объекта через Попытку и проверку активности транзакции - уже тратится порядка 4-х пунктов "когнитивной сложности". И как теперь записывать объекты?
Это какая-то паранойя.
Это все ведь вроде настраивается. Можете увеличить когнитивную сложность
Выделять нужные действия в разные функции это норм. Так легче читать, и понимать что делает та или иная процедура.
(3) Да, согласен, надо и верно выделять куски кода в отдельные процедуры, сам за это ратую!
Но как доказать что-то заказчику, который твой код прогоняет через Сонар и не принимает работу из-за "когнитивная сложность > 15"?
Народ, не дайте зачахнуть.
Общий модуль недопустимого типа. Так ругается и Сонар и АПК.
Как сделать общий модуль допустимого типа? Модуль клиентский. Обозвал его "канонически" ОбщегоНазначенияГлобальный. Внутри ОДНА процедура и все. Оно все работает, а вот проверку не проходит. Куда копать?
(5) Так вы так и не рассказали - какие галочки у вас стоят и какие директивы препроцессора используются в коде модулей. А догадываться мы не умеем.
(9) Где тогда слово Клиент в наименовании модуля? Кто-то утверждал, что все сделано согласно рекомендаций ИТС.
(10) Из ИТС Пример наименования модуля ОбщегоНазначенияКлиент (или ОбщегоНазначенияГлобальный)
Делал Общие.ОбщиеМодули.ОбщегоНазначенияКлиентГлобальный, пофиг. Та же ошибка: Общий модуль недопустимого типа. Т.е. дело не в названии, судя по всему. А вот в чем, не догоняю :-(
(12) А это случайно не в расширении?
Простите, а какова когнитивная сложность модуля расходной накладной в ут 11.4? Так чтобы понимать примерно каков эталон.
(6) Сонар считает, что предел 15. Каждое условие (включая И или ИЛИ) прибавляет +1. Цикл аналогично. Каждый вложенное условие прибавляет +2. Максимальный уровень вложенности - 4.
Если в итоге больше 15, то типа это плохо. Дефект когда, причем критический.
(14) Правила проработал самым тщательным образом. Но так и не догнал, почему мои общие модули не проходят проверку. Ни клиентский, ни серверный. Как еще определяется тип, кроме как галочками. Галочки стоят.
По когнитивной сложности скажу лишь, что идеальная функция это 3-5 строк. Она понимается почти мгновенно.
Но с большим числом вызовов тоже есть засада. Во первых, их будет чуть меньше чем тех самых одиночных "Если". Во-вторых, мало кто утруждает себя придумыванием осмысленных названий соответствующих содержимому.
Проблема сонара и вообще всех автоматических статических анализаторов кода, что они пока не могут адекватно оценить когнитивную сложность при использовании неудачных наименований вызываемых функций.
Сложность где "сложно добраться до сути" тоже не всегда сложна. Бывает там 3-5 вызовов, необходимых для проброса с клиента, до системного модуля. Непривычно тут бывает только семерочникам, и тем кто пишет функции на клиенте в 2к+ строк. Случаи неоправданного усложнения встречаются, но частенько "неоправданность" это "нашей компании это не нужно, зачем это в тиражной типовой?".
(15) Наверно не соглашусь про идеальную функцию. Понимается мгновенно? Так кода не видно. Т.е. что должно делаться видно, а вот что реально делается, надо проваливаться, проваливаться и проваливаться.
(17) Если надо "проваливаться", то значит это плохая функция. Либо название не соответствует действию, либо действие не соответствует названию.
(18) Название соответствует действию. Но действие делается неверно. И вот тут-то и начинается танец с бубном. Потому как реально выполняемый код появляется на третьем, а то и четвертом уровне вложения. А все выше правильно названные функции. Когнитивная сложность низкая, все хорошо, но искать ошибку вдруг оказывается очень тяжело. Сам с таким сталкивался.
(19) А в чем сложность? В незнании архитектуры решения? Так это до первого раза. Потом наоборот уже станет ясно, где локализована ошибка. "Проваливание" ведь не в пустоту идет, а в модуль который выполняет какой-то специфический расчет или иные действия. Вот туда и надо идти в первую голову, если проблема есть.
(20) В принципе да. Зная архитектуру решения, все наверно проще. Но по той же типовой бух от 1С есть документация разработчика? И тогда ситуация несколько иная. В процедуре 3 строки, в каждой функция. проваливаешься в qeyrwb.? а там тоже три строки из функций, проваливаешься дальше, а там тоже только вызов функций. И все названы красиво и правильно, но до кода добраться - это небольшой квест. А с учетом того что функций сотни, однозначно определить визуально без "проваливания", что надо именно тут смотреть, несколько затруднительно. Хотя конечно сваливать весь код в одну функцию тоже неправильно, но и выделять одно условие в функцию, которая существует только локально - тоже наверно перебор.
Но этот вопрос как-то больше технологический.
Может уважаемый Артано подскажет, что у меня не так с общими модулями? Стоит одна галка "Клиент", внутри буквально три функции. И Сонар и АПК дают "Общий модуль недопустимого типа". Уже всю голову сломал.
(21) Здесь вопрос в методике поиска. Сначала чтением логов или глазами в отладчике ошибка локализуется, а потом, зная где она может воспроизвестись, или хотя бы зная например свойство объекта, которое должно поменяться, можно легко зайти в нужную функцию.
по сабжу топика. Галки должно быть две (обычное и управляемое приложение). В названии модуля должно быть слов Клиент (например ОбщегоНазначенияИмениНуралиеваБорисаГеоргиевичаКлиент). Может быть еще директивы компиляции проверяются внутри модуля. Но чтобы сказать точно лучше найти в справочнике того же АПК эту проверку и посмотреть код проверки.
(22) Согласен. Когда ошибка локализована, уже есть от чего оттолкнуться. Хуже, когда ошибки не идентифицируется, просто неправильно считает. Вот тогда можно потанцевать с бубном от души.
Блин. Про заглянуть в код в АПК я даже не подумал :-). Спасибо за наводку. По коду уж соображу что ей не нравится.
(22) Спасибо Артано. Залез в код и расковырял ошибку "Недопустимый тип". Надо переключить в Параметрах Управляемое и обычное приложение. И появятся галочки, которые не видны в режиме Управляемое приложение. Проставить галочки напротив Обычное приложение, потом можно переключится обратно и проверки проходят. С учетом того, что у меня нет форм для обычного приложение и работает исключительно в режиме управляемого, то как-то смахивает на бред. Но . мучавший меня вопрос наконец-то решился.
if ('function' == typeof val) return val.length === 0 ? Как функция, которая не принимает аргументов, «пуста»?
if (val.toString == toString) эта проверка кажется неправильной. Хотя бы потому, что вы можете просто использовать instanceof для проверки Map, Set и File.
Итак, давайте проанализируем, где в первую очередь возникают значения высокой сложности из вашего метода:
Если вы посмотрите на комментарии, которые я добавил, вы легко увидите, где находится наиболее проблематичный код, связанный с цикломатической сложностью. Это также относится к удобочитаемости кода человеком.
Таким образом, один простой шаг к повышению удобочитаемости и одновременному снижению когнитивной сложности — это поиск вариантов « раннего возврата ».
Чтобы проиллюстрировать это, я просто инвертировал оператор *if (val.toString == toString)", чтобы немедленно вернуть false, если *val.toString != toString":
Теперь последний оператор switch может выполняться вне оператора if, и мы уменьшили уровень вложенности на единицу. Благодаря этому простому изменению когнитивная сложность теперь снизилась до 14 вместо 17.
Затем вы могли бы пойти еще дальше и изменить последний оператор case, извлекая возвращаемое значение в переменную и либо извлекая отдельный метод из блока кода. Это еще больше уменьшит сложность метода isEmpty().
И помимо извлечения метода вы также можете использовать декларативный подход и использовать, например, метод Array find() , который еще больше уменьшит когнитивную сложность.
Чтобы проиллюстрировать идею, я сделал оба:
Это должно снизить когнитивную сложность метода isEmpty() до 8 и всего кода , включая извлеченную функцию checkForComplexTypes() , до 9 баллов сложности .
Примечание. В настоящее время JavaScript не является моим основным языком, поэтому я не могу полностью гарантировать правильность последнего шага рефакторинга.
Коллеги, подскажите пожалуйста способы, которыми можно уменьшить когнитивную сложность метода, средствами Java 8
Хотелось бы какое то элегантное решение, может быть предикатами или лямбдами переделать. Есть какие идеи ?
Здравствуйте, HAXT, Вы писали:
HAX>Коллеги, подскажите пожалуйста способы, которыми можно уменьшить когнитивную сложность метода, средствами Java 8
HAX>Хотелось бы какое то элегантное решение, может быть предикатами или лямбдами переделать. Есть какие идеи ?
Зачем тут лямбды?
Вынести request.getColumns() в локальную переменную наружу цикла.
Тело цикла вынести в отдельный метод.
Строчку формирования List products вынести в отдельный метод.
formatDate вынести в отедьный сервис и туда заинжектить зависимости zoneId/dateFormat, чтобы не приходилось повсюду отдельно протаскивать эти параметры.
Здравствуйте, HAXT, Вы писали:
HAX>Коллеги, подскажите пожалуйста способы, которыми можно уменьшить когнитивную сложность метода, средствами Java 8
HAX>Хотелось бы какое то элегантное решение, может быть предикатами или лямбдами переделать. Есть какие идеи ?
Сильно лучше тут не сделать.
Но:
Я бы сделал extract method того, что внутри цикла. И этот метод бы принимал columns и IndexedDraftOrderDto, а возвращал набор строк, которые добавляются в csvBuilder. Было бы немножко почище, основное мясо бы уже имело поменьше зависимостей и было бы покомпактней. То, что внутри if, возможно бы тоже выделил в методы отдельного класса extractors или convertors, скорее всего статические. И к ним комментарии почему именно так и далее на эти экстракторы можно отдельно тесты писать.
И дополнительно — мне не очень понравилось черти сколько аргументов метода, я бы постарался рефакторнуть даже уровнем выше.
Соответственно в соответствии с SRP выделил бы отдельно:
1) extractor или трансформаторы для каждого варианта столбца;
2) трансформатор для строки целиком
3) трансформатор для всех строк целиком в удобное для преобразования в csv представление. И далее это представление можно реюзать для записи не в CSV, в в другую таблицу, в excel, в Json, в xml — куда угодно. Хоть это сейчас и не надо — но может быть полезно.
4) собственно генератор csv
Но это ИМХО, именно мой вкус — я очень не люблю логику внутри for и когда что то нетривиальное внутри if. Начитался в свое время Макконела на свою голову и выработал привычку, делаю такое на автомате даже не задумываясь. А сам метод не такой уж и монструозный чтоб сильно заморачиваться чем то еще.
Здравствуйте, HAXT, Вы писали:
HAX>Коллеги, подскажите пожалуйста способы, которыми можно уменьшить когнитивную сложность метода, средствами Java 8
Оборачивать всё это дело в лямбды, думаю, не всегда удачная идея. Помимо накладных расходов на конструирование лямбд и их вызов, теряется гибкость в использовании операторов типа if/else, for и т.д. В своих проектах иногда использую следующую конструкцию:
Скрытый текст |
То есть, если в некой конфигурации установлены флаги "foo" и "bar", но не "baz", вывод будет следующим:
Как уменьшить сложность данного фрагмента кода? Я получаю эту ошибку в Sonarqube---> Выполните рефакторинг этого метода, чтобы снизить его когнитивную сложность с 21 до 15 разрешенных.
3 ответа
Немного информации о том, как работает цикломатическая сложность и почему вы должны держать ее на низком уровне
Прежде всего, важно понять, как работает " когнитивная сложность " по сравнению с " цикломатической сложностью ". Когнитивная сложность учитывает сложность, воспринимаемую человеческим мозгом. Вот почему он не просто указывает количество условных путей (упрощенное количество условных операторов плюс 1 для оператора возврата).
С другой стороны, цикломатическая сложность также учитывает вложенные условия (например, if внутри оператора if), что еще больше затрудняет чтение и понимание кода с точки зрения человека.
Так что имейте в виду, что бинарные операции добавляют сложности, но вложенные условия добавляют балл плюс 1 за каждое вложенное условие. Здесь когнитивная сложность будет равна 6, а цикломатическая сложность - только 4 (по одному для каждого условного и одного для обратного пути);
Если вы сделаете свой код более читабельным для человека, например, извлекая методы из строк, содержащих условные выражения, вы добьетесь как лучшей читаемости, так и меньшей цикломатической сложности.
Хотя в предоставленном вами коде нет вложенных условных выражений, я думаю, что важно сначала понять, как работает вычисление цикломатической сложности, и почему рекомендуется поддерживать его на низком уровне.
[TL;DR] Возможный подход к преобразованию кода в менее сложную и более читаемую версию
Давайте сначала посмотрим, как выполняется расчет сложности для вашего кода, как указано в комментариях:
Это может быть отредактированная версия вашего кода, которая (из быстрого ручного подсчета без реального анализа SonarQube) должна снизить когнитивную сложность до 12. (Имейте в виду, что это всего лишь ручной расчет).
С помощью рефакторинга простого метода извлечения также было устранено множество дубликатов (см. Функцию getInfoItem()), что позволяет легко снизить сложность и повысить удобочитаемость.
Честно говоря, я бы даже пошел дальше и реструктурировал ваш код еще больше, чтобы логика проверки пустых элементов и установки значения по умолчанию (здесь пустая строка) при предоставлении сведений об устройстве выполнялась классом устройства или класс деталей устройства, чтобы обеспечить лучшую согласованность данных и логики, которая работает с этими данными. Но поскольку я не знаю остальной части кода, этот начальный рефакторинг должен сделать вас еще на один шаг вперед к лучшей читаемости и меньшей сложности.
Читайте также: