ClassFanOutComplexity metric

Hi everyone,

so I’m opening this as a brainstorming topic on a maybe sensitive subject which is the ClassFanOutComplexity metric that we use with Checkstyle. Right now the CFOC threshold is 20 and I’m wondering if:

  1. we should keep this metric
  2. we should keep this threshold

I’m starting this discussion because that’s the checkstyle issue I hit the most often when I develop new stuff in XWiki, and the solution to fix those are generally not obvious to avoid adding meaningless classes just there to pass the checkstyle. Another reason is that I’ve just seen this issue: https://jira.xwiki.org/browse/XCOMMONS-2294 which appears related to a fix of checkstyle which was badly counting the CFOC. When they will solve it we might discover new issues in our existing code.
Finally a last reason, is that I googled “how to reduce class fan out complexity” (don’t judge me :)) and I surprisingly found first this XWiki markmail archive: http://markmail.org/message/snun6avp3qlkg54k

So clearly not the first time the subject is on the table, but just to add some information, after reading it I checked and apparently Checkstyle itself doesn’t use a threshold of 20 for CFOC, they use 25: https://github.com/checkstyle/checkstyle/blob/master/config/checkstyle_checks.xml#L647

So I’m not proposing right now that we increase our value, even if I’d like it, just wondering what do you think about this metric nowadays? I know that on my side I really don’t like it and I don’t find it very useful personally. We could check but I think most of the checkstyle problems we ignore are actually related to this one.

So, sorry for this hot topic, hope it won’t become a flamewar but would be interesting to know your opinion.

Thanks.

Reminder:
Class fan-out complexity : The number of other classes a given class relies on.

Note that we already covered this topic in the past:

My POV is that 20 is a good value and we shouldn’t change it. Above that would most likely means that the design is not good and too much different domains are put in the same class. That’s the reason these errors are hard to fix; they require hard design thinking.

What we did in the past and what we could check/review is to add some known classes that don’t increase the mental complexity because they are well known base classes (such as the java collections), if we find some that are not already in our ignore list (we already have quite a lot but it’s possible we missed some: https://github.com/xwiki/xwiki-commons/blob/05afe1a2f9b07784c3d4e136a1ea5b4bfc27d075/xwiki-commons-tools/xwiki-commons-tool-verification-resources/src/main/resources/checkstyle.xml#L109-L120 ).

I still agree with what I said in the past: let’s see some code that fails and brainstorm about how this code should be written for the best readability/design.

Personally I don’t fall on CFOC often. Happens from time to time. Sometimes it’s a bit painful and I have to rack my brain to find a solution and sometimes I even don’t find a good solution but most of the time the resulting code is simpler to read.