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

Comments

johnennew’s picture

Status: Active » Needs review
FileSize
7.89 KB

Not 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.

johnennew’s picture

This patch also adds a warning at greater than 5 nesting levels and an error at 10 which is quite lenient!

klausi’s picture

Status: Needs review » Needs work

Please add a rule to ruleset.xml and try that before copying the code.

johnennew’s picture

Status: Needs work » Needs review
FileSize
530 bytes

Please find revised patch with ruleset.xml rather than code patch.

klausi’s picture

While 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.

johnennew’s picture

Hi 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?!

klausi’s picture

Status: Needs review » Postponed (maintainer needs more info)

In the drupal core issue queue - please link any issue you open here for reference. Until then this is postponed.

johnennew’s picture

Drupal core issue for debate: http://drupal.org/node/1960996

klausi’s picture

Issue summary: View changes
Status: Postponed (maintainer needs more info) » Closed (won't fix)

Coder 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.