10 Sep
2012
10 Sep
'12
4:59 p.m.
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
10 Sep
10 Sep
6:17 p.m.
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 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:
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
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
7:23 p.m.
Hi Jerome,
Hey I didn't really want to awaken this topic again ;) I already gave my opinion on
the other thread (I still need to reply to Sergiu's last mail BTW). I just wanted to
explain how checkstyle computes the class fan out and share that since it was useful for
me to fix one class fan out issue I had.
See below.
On Sep 10, 2012, at 7:06 PM, Jerome Velociter <jerome(a)velociter.fr> wrote:
On 09/10/2012 06:17 PM, Vincent Massol wrote:
Would be interesting to ask the checkstyle guy why they picked this value. My guess is
that they used something like 7 + standard java classes.
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.
Not my choice. All references to Class Fan Out say that the default value should be around
7 and quote this article.
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
I've already answered to this. I'm -1 to change what we have in a random manner
with another random value. What I said is to show me some code and we can discuss on this
basis. Sergiu has pointed out some code but I didn't get the time to look at it yet. I
need to do that and if the outcome is that we can't make it pass with a good design
then we can discuss either make an exclude for this special case or decide that the case
is generic.
- 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.
Well, there's not much point in saying that something is wrong if you don't
provide an alternative… If you're going against the established standard then you
better be an expert in the domain. I know I'm not. But even if we agree that the study
has no meaning, it doesn't change the fact that to change what we have we need to have
a good reason to do so and we need to define the new value while still ensuring that the
check still makes 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 ?
Yep, I mentioned that in my mail (I said "Collection" classes).
For example, what about common-langs (StringUtils,
etc.) ?
I wouldn't put those, not everyone know them and again static method calls **don't
count** for the class fan out …
Anyway ATM I don't agree about starting to ignore classes without at least one good
example where we have to do so to make the fan out check pass.
I write new code almost everyday and it's seldom that I have issues with class fan out
and most of the time when it happens it's because I've been lazy and I can fix the
issue in a few minutes. This makes me think that the current value is good. As I've
already said in the other thread for me it would be much better to agree to do a
checkstyle exclude of *only* the ClassFanOut check for a given class rather than change
the default for everyone.
Thanks
-Vincent
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:
>
>> 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
7:37 p.m.
On 09/10/2012 07:23 PM, Vincent Massol wrote:
I never said it's wrong :) I was just trying to point out that attempts
at trying to justify or compute the magic numbers would sound like
guesswork ; since indeed none of us is an expert in the area.
And you're right, that topic sounds by itself like a PhD thesis subject,
and I have absolutely no pretention in the domain.
Jerome
Hi Jerome,
Hey I didn't really want to awaken this topic again ;) I already gave my opinion on
the other thread (I still need to reply to Sergiu's last mail BTW). I just wanted to
explain how checkstyle computes the class fan out and share that since it was useful for
me to fix one class fan out issue I had.
See below.
On Sep 10, 2012, at 7:06 PM, Jerome Velociter <jerome(a)velociter.fr> wrote:
Wouldn't they have rather ignored those classes ? I agree insights on
the rationale would be interesting.
On 09/10/2012 06:17 PM, Vincent Massol wrote:
Would be interesting to ask the checkstyle guy why they picked this value. My guess is
that they used something like 7 + standard java classes. 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.
Not my choice. All references to Class Fan Out say that the default
value should be around 7 and quote this article.
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
I've already answered to this. I'm
-1 to change what we have in a random manner with another random value. What I said is to
show me some code and we can discuss on this basis. Sergiu has pointed out some code but I
didn't get the time to look at it yet. I need to do that and if the outcome is that we
can't make it pass with a good design then we can discuss either make an exclude for
this special case or decide that the case is generic.
- 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.
Well, there's not much point in saying that something is
wrong if you don't provide an alternative… If you're going against the established
standard then you better be an expert in the domain. I know I'm not. But even if we agree that the
study has no meaning, it doesn't change the fact that to change what we have we need
to have a good reason to do so and we need to define the new value while still ensuring
that the check still makes sense.
--
Peace,
—Jerome
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 ?
Yep, I mentioned that in my mail (I said "Collection" classes).
For example, what about common-langs
(StringUtils, etc.) ?
I wouldn't put those, not everyone know them and again
static method calls **don't count** for the class fan out …
Anyway ATM I don't agree about starting to ignore classes without at least one good
example where we have to do so to make the fan out check pass.
I write new code almost everyday and it's seldom that I have issues with class fan
out and most of the time when it happens it's because I've been lazy and I can fix
the issue in a few minutes. This makes me think that the current value is good. As
I've already said in the other thread for me it would be much better to agree to do a
checkstyle exclude of *only* the ClassFanOut check for a given class rather than change
the default for everyone.
Thanks
-Vincent
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:
>
>> 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
7:48 p.m.
On 09/10/2012 07:23 PM, Vincent Massol wrote:
Hi Jerome,
Hey I didn't really want to awaken this topic again ;) I already gave my opinion on
the other thread (I still need to reply to Sergiu's last mail BTW). I just wanted to
explain how checkstyle computes the class fan out and share that since it was useful for
me to fix one class fan out issue I had.
See below.
On Sep 10, 2012, at 7:06 PM, Jerome Velociter <jerome(a)velociter.fr> wrote:
Yes, this is a way better approach than trying to rationalize said
random numbers. That was exactly my point actually.
Jerome
On 09/10/2012 06:17 PM, Vincent Massol wrote:
Would be interesting to ask the checkstyle guy why they picked this value. My guess is
that they used something like 7 + standard java classes.
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.
Not my choice. All references to Class Fan Out say that the default
value should be around 7 and quote this article.
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
I've already answered to this. I'm
-1 to change what we have in a random manner with another random value. What I said is to
show me some code and we can discuss on this basis. Sergiu has pointed out some code but I didn't
get the time to look at it yet. I need to do that and if the outcome is that we can't
make it pass with a good design then we can discuss either make an exclude for this
special case or decide that the case is generic.
--
Peace,
—Jerome
- 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.
Well, there's not much point in saying that something is
wrong if you don't provide an alternative… If you're going against the established
standard then you better be an expert in the domain. I know I'm not. But even if we
agree that the study has no meaning, it doesn't change the fact that to change what we
have we need to have a good reason to do so and we need to define the new value while
still ensuring that the check still makes 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 ?
Yep, I mentioned that in my mail (I said "Collection" classes).
For example, what about common-langs
(StringUtils, etc.) ?
I wouldn't put those, not everyone know them and again
static method calls **don't count** for the class fan out …
Anyway ATM I don't agree about starting to ignore classes without at least one good
example where we have to do so to make the fan out check pass.
I write new code almost everyday and it's seldom that I have issues with class fan
out and most of the time when it happens it's because I've been lazy and I can fix
the issue in a few minutes. This makes me think that the current value is good. As
I've already said in the other thread for me it would be much better to agree to do a
checkstyle exclude of *only* the ClassFanOut check for a given class rather than change
the default for everyone.
Thanks
-Vincent
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:
>
>> 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 
7:52 p.m.
On Mon, Sep 10, 2012 at 7:23 PM, Vincent Massol <vincent(a)massol.net> wrote:
Hi Jerome,
Hey I didn't really want to awaken this topic again ;) I already gave my
opinion on the other thread (I still need to reply to Sergiu's last mail
BTW). I just wanted to explain how checkstyle computes the class fan out
and share that since it was useful for me to fix one class fan out issue I
had.
See below.
On Sep 10, 2012, at 7:06 PM, Jerome Velociter <jerome(a)velociter.fr> wrote:
I completely agree about that last point. Specific exclusion of specific
rules (not only ClassFanOut) would be a really better practice than
excluding the whole file.
About ClassFanOut in particular, there is something that is not properly
taken into account when measuring this metrics. There is well know or
simple classes, that could be used and depends upon, and other no so well
know one, that will really increase the readability of the code, and its
independence. Since the computation of ClassFanOut metric does not take
that into account, it sometimes consider some code as badly design while it
is simply using many small simple classes, which is usually not a bad
design.
Excluding some custom well-know classes from the count could helps
mitigating that. IMO, classes representing References, Context, Collections
(as you suggest), and ... well this is where we need an expert. So this is
really not an easy topic and it will be very difficult to find a correct
number, and correct exclusions.
One last remark, when you leave a class with a ClassFanOut equal to the
limit, there is a lot of chance that a simple bug fix could exceed the
limit and cause lot of trouble while having not necessarily cause a worse
situation than before. IMO, the priority is to fix the issue first, and to
improve the code quality later if time is available. Of course, it depends
on the details, and it's not always true. This is generally why I am
always skeptical using these metrics as constraints. Could not the dev team
be more responsible without constraint (but keeping an eyes on metrics of
course !) ?
This is just my own ideas and I am not expert ;)
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 ?
Would be interesting to ask the checkstyle guy why they picked this value.
My guess is that they used something like 7 + standard java classes.
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.
Not my choice. All references to Class Fan Out say that the default value
should be around 7 and quote this article.
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
I've already answered to this. I'm -1 to change what we have in a random
manner with another random value. What I said is to show me some code and
we can discuss on this basis. Sergiu has pointed out some code but I didn't
get the time to look at it yet. I need to do that and if the outcome is
that we can't make it pass with a good design then we can discuss either
make an exclude for this special case or decide that the case is generic.
- 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.
Well, there's not much point in saying that something is wrong if you
don't provide an alternative… If you're going against the established
standard then you better be an expert in the domain. I know I'm not. But
even if we agree that the study has no meaning, it doesn't change the fact
that to change what we have we need to have a good reason to do so and we
need to define the new value while still ensuring that the check still
makes 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 ?
Yep, I mentioned that in my mail (I said "Collection" classes).
For example, what about common-langs
(StringUtils, etc.) ?
I wouldn't put those, not everyone know them and again static method calls
**don't count** for the class fan out …
Anyway ATM I don't agree about starting to ignore classes without at least
one good example where we have to do so to make the fan out check pass.
I write new code almost everyday and it's seldom that I have issues with
class fan out and most of the time when it happens it's because I've been
lazy and I can fix the issue in a few minutes. This makes me think that the
current value is good. As I've already said in the other thread for me it
would be much better to agree to do a checkstyle exclude of *only* the
ClassFanOut check for a given class rather than change the default for
everyone.
Thanks
-Vincent
--
Denis Gervalle
SOFTEC sa - CEO
eGuilde sarl - CTO
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:
>
>> 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
10:01 p.m.
On 09/10/2012 12: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).
That magical number doesn't really work well for software, IMO.
First, does everybody agree that a Block is that much different from
List<Block> to take up two slots? Same question for SectionBlock versus
GroupBlock, or BulletedListBlock versus NumberedListBlock. Yes, they are
all different, but I don't think that they really make it so hard for a
good software developer to work with several types of blocks, so that
each one is considered a different entity that must be counted. I'd
rather count types of classes than classes directly.
Then rarely does one need to understand the whole class and nothing but
the class. I need to keep in memory a code path at a time, and that
could go through several classes, but rarely through an entire class at
a time. So if we have to limit the number of different "entities" that
interact at a point, the class is not the best unit to investigate.
Third, good software developers are good especially because their brains
cope well with abstract thinking, and can hold entire algorithms in
memory. 7 is the average number observed for average people that don't
necessarily work in software development. I'd be interested in some
similar statistics for software developers.
I'm not arguing that we shouldn't limit the CFOC, I'm just saying that
we shouldn't bring the wrong arguments for a decision (don't use the
Chewbacca defense).
A kind of conclusion is that it's a real pain to work with XDOM.
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:
> 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");
The fact that so many exceptions are excluded by default makes me think
that most exceptions could be safely ignored. So I think that a better
rule would be "ignore descendants of Throwable" than to list some
classes. So here two different code quality aspects collide: making an
API that's clear about its failure cases, and keeping a low count on the
number of imported classes.
An exception to this would be badly designed code that uses exceptions
as normal code flow, but we should stay away from such code anyway.
> * 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
>
--
Sergiu Dumitriu
http://purl.org/net/sergiu/
11 Sep
11 Sep
2:27 p.m.
BTW, one more thought on this topic:
* We're doing field injection in our code and there's a risk when doing this. The
risk is that we don't visually see how many dependencies we add to our class since
it's so easy to use.
* If we were using Constructor injection we would have a list of parameters and above 6-7
we would start to be worried. Actually Checkstyle would catch us since there's a check
on the number of parameters a method accepts.
* Thus Class Fan Out is one way to tell us that we're getting injected too many
components and that we need to introduce an intermediary component level.
* Note that there's another warning: it's the size of the Mock Expectations block
we have in our unit tests (when using AbstractMockingComponentTestCase). When it starts to
be too big it's a code smell and more specifically a design smell that we have too
many injections. The problem is that we don't have any failure on any threshold for
this so it's good that the Class Fan Out has this threshold to warn us.
Thanks
-Vincent
On Sep 10, 2012, at 4:59 PM, Vincent Massol <vincent(a)massol.net> wrote:
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
8 Oct
8 Oct
4:54 p.m.
Note: there seems to be a big bug in Checkstyle and it seems that it's currently
counting java.lang classes even though it should not.
See
https://sourceforge.net/tracker/?func=detail&aid=3575496&group_id=2…
which I have just opened.
I propose that while waiting for this bug to be fixed we use a specific checkstyle exclude
*only for this check* when needed and put in comment TODO to remove it when
https://sourceforge.net/tracker/?func=detail&aid=3575496&group_id=2…
is fixed.
Thanks
-Vincent
On Sep 10, 2012, at 4:59 PM, Vincent Massol <vincent(a)massol.net> wrote:
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
4:56 p.m.
+1, I noticed it too.
On Mon, Oct 8, 2012 at 4:54 PM, Vincent Massol <vincent(a)massol.net> wrote:
Note: there seems to be a big bug in Checkstyle and it
seems that it's currently counting java.lang classes even though it should not.
See
https://sourceforge.net/tracker/?func=detail&aid=3575496&group_id=2…
which I have just opened.
I propose that while waiting for this bug to be fixed we use a specific checkstyle
exclude *only for this check* when needed and put in comment TODO to remove it when
https://sourceforge.net/tracker/?func=detail&aid=3575496&group_id=2…
is fixed.
Thanks
-Vincent
On Sep 10, 2012, at 4:59 PM, Vincent Massol <vincent(a)massol.net> wrote:
--
Thomas Mortagne
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
4414
days inactive
4442
days old
10 comments
5 participants
participants (5)
-
Denis Gervalle -
Jerome Velociter -
Sergiu Dumitriu -
Thomas Mortagne -
Vincent Massol