Как уменьшить цикломатическую сложность этого метода c#?

#c# #cyclomatic-complexity

Вопрос:

В настоящее время я занимаюсь проектом для своих классов c#. Наш учитель дал нам некоторые ограничения по метрикам кода, которые мы должны соблюдать, и одним из них является цикломатическая сложность. Прямо сейчас сложность приведенного ниже метода составляет 5, но она должна быть 4. Есть ли какой-нибудь способ улучшить это?

МефодиЙ говорил о:

 private bool MethodName()
    {
        int counter = 0;

        for (int k = 0; k < 8; k  )
        {
            for (int j = 0; j < 3; j  )
            {
                if (class1.GetBoard()[array1[k, j, 0], array1[k, j, 1]] == player.WhichPlayer()) counter  ;
            }

            if (counter == 3) return true;
            else counter = 0;
        }
        return false;
    }
 

Комментарии:

1. Вы можете использовать счетчик int = 0; внутри первого цикла for. Затем вы можете удалить else counter = 0;

2. Напишите какой-нибудь непонятный линк! (На самом деле это может быть то, что нужно сделать здесь.)

3. Более серьезно, стандартный способ решить эту проблему (если только вы не можете упростить код как таковой) — разделить некоторые блоки (например, внутренний цикл for) на отдельные функции.

4. Понятия не имею об этой метрике, но , похоже class1.GetBoard() , она не зависит от обоих j и k , поэтому определенно я бы назвал ее только один раз(!) за пределами циклов for, вместо того, чтобы называть ее 8*3 раз внутри циклов for. То же самое касается player.WhichPlayer() .

5. Одна микрооптимизация может основываться на том факте, что counter на самом деле используется только локально во внешнем цикле; переместите определение и инициализацию на его вершину, тем самым устраняя ветвь else.

Ответ №1:

Я могу завернуть условия, чтобы уменьшить его. Например

 private bool MethodName()
    {
        for (int k = 0; k < 8; k  )
        {
            bool b = true;
            for (int j = 0; j < 3; j  )
            {
                b amp;= class1.GetBoard()[array1[k, j, 0], array1[k, j, 1]] == player.WhichPlayer();
            }

            if (b) return true;
        }
        return false;
    }
 

Комментарии:

1. Преобразование bool в int для уменьшения цикломатической сложности… Это может сработать, но сильно усложняет код. менее ясно. Я бы не стал этого делать, если бы у меня не было очень веской причины.

2. Я имею в виду, технически, вы также могли бы удалить if (counter == 3) return true условие и сделать goto заявление… а ты бы стал?

3. @RodrigoRodrigues, не было времени на чистый код. Нет необходимости в обращении. Цикломатическая сложность в любом случае редко используется в реальных проектах

4. @mandarancza Это логично И. Это означает, что «b» будет истинным только в том случае, если все условия верны. Любая ложная опция в результате принесет вам ложь.

5. @BorisSokolov спасибо за объяснение, это кажется лучшей альтернативой, спасибо

Ответ №2:

Для операции (которая, похоже, только начинается с программирования):

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

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

Для более опытных программистов:

Эта простая проблема напомнила мне об очень известной дискуссии 1968 года между Дейкстрой и другими ребятами на ACM periodic. Хотя я склонен соглашаться с ним в этом вопросе, это был один из очень разумных ответов Фрэнка Рубина.

Фрэнк в основном выступает за то, что «элегантность» может во много раз проистекать из ясности кода, а не из каких-либо других показателей практики. Тогда дискуссия шла о чрезмерном использовании заявления goto на популярных языках того времени. Сегодня дискуссия идет вокруг цикломатической сложности, краткости, ооп или чего-то еще.

Суть в том, что, на мой взгляд,:

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

Вернемся к вопросу

Реализация, представленная в вопросе, получила следующие оценки в моем анализаторе Visual Studio:

Цикл. Комплектация: 5; Ремонтопригодность: 67

Фрагмент, представленный @Boris, получил следующее:

Цикл. Комплектация: 4; Ремонтопригодность: 68

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

Просто для развлечения давайте посмотрим, как будет выглядеть решение, аналогичное тому, которое представил Фрэнк Рубин, использующий страшное goto утверждение:

 private bool MethodName() {
  for (int k = 0; k < 8; k  ) {
    for (int j = 0; j < 3; j  ) {
      if (watheverTestCondition(k, j) is false) goto reject;
    }
    // condition is true for all items in this row
    return true; 
    // if condition is false for any item, go straight to this line
    reject:;
  }
  return false;
}
 

Честно говоря, я думаю, что это наиболее ясная, простая и эффективная реализация для этого. Рекомендую ли я goto в целом в качестве функции кода? нет. Подходит ли это идеально и гладко в данном конкретном случае? ДА. А как насчет показателей?

Цикл. Комплектация: 4; Ремонтопригодность: 70


Бонус

Просто потому, что я не смог бы заснуть, если бы не сказал этого, вот как вы бы реализовали это в реальной жизни:

 obj.Any(row => row.All(watheverTestCondition));
 

Цикл. Комплектация: 1; Ремонтопригодность: 80