Re: [xwiki-devs] [VOTE] Change the Class Fan-Out Complexity maximum value in our checkstyle configuration

On Mon, Jul 9, 2012 at 8:59 PM, Vincent Massol <vincent(a)massol.net> wrote: Complexity checkstyle rule from 20 to 25, since a bare component already uses 1 to 4 imports, and the 20 rule was set before we had components. used by a class. Keeping this to a small value is a good goal, since it favors loose-coupling, small and independent classes, and makes it harder to put more than one responsibility in a single class. java.util.List, and a fairly complex class uses more than one such utility class; they usually come in pairs (the interface and the implementation). And more and more classes are using apache-commons utilities like StringUtils or IOUtils, which are just shortcut helper methods that could be implemented in a few lines of code, so we're trading one import for reduced code complexity, which is a good thing, even though apparently it's increasing the Fan-Out measure. import: barely using some of our dependencies will add a lot of imports. For example, the LucenePlugin has 25 org.apache.lucene.* imports just for initializing the server and sending search requests. classes and interfaces, I've found myself often enough trying to refactor a class to get down from 23 or even 21 imports, and usually I find myself doing ugly tricks, keeping the actual code complexity the same or even worse. Best case is that I extract an extra class that just contains the non-public utility methods that were initially in the component implementation, but that class is meaningless out of the context of its parent class, so that is also a bad decision, IMHO. requirement that classes should be kept as small as possible. recommended value of 20 and then decide if the problem is in the class design or in this parameter threshold. design… :) Whether you use components or not is exactly the same. Quite the opposite actually since components favor decoupling and thus reduces class fan out. bad sign. You have a design issue. Just write some unit tests using AbstractMockingComponentTestCase and you'll understand you have a problem because you'll start needing to mock way too many things. it's because you're not thinking at the level of design. The problem has more to do with the fact that you're usually working on fixing something and suddenly you execute the build and you don't feel like redesigning the code just because of a class fan out issue and thus we usually end up adding technical debt by removing it from the checkstyle check. IMO this is the part that we need to fix but not the threshold. of the others are syntactic sugar and are not important whereas this one is really about design. Of course since it's about design it's also harder to fix but that doesn't remove its value. Having a good design has always been hard. And not having a good design generates technical debt. had any real issue with this class fan out except for old code I didn't write and for which I brought some quick bugfix and I didn't want to spend the time to refactor it. That doesn't make the rule bad. I agree with Vincent. I've hit the limit myself a few times but I'm sure it was due to bad code design. I'm -0.

Thanks, Marius _______________________________________________ devs mailing list devs(a)xwiki.org http://lists.xwiki.org/mailman/listinfo/devs