10 Sep
2012
10 Sep
'12
7:06 p.m.
On 09/10/2012 06:17 PM, Vincent Massol wrote:
With this finding writing a macro requires between 5
to 6 Classes:
* DefaultContentDescriptor (only if there's content)
* XXXMacroParameters
* List
* Block
* MacroTransformationContext
* MacroExecutionException
Our checkstyle config fails above 20 which means Macro classes have about 14 additional
deps on external classes allowed before they break. Which should normally be more than
enough.
My understanding is that reading the code is hard when there are too many references to
external classes (incidentally it also makes the code more brittle as there are more
chances it'll break due to an issue with the used classes). Apparently 7 (+/- 2) is
the magic number (see
http://en.wikipedia.org/wiki/The_Magical_Number_Seven,_Plus_or_Minus_Two)
So this shows "20" was magic to begin with. How do you go from 7 +/-2 to
20 ?
I find it a bit specious to apply the theories exposed in the article to
a program's class external classes without a pinch of salt. It would be
saying their is a good match between the notion of "object" as described
in the experiments (so in the sense of an item of input, like a word in
a list of words or a sound in a list of distinct sounds) and the notion
of a class of objects in programming.
What I'm trying to say is that maybe we should rather accept the fact 20
is magic to begin with and try and figure out if this works for us and
tweak it if necessary - rather than trying to rationalize it with
bridges to psychology thesis that are far from obvious (not that the
thesis don't apply to programming ; just that the impendance of an
"input" or "unit of thought" is IMHO not so easy to match with
programming concepts without making blind guesses). Maybe I'm wrong,
it's just I find this kind of rationalization somehow naïve.
Hope that made sense.
Also, a note about ignoring classes that "don't require effort to
understand" (or to look at their source code), they would probably span
more than "standard Java classes" like Collections, no ? For example,
what about common-langs (StringUtils, etc.) ?
Jerome
So IMO we have 2 options that could make sense (ie we can rationalize them):
* Keep this default value of 20 total which includes some standard JDK classes. 14 deps
for Macro should be enough, even if we count, say 2-4 more for standard Java classes like
Collection classes.That still gives us about 10 deps for additional XWiki classes.
* Decide to exclude some classes that are basically part of the Java language (such as
Collection classes) since they don't increase the reading complexity of the code since
everyone knows what they do and we don't need to look their source code or doc to
understand them. However, if we do so then we should reduce the allowed Fan out to 7 (+/-
2) so let's say to 9. So that gives us only 9 deps for XWiki classes, compared to
about 15 ATM...
IMO the second option is much harder for us than the first one and that's why I'd
keep the first option...
BTW the reason I wrote about all this is because I made a change to the ChartMacro and
the fan out became 21… In the end I refactored the code and got a slightly better design
(and fixed a bug at the same time…).
Thanks
-Vincent
On Sep 10, 2012, at 4:59 PM, Vincent Massol <vincent(a)massol.net> wrote:
--
Peace,
—Jerome
Hi devs,
I wanted to understand how Checkstyle computes the Class Fan out so I debugged it.
Here are my findings:
* Some classes are excluded by default:
mIgnoredClassNames.add("boolean");
mIgnoredClassNames.add("byte");
mIgnoredClassNames.add("char");
mIgnoredClassNames.add("double");
mIgnoredClassNames.add("float");
mIgnoredClassNames.add("int");
mIgnoredClassNames.add("long");
mIgnoredClassNames.add("short");
mIgnoredClassNames.add("void");
mIgnoredClassNames.add("Boolean");
mIgnoredClassNames.add("Byte");
mIgnoredClassNames.add("Character");
mIgnoredClassNames.add("Double");
mIgnoredClassNames.add("Float");
mIgnoredClassNames.add("Integer");
mIgnoredClassNames.add("Long");
mIgnoredClassNames.add("Object");
mIgnoredClassNames.add("Short");
mIgnoredClassNames.add("String");
mIgnoredClassNames.add("StringBuffer");
mIgnoredClassNames.add("Void");
mIgnoredClassNames.add("ArrayIndexOutOfBoundsException");
mIgnoredClassNames.add("Exception");
mIgnoredClassNames.add("RuntimeException");
mIgnoredClassNames.add("IllegalArgumentException");
mIgnoredClassNames.add("IllegalStateException");
mIgnoredClassNames.add("IndexOutOfBoundsException");
mIgnoredClassNames.add("NullPointerException");
mIgnoredClassNames.add("Throwable");
mIgnoredClassNames.add("SecurityException");
mIgnoredClassNames.add("UnsupportedOperationException");
* All classes in java.lang.* are excluded too
* Annotation classes are not counted
* Classes in the same package are counted (they won't appear in import since it's
in the same package so don't count imports to get class fan out)
* Static method calls are not counted. So for example StringUtils from Commons Lang never
counts for class Fan out
* Enums are not counted (no new XXX() done. That's why static method calls are not
counted too BTW)
* Classes used in class extend or implement are not counted too.
Hope it helps
-Vincent
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs