Цикломатическая сложность равна 31, откуда это взялось?

Я разрабатываю приложение, которое извлекает данные из файла Excel (у меня нет доступа к фактической базе данных), и я написал метод, единственная функция которого состоит в извлечении данных из электронной таблицы Excel, как показано ниже. .

private IEnumerable<SMEntity> ExtractSMData(List<MSExcel.Range> SMData)
{
    List<SMEntity> SMEntities = new List<SMEntity>();

    foreach (MSExcel.Range Row in SMData)
    {
        SMEntity entity = new SMEntity();
        entity.IncidentNumber = Row.get_Range("K1").get_Value();
        entity.SRNumber = Row.get_Range("L1").get_Value();
        entity.SRCategory = Row.get_Range("M1").get_Value();
        entity.SiebelClientCall = EntityConversions.DateTimeConversion(Row.get_Range("N1").get_Value());
        entity.SiebelOpenedDate = EntityConversions.DateTimeConversion(Row.get_Range("O1").get_Value());
        entity.IncidentOpenDate = EntityConversions.DateTimeConversion(Row.get_Range("P1").get_Value());
        entity.PickedUpBeforeClient = Row.get_Range("Q1").get_Value().ToString().ToLowerCase() == "no" ? false : true;
        entity.OutageStartTime = EntityConversions.DateTimeConversion(Row.get_Range("R1").get_Value());
        entity.DetectionPoint = EntityConversions.DateTimeConversion(Row.get_Range("S1").get_Value());
        entity.SecondsToDetection = EntityConversions.ConvertDetectionTimeToInt(Row.get_Range("T1").get_Value());
        entity.OutageEndTime = EntityConversions.DateTimeConversion(Row.get_Range("U1").get_Value());
        entity.MTTR = EntityConversions.ConvertMTTRStringToInt(Row.get_Range("V1").get_Value());
        entity.RepairedOnTime = Row.get_Range("W1").get_Value().ToString().ToLowerCase() == "no" ? false : true;
        SMEntities.Add(entity);
    }

    return SMEntities;
}

Я запустил анализ кода (я использую Visual Studio 2012 и разрабатываю в .NET 4.5), и у меня есть CA1502: Avoid excessive complexity (скопировано ниже). Будучи младшим разработчиком (мне 17 лет), я пытался узнать больше об этом с помощью MSDN, однако я немного озадачен тем, почему у меня цикломатическая сложность 33.

CA1502

Избегайте чрезмерной сложности

'Extraction.ExtractSMData(List<Range>)' имеет цикломатическую сложность 33. Перепишите или реорганизуйте метод, чтобы уменьшить сложность до 25.

Core.Extraction.cs:104

Я вижу с моими быстрыми если (condition ? if_true : if_false, как они называются?), что это может быть плохо, но я все еще вижу только как 5.

ОБНОВЛЕНИЕ:

Цикломатическая сложность теперь составляет 33...

Если я закомментирую entity.IncidentNumber = Row.get_Range("K1").get_Value();, сложность станет 32. Я думал, что get_Range() и get_Value() по одному, но ладно...

Если я закомментирую entity.RepairedOnTime = Row.get_Range("W1").get_Value().ToString().ToLower() == "no" ? false : true;, сложность станет 28...

get_Range(), get_Value(), быстро, если 3, считать ли ToString() и ToLower()?


c# code-analysis fxcop cyclomatic-complexity
person Jake Hendy    schedule 06.12.2012    source источник
comment
Ваши быстрые если называются условным оператором. Хотя мне нравится, как вы используете фразу «быстро, если». Это аккуратно. ;)   -  person J. Steen    schedule 06.12.2012
comment
@alexh Это НЕ называется тернарным оператором - это условный оператор и a< /b> тернарный оператор   -  person Andras Zoltan    schedule 06.12.2012
comment
Цикл также добавляет цикломатической сложности (это не циклический, он цикломатический).   -  person Oded    schedule 06.12.2012
comment
? как говорит Дж. Стин называется conditional operator. Это тернарный оператор, а не тернарный оператор. Хотя это единственный тернарный оператор в языке. :)   -  person Sani Singh Huttunen    schedule 06.12.2012
comment
bool x = Row.get_Range("W1").get_Value().ToString().ToLowerCase() == "no" ? false : true; можно заменить на bool x = Row.get_Range("W1").get_Value().ToString().ToLowerCase() != "no", что не является обязательным оператором, поэтому вы можете немного уменьшить сложность.   -  person petro.sidlovskyy    schedule 06.12.2012
comment
@Andras Zoltan, спасибо за точность: в моей стране мы используем троичное слово для определения этого оператора. это ошибка   -  person AlexH    schedule 06.12.2012
comment
@AlexH эй - я думаю, что меня (и других) можно обвинить в определенной доле педантизма в этом отношении. Точно так же, как разделение инфинитива — это плохая грамматика английского языка, но почти все это делают! Однако на SO я думаю, что общий консенсус заключается в том, чтобы попытаться придерживаться собственных имен вещей; и tbh это не то, чтобы я или кто-либо еще никогда не совершал ту же ошибку :)   -  person Andras Zoltan    schedule 06.12.2012
comment
Ааа. Я понимаю. спасибо :) Поиск в Google? оператор, к сожалению, мало что дает. Может быть, мне нужно больше кофе, когда я ищу, потому что? оператор тоже не помог!   -  person Jake Hendy    schedule 06.12.2012
comment
@petro.sidlovskyy Я об этом не подумал. Спасибо!   -  person Jake Hendy    schedule 06.12.2012
comment
Кроме того, в Google можно найти ?: оператор, так как двоеточие является частью оператора. знак равно   -  person J. Steen    schedule 06.12.2012
comment
@ J.Steen Я понял, что... Плохо! Та :)   -  person Jake Hendy    schedule 07.12.2012
comment
Вы смотрели на Ил?   -  person Mike Cowan    schedule 07.12.2012

Ответы (2)


arrow_upward
1
arrow_downward

Я вычисляю сложность самого метода, foreach и двух условных операторов как 4 всего. Если каждый из 13 вызовов get_Range стоит +1 сложности, а каждый из 13 вызовов get_Value стоит +1 сложности, то общая сложность составит 30 (по-прежнему 1 короткий, но близкий). Я не уверен, почему эти две функции могут увеличивать сложность, но это кажется правдоподобным.

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

person Mike Cowan    schedule 06.12.2012
comment
FxCop работает на сгенерированном IL-коде. Я предполагаю, что компилятор вставляет некоторый код взаимодействия COM для каждого вызова (например, проверка на нуль). Есть 28 вызовов COM (13xget_Range, 13xget_value, GetEnumerator, GetNext), 2x ?: + foreach = 31 - person adrianm; 06.12.2012
comment
Проверьте обновление, ребята. Я тоже так думал, Адриан, но сейчас ему 33 года! - person Jake Hendy; 07.12.2012

arrow_upward
-1
arrow_downward

Ваш возвращаемый тип — IEnumerable, поэтому не используйте список. Это делает IENumerable бесполезным. В противном случае вы не будете использовать ленивую оценку См.: http://blogs.msdn.com/b/pedram/archive/2007/06/02/lazy-evaluation-in-c.aspx

лучше использовать доходность, затем:

private IEnumerable<SMEntity> ExtractSMData(List<MSExcel.Range> SMData)
{
    foreach (MSExcel.Range Row in SMData)
    {
        SMEntity entity = new SMEntity();

        entity.IncidentNumber = Row.get_Range("K1").get_Value();
        entity.SRNumber = Row.get_Range("L1").get_Value();
        entity.SRCategory = Row.get_Range("M1").get_Value();
        entity.PickedUpBeforeClient = !Row.get_Range("Q1").get_Value().ToString().ToLowerCase() == "no"
        entity.RepairedOnTime = !Row.get_Range("W1").get_Value().ToString().ToLowerCase() == "no"

        entity.SiebelClientCall = EntityConversions.DateTimeConversion(Row.get_Range("N1").get_Value());
        entity.SiebelOpenedDate = EntityConversions.DateTimeConversion(Row.get_Range("O1").get_Value());
        entity.IncidentOpenDate = EntityConversions.DateTimeConversion(Row.get_Range("P1").get_Value());
        entity.OutageStartTime = EntityConversions.DateTimeConversion(Row.get_Range("R1").get_Value());
        entity.DetectionPoint = EntityConversions.DateTimeConversion(Row.get_Range("S1").get_Value());
        entity.OutageEndTime = EntityConversions.DateTimeConversion(Row.get_Range("U1").get_Value());


        entity.MTTR = EntityConversions.ConvertMTTRStringToInt(Row.get_Range("V1").get_Value());
        entity.SecondsToDetection = EntityConversions.ConvertDetectionTimeToInt(Row.get_Range("T1").get_Value());


        yield return entity;
    }

}

Вы также можете написать это так:

private IEnumerable<SMEntity> ExtractSMData(List<MSExcel.Range> SMData)
{
    foreach (MSExcel.Range Row in SMData)
    {
        yield return new SMEntity 
        {

            IncidentNumber = Row.get_Range("K1").get_Value(),
            SRNumber = Row.get_Range("L1").get_Value(),
            SRCategory = Row.get_Range("M1").get_Value(),
            PickedUpBeforeClient = !Row.get_Range("Q1").get_Value().ToString().ToLowerCase() == "no"
            RepairedOnTime = !Row.get_Range("W1").get_Value().ToString().ToLowerCase() == "no"

            SiebelClientCall = EntityConversions.DateTimeConversion(Row.get_Range("N1").get_Value()),
            SiebelOpenedDate = EntityConversions.DateTimeConversion(Row.get_Range("O1").get_Value()),
            IncidentOpenDate = EntityConversions.DateTimeConversion(Row.get_Range("P1").get_Value()),
            OutageStartTime = EntityConversions.DateTimeConversion(Row.get_Range("R1").get_Value()),
            DetectionPoint = EntityConversions.DateTimeConversion(Row.get_Range("S1").get_Value()),
            OutageEndTime = EntityConversions.DateTimeConversion(Row.get_Range("U1").get_Value()),


            MTTR = EntityConversions.ConvertMTTRStringToInt(Row.get_Range("V1").get_Value()),
            SecondsToDetection = EntityConversions.ConvertDetectionTimeToInt(Row.get_Range("T1").get_Value())
        };
    }
}
person CSharpie    schedule 06.12.2012
comment
Ленивая оценка хороша для простых вещей в памяти. Имея дело с внешними зависимостями или другими сложными вещами, вам лучше иметь возможность обрабатывать ошибки. Потребитель IEnumerable не должен иметь дело с COMException, SqlExcepton, IOException,... на основе деталей реализации метода? - person adrianm; 06.12.2012
comment
Исходные данные уже находятся в памяти, поскольку функция требует списка в качестве параметра. Думаю, здесь дело в личных предпочтениях. Если он хочет сбросить все строки сразу или получить некоторые до того, как произойдет возможная ошибка. - person CSharpie; 06.12.2012