Cyclomatic complexity there are 31, where did it come from?

Question

I am developing an application that extracts data from an Excel file (I do not have access to the actual database), and I wrote a method that has a single function to extract data from an Excel spreadsheet, as shown below.

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; } 

I launched Code Analysis (I am using Visual Studio 2012 and developing in .NET 4.5) and I have CA1502: Avoid excessive complexity (copied below). As a junior developer (I'm 17), I tried to learn more about this using MSDN, but I'm a bit puzzled by why I have cyclic complexity of 33.

CA1502

Avoid excessive complexity

'Extraction.ExtractSMData(List<Range>)' has a cyclomatic complexity of 33. Rewrite or reorganize the method to reduce complexity to 25.

Core.Extraction.cs:104

I can see my quick ifs ( condition ? if_true : if_false , what are they called?) That this might be bad, but I can still see only 5.

UPDATE

Cyclomatic complexity is now 33 ...

If I comment on entity.IncidentNumber = Row.get_Range("K1").get_Value(); , the complexity will be 32. I thought that get_Range() and get_Value() were one at a time, but everything is fine ...

If I comment on entity.RepairedOnTime = Row.get_Range("W1").get_Value().ToString().ToLower() == "no" ? false : true; entity.RepairedOnTime = Row.get_Range("W1").get_Value().ToString().ToLower() == "no" ? false : true; , the difficulty will be 28 ...

get_Range() , get_Value() , quick-if is 3, do ToString() and ToLower() count?

Answer 1

I calculate the complexity of the method itself, foreach , and two conditional statements - 4. If each of 13 calls to get_Range costs +1 complexity, and each of 13 calls to get_Value costs +1 complexity, then the total complexity will be up to 30 (another 1 short, but close). I'm not sure why these two functions can increase complexity, but this seems plausible.

Try removing one of the lines calling get_Range and get_Value , and see if the cyclic complexity drops to 29.

Answer 2

The return type is IEnumerable, so do not use a list. This makes IENumerable useless. Otherwise, you are not using Lazy-Evaluation See: http://blogs.msdn.com/b/pedram/archive/2007/06/02/lazy-evaluation-in-c.aspx

it’s better to use return returns, then:

 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; } } 

You can also write it like this:

 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()) }; } }