8 Replies Latest reply on Jan 13, 2017 3:24 AM by Ian Proudfoot

# Reducing Cyclomatic Complexity - in all cases?

Dear experts
Everything works in my script (hmm, you read about ghosts and twins...). However JsHint.com reports for some long function a Cyclomatic Complexity beyond the acceptable limit - up to 50.

In many cases I could cut this down to about 10 (replacing switch by if/else if and splitting off reasonable chunks into sub-functions).

But for a function like the following (complexity 22) I would need to split off the actions in the if (character ... clauses into separate function with many parameters. IMHO this would be just l'art pour l'art:

The function in question parses a string and acts at particular characters thus expanding it step by step to an evaluative statement:

```#vat_amount = Round(((Sum(Left(3))) + #item + [CELL 5, 5]) * #VAT, 2) {F2};
```

To be evaluated it has to be expanded step by step:

```#vat_amount = Round(((Sum(Left(3))) + #item + [CELL 5, 5]) * #VAT, 2)
#vat_amount = M_Round(((Sum(Left(3))) + #item + [CELL 5, 5]) * #VAT, 2);
#vat_amount = M_Round(((M_Sum(Left(3))) + #item + [CELL 5, 5]) * #VAT, 2)
#vat_amount = M_Round(((M_Sum(17.0, 23.7, 37.9)) + #item + [CELL 5, 5]) * #VAT, 2)
#vat_amount = M_Round(((M_Sum(17.0, 23.7, 37.9)) + 12.34 + [CELL 5, 5]) * #VAT, 2)
#vat_amount = M_Round(((M_Sum(17.0, 23.7, 37.9)) + 12.34 + 56.78) * #VAT, 2)
#vat_amount = M_Round(((M_Sum(17.0, 23.7, 37.9)) + 12.34 + 56.78) * 0.08, 2)
```

The interpretation of the final line is:
User variable #vat_amount will receive the result from the formula. M_Round and M_sum are functions.

The skeleton of my function looks like this:

```function EvaluateStmntsC (text) {
//...
for(indexOfChar = 0; character = localText[indexOfChar]; indexOfChar++) {
if (character.match(/[ (),+\-*\/]/) !== null) {
continue;
} else
if (character == "#") {
// expand the user variable
} else
if (character == "@"){
// expand the constant
} else
if (character.match(/[ABCEFHLMPRST]/) !== null) {
// expand function name (e.g. Sin => Math.sin)
// and check for the number of arguments in the ()
} else
if (character.match(/[0-9]/) !== null) {
// numeric value, just advance behind it
// optional - sign has been skipped already
} else
if (character == "["){
// Expand to cell contentss
} else
if (character == "<") {
// if followed by =, this introduces a vector notation (list of values).
} else
if (character == "{") {
// put the output format aside for later use
//...
} else {
// indicate illegal character (e.g. unknown function)
}
EvaluateStmntsCfinalise (aStatement[j], jAssign, bHasVector, sVarName, fmtString)
} //--- end for indexOfChar, statement worked off
} //--- end for statements
return true;
} //--- end EvaluateStmntsC
```

This skeleton alone result in a complexity of 10. Of course there are some 'intra'-case structures with 1-2 nested if's => leading to a final complexity of 22.
Towards the end I have extracted what is there into a new function, which on it's own has a complexity of 6 and needs quite a number of parameters:

```function
EvaluateStmntsCfinalise (sStatement, jAssign, bHasVector, sVarName, fmtString) {
var kText, bIsOK, sEval, rResult, sFormatted;

kText = StrTrim(localText.substring (jAssign+1));// a vector definition is kept intact within variable
bIsOK = ContainsVector (kText);               // may be the result of a table location
if (bHasVector || bIsOK) {                    //--- store whole list in variable - unformatted!
iLength = sVarName.length;
if (sVarName.indexOf ("<") > 0) {iLength = iLength - 1;} // "#TEST <"  => "#TEST"
sVarName = sVarName.substring (0, iLength);
sVarName = StrTrim(sVarName);               // there may be blanks
WriteFeedback (" Filling variable = "+ sVarName + " with \n >"+kText);
DefineUserVariable (glbl.oCurrentDoc, sVarName, kText); // Fill user var with vector
} else {                                      //--- scalar variable: evaluate, format, store
sEval = "rResult = " + kText;
WriteFeedback (" To evaluate:\n>"+sEval);
try {eval (sEval);}                         // JShint: eval can be harmful.
catch (e) {
MsgErrEvaluation (sStatement, sEval, e.name, e.message);
return false;
}
WriteFeedback (" rResult = "+rResult);
sFormatted = FormatValue (rResult, fmtString, true); // format result
if (glbl.bTempVar) {glbl.sTempVar = sVarName;}       // for direct insertion
DefineUserVariable (glbl.oCurrentDoc, sVarName, sFormatted); // Fill user var
}
}
```

So my question is this:
Is it worth to create a huge number of functions just to reduce the complexity of a somewhat linearly long function?

• ###### 1. Re: Reducing Cyclomatic Complexity - in all cases?

Hi Klaus,

I am not the expert to whom you addressed this question. So, for the real answer, I'll defer until that person answers. Because indeed, I never even heard of "cyclomatic complexity" until today. What I can say, though is that before today I have written unknown thousands of lines of code over many years. At times, this code includes very long functions, containing hundreds or occasionally thousands of lines. I have never seen any environment complain about my cyclomatic complexity. I'd be very interested if someone could actually find fault in your very short function, because if you've done something wrong, I can't see how anything I ever wrote works at all.

Russ

• ###### 2. Re: Reducing Cyclomatic Complexity - in all cases?

Hi Klaus,

Your code is working, right? It's a generall question of how to implement some use cases which are more complex.

As Russ already stated, there are AFAIK no technical problems which such complex code... It doesn't matter if you are writign "Spagetti Code", functional or object oriented code, so far.

> Is it worth to create a huge number of functions just to reduce the complexity of a somewhat linearly long function?

Yes it is, if you have requirements in direction of reuse and maintainable code. Reducing complexity means a higher understanding for other developers.

Markus

• ###### 3. Re: Reducing Cyclomatic Complexity - in all cases?

Experts in my understanding have successfully written many scripts/programs ... eventhough the may not have digested all 7 volumes of Knuth's "The art of computer programming". IMHO You and others on this list belong to this qualification.

According to Wikipedia the term was coined by Thomas J. McCabe, Sr. in 1976. It is not explicitly addressed in (my) books:

[1]    D. Crockford, JavaScript: The good parts: Unearthing the Excellence in JavaScript, 2nd ed. Farnham: O'Reilly, 2013.

[2]    D. Flanagan, JavaScript: Pocket reference, 1st ed. Sebastopol, CA: O'Reilly, 1998.

[3]    S. Koch, JavaScript: Einführung, Programmierung und Referenz, 2nd ed. Heidelberg: Dpunkt-Verl, 1999.

I stumbled across this concept by looking for an JS-checker in the web and endet up on JsHint.com, which is a really useful tool.

IMHO the concept is relevant in software testing to find out how many test cases are needed to traverse all possible paths. But - as Your answer supports - the concept may be over-emphasised in many cases.

My conclusion from this brief discussion: Use it as a measure to think about the current program structure:

• Can something be externalised into an own function - and use this function on more than one place?
• Can specific constructs (such as Swith with falling through cases) be replaced by something else, e.g. logical connections?

For example, from

```function StringToBool (string) { // CC = 16
switch (string.toLowerCase()) {
case "true":  case "yes": case "on":  case "ja":  case "ein": case "oui": case "1": return true;
case "false": case "no":  case "off": case "nein": case "aus": case "non": case "0": return false;
case null: return false;
default: return Boolean(string);
}
} //--- end StringToBool
```

to

```function StringToBool (string) { // CC = 3
var bResult = false, sTrue = ["true", "yes", "on", "ja", "ein", "oui", "1"];
if (string === 1 || IsInArray (sTrue, string.toLowerCase() !== null)) {bResult =true;}
return bResult;
} //--- end StringToBool
```

... the discussion on this 'issue' may have calmed due to better programming languages since the early 70ies, when McCabe put it on the table.

• ###### 4. Re: Reducing Cyclomatic Complexity - in all cases?

Also thanks to Markus.

Of course re-use of code is a good idea - look at the small pieces Ric always provides!

For debugging - step by step walking through - I use more variables thean necessary and also mor steps then necessary.

For example I still prefer

```  fRange = oDoc.Find  (oTxtLoc1, findParams);     // finds variable
if (fRange.beg.obj.ObjectValid()) {             // variable found ?
if (fRange.beg.obj.Unique == iUnique1) {      // same ¶ as the marker ?
if (fRange.beg.offset == oTxtLoc1.offset+1) { // TR =? variable behind marker
oDoc.DeleteText(fRange);                  // Delete the selected Variable
return true;
}
}
}
```

over

```  if (fRange.beg.obj.ObjectValid() &&            // variable found ?
fRange.beg.obj.Unique == iUnique1 &&       // same ¶ as the marker ?
fRange.beg.offset == oTxtLoc1.offset+1) {  // TR =? variable behind marker
oDoc.DeleteText(fRange);                     // Delete the selected Variable
return true;
}
```

I think, we have discussed this enough now.

Klaus

• ###### 5. Re: Reducing Cyclomatic Complexity - in all cases?

> eventhough the may not have digested all 7 volumes of Knuth's "The art of computer programming".

Why should someone care about this, when we talk about scripting? In few cases perhaps this makes sense but in most cases of scripting, you need a small function (not a system) and when it works it works. That's the target of a script: Make something working.

Of Course, if you write a lot of scripts, you can/should write and extract common functionality to a library for making reuse.

To care about "The Art of Computer programming" results in costs, and who pays money for that, in such situations. Where is the business case for that?

If there are requirements in that direction, IMHO Scripting is not the right choice to build up such systems: JavaScript is a mess by definition.

And by the way, why do you think, the second snippet makes anything better? The first is more readable and so it can be maintained by other guys without code analysis, debugging or something else.

• ###### 6. Re: Reducing Cyclomatic Complexity - in all cases?
``` fRange = oDoc.Find  (oTxtLoc1, findParams);     // finds variable
if (fRange.beg.obj.ObjectValid()) {             // variable found ?
if (fRange.beg.obj.Unique == iUnique1) {      // same ¶ as the marker ?
if (fRange.beg.offset == oTxtLoc1.offset+1) { // TR =? variable behind marker
oDoc.DeleteText(fRange);                  // Delete the selected Variable
return true;
}
}
}

```

I prefer:

```Frange = oDoc.Fin...
if (fRange.beg.obj.ObjectValid()==false)
return false;
if (fRange.beg.obj.Unique)!=iUnique1)
return false;
if (fRagne.beg.offset != oTextLoc1.offset +1)
return false;
oDoc.DeleteText(fRange);
return true;

```

it brings functionality in Focus, and devides checking from function.

I know, not everybody is with me here :-)

• ###### 7. Re: Reducing Cyclomatic Complexity - in all cases?

Hi Klaus,

Just as an aside, the following article on clean code in javascript may provide some inspiration (or even more questions) on functions and readable and resuable code: