Copy link to clipboard
Copied
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?
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
...Copy link to clipboard
Copied
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
Copy link to clipboard
Copied
Thanks Russ for Your answer,
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:
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.
Copy link to clipboard
Copied
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.
Thanks for Your contributions.
Klaus
Copy link to clipboard
Copied
- 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 🙂
Copy link to clipboard
Copied
> 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.
Copy link to clipboard
Copied
I would endorse JavaScript: The good parts by D. Crockford. I write all of my ExtendScript according to his guidelines and use JSLint too. That has worked well for me as the code is efficient, clear and easy to understand, even after a break from it of nearly five years...
My background isn't in computer science, so I find terms like 'Cyclomatic Complexity' quite off-putting, but is it important enough that I should be worried about it?
Copy link to clipboard
Copied
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
Copy link to clipboard
Copied
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:
GitHub - ryanmcdermott/clean-code-javascript: Clean Code concepts adapted for JavaScript
Get ready! An upgraded Adobe Community experience is coming in January.
Learn more