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

    Reducing Cyclomatic Complexity - in all cases?

    K.Daube Level 1

      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?
          Russ Ward Level 4

          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?
            Wiedenmaier Level 3

            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?
              K.Daube Level 1

              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:

              • 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?
                K.Daube Level 1

                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

                • 5. Re: Reducing Cyclomatic Complexity - in all cases?
                  Wiedenmaier Level 3

                  > 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?
                    Wiedenmaier Level 3
                     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?
                      Arnis Gubins Adobe Community Professional & MVP

                      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

                      1 person found this helpful
                      • 8. Re: Reducing Cyclomatic Complexity - in all cases?
                        Ian Proudfoot Level 3

                        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?