How can I improve the Cognitive Complexity in my code?
I have a method which has while loop and inside that lot of IF ELSE blocks, I tried to remove IF ELSE with SWITCH Statements but no improvement with Cognitive Complexity as per SONAR cube analysis.
This is my existing code:
while (this.moveNextDelegate(fileLineEnumerator))
{
var line = fileLineEnumerator.Current;
var recordType = GetRecordType(line); // This Method returns the type of record
if (recordType == "1")
{
headerId++;
fileHeader = line; // Here fileHeader is being used in downsteam code flow - line 19
// some custom logic - deleted
}
else if (recordType == "5")
{
batchHeader = line; // Here batchHeader is being used in downsteam code flow - line 19
isRepeativeRecord = false;
}
else if (recordType == "6")
{
batchHeaderId = // some custom logic - deleted
// Here batchHeaderId is being used in downsteam code flow - line 35
detailId++;
isFlag = false;
isRepeativeRecord = true;
// some custom logic - deleted
}
else if (recordType == "7" && !isFlag)
{
addendaId++;
detailRecordsExist = true;
// some custom logic - deleted
}
currentIndex++;
}
My new code using Switch statement - but still there is no improvement with complexity
while (this.moveNextDelegate(fileLineEnumerator))
{
var line = fileLineEnumerator.Current;
var recordType = GetRecordType(line);
switch (recordType)
{
case "1":
{
headerId++;
fileHeader = line;
// some custom logic - deleted
break;
}
case "2":
{
batchHeader = line;
isRepeativeRecord = false;
break;
}
case "6":
{
// some custom logic - deleted
break;
}
case "7":
{
if (!isFlag)
{
// some custom logic - deleted
}
break;
}
case "8":
{
if (!isFlag)
{
// some custom logic - deleted
}
break;
}
}
currentIndex++;
}
Switching from
if/else
toswitch
statement can be a good idea, don't discard that from final code, but will not reduce complexity because the "if/else/switch" statement still exists in the body with the othersif
statements.To avoid sonar complexity, write more simple methods, each with a single goal, it are better to test and have less logic to understand.
https://en.wikipedia.org/wiki/SOLID
Looking to your code, you can extract some methods, I did some refactor to reduce complexity, as you can see in the list bellow:
Looks like you are writing a reader, so if you have a lot of readers/steps like reading header, body, footer of file, with details and recursive content you can separate each fragment in a separate class like
HeaderReader
,BodyReader
,DetailReader
, but this can be a over engineering, so you will decide if worth it.With separate readers you can group in a list of
IYourCustomFileFragmentReaderInterface
, callTest()
and if true then call theRead
method.