Who thinks it would be great if the Drupal Coding Standards included checks for Cyclomatic Complexity?
This is a metric used to check the complexity of a function by checking the number of paths through it. There are those that think this should be limited at 10, any more and the function is too complex and should be refactored into a number of smaller, more easy to read, functions.
A good place this has highlighted faults is with people's use of the hook_form_alter() which ends up with a switching if of doom when they should really be using hook_form_FORM_ID_alter in most cases to just work on the form they actually want to change.
The PHPCS Generic CoderSniffer rules include the logic for this check already. I've pulled their code into the Drupal Coding Standards and will supply a patch for review below.
For more details on this metric, see: http://en.wikipedia.org/wiki/Cyclomatic_complexity
Comment | File | Size | Author |
---|---|---|---|
#4 | coder-cyclomatic_complexity_to_dcs-1934594-7151932.patch | 530 bytes | johnennew |
#1 | coder-cyclometric_complexity-1934594-1.patch | 7.89 KB | johnennew |
Comments
Comment #1
johnennew CreditAttribution: johnennew commentedNot sure on the rules of code reuse, this code is as it was when I pulled it out of the Generic CodeSniffer Metrics folder with Generic changed for Drupal. It works but the code is (ironically) not DCS compliant itself.
Let me know if this is of interest and if it can be used if cleaned up.
Comment #2
johnennew CreditAttribution: johnennew commentedThis patch also adds a warning at greater than 5 nesting levels and an error at 10 which is quite lenient!
Comment #3
klausiPlease add a rule to ruleset.xml and try that before copying the code.
Comment #4
johnennew CreditAttribution: johnennew commentedPlease find revised patch with ruleset.xml rather than code patch.
Comment #5
klausiWhile I think this is useful I'm not sure if there is an actual coding standard documented for this, can you point me to a paragraph in our documentation?
As long as this is no official standard I think we need to set the $absoluteComplexity to something very high (999?), to not throw any errors (you can override it in ruleset.xml).
And $complexity with a value of 10 might be a bit low in Drupal land, so I guess we should increase that as well to not through unnecessary warnings.
Comment #6
johnennew CreditAttribution: johnennew commentedHi Klausi,
This is not part of the coding standards - I think it should be though.
Do you know where do the coding standards get discussed?!
Comment #7
klausiIn the drupal core issue queue - please link any issue you open here for reference. Until then this is postponed.
Comment #8
johnennew CreditAttribution: johnennew commentedDrupal core issue for debate: http://drupal.org/node/1960996
Comment #9
klausiCoder 7.x is frozen now and will not receive updates. Coder 8.x-2.x can be used to check code for any Drupal version, Coder 8.x-2.x also supports the phpcbf command to automatically fix conding standard errors. Please check if this issue is still relevant and reopen against that version if necessary.