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, 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?
AdChoices