Thanks Tom for this opportunity to explain and advocate my
interpretation of the Code Conventions for the Java Programming Language
from Sun. ;-)
I would really like that new commits and changes to the code didn't
increase the number of checkstyle warnings. Michiel did a great job a
while back in addressing all these warnings. We were down to 20 or so
warnings but then the Xenofarm setup went offline for a couple of months
and suddenly we were at 600++ warnings. Since, I have also set up the
findbugs to run nightly and we have gotten Eclipse with new and
interesting warnings so things have happened in this area.
Tom Morris wrote:
> I'd like to see the following changes:
>
> 1. revise the coding conventions to something that we'll ALL use (and
are
> easily supported by our tools) 2. come up with a set of
> Eclipse/Emacs/whatever settings that support our conventions 3.
enforce
> conformance by asking the author to make revisions to non-conforming
> commits
> rather than changing their code for them 4. when reformatting old
code,
> make the minimal change which will bring the code into conformance 5.
> Stop
> mechanically resolving checkstyle warnings without adding value - this
> just
> masks real problems
> (e.g. adding comments like /* Constructor */ or Javadoc @param tags
> with
> no contents)
About your points 1 - 5, that is a good plan. I see just two small
weaknesses.
In your item 1. This is the kind of discussions that could go on forever
and that I am very fond of taking part in. Endless discussions are not
beneficial to the project. For this reason I have been stubborn about
sticking to the Code Conventions for the JavaTM Programming Language
from Sun. This is both to end the discussion quickly and to not having
to do any changes. Last changes we made to this were that we removed
some of the points where we deviated.
By sticking to probably the oldest and most well known Code Conventions
for Java I had expected that it would have had the best tool support.
Alas that has not been so. There are still in Eclipse 3.1 bugs about
this.
In your item 3. I have been trying to avoid this kind of pointing finger
because when I attempted it, the result was not satisfying. Instead I
have set up systems to check things automatically hoping that everyone
would pursue this. Because the number of checkstyle warnings have
increased, one could argue that I have failed also in this. We need
someone with better socials skills than I have to do the "asking".
I will provide explanations on this:
[...]
> - Eclipse shows the constructor Javadoc for its tips, so we need to
> encourage people to make them descriptive. A comment of /*
Constructor.
> */
> is useless.
What kind of information would be useful for the constructor? Is it the
information that the class has? At this late hour, the only thing I can
come up with is when a class has several constructors. In that case it
is important to know why one of them should be choosed.
> - Why is the 2nd form better than the first below? If we are going to
> require #2, let's come up with the Eclipse settings that will make it
wrap
> things that way (it does #1 by default).
>
> #1: for (Iterator iter =
modelImpl.getModelPackage().getMofClass()
> .refAllOfClass().iterator(); iter.hasNext();) {
> #2: for (Iterator iter =
>
modelImpl.getModelPackage().getMofClass().refAllOfClass()
> .iterator();
> iter.hasNext();) {
This comes from my (strange?) interpretation of the Wrapping rules. My
interpretation is with the emphasis on "Prefer higher-level breaks to
lower-level breaks". Since ';' is a higher level than '=' and '.' the
first break is there. Then '=' is a higher level than '.' so the next
break is at that one.
For statements also has the peculiarity that the initialization and the
condition looks like statements. They end with ';' and everything. So I
prefer writing them each on their own line if wrapping is needed.
> - I don't see how expanding the concise single line of #1 below to 4
(!)
> lines adds any value at all :
>
> #1: subkeys = new String[] {""};
> #2: subkeys =
> new String[] {
> "",
> };
This is not stated in the Code Conventions. This comes from a
combination of checkstyle rules (NoWhitespaceAfter (comman, left brace)
and ArrayTrailingComma) together with the "higher-level breaks
preferred" as above.
> - Non-Javadoc comments are getting changed to Javadoc versions for
private
> fields and methods which seems wrong to me. Isn't this just going to
> clutter the Javadoc with irrelevant information? Note I'm not saying
> things
> shouldn't be commented, just arguing whether we should be promoting
the
> comments to Javadoc status.
I disagree. Either the private fields and methods are interesting to the
person examining the class or they are not. If they are interesting,
then the javadoc view could just as well contain their documentation. If
they are not interesting, then why should the javadoc contain them at
all? From this reasoning, it is not the writer of the code that decides,
on a per field or method basis, if the reader of the code using the
javadoc view is interested in the documentation or not. It is the reader
of the code that decides how his javadoc view is created, i.e. with or
without private fields and methods.
We have currently two views created:
* The outside view (called javadocs-api) at
http://argouml-stats.tigris.org/nonav/reports/javadocs-api/ (nightly
build), only the subsystems' main package, completely without private
fields and methods. This is for developers working without wanting or
needing knowledge of the internals of the subsystem they are using.
* The inside view (called javadocs) at
http://argouml-stats.tigris.org/nonav/reports/javadocs/ (nightly build),
with all classes, all fields and methods (private or not). This is for
developers working with full knowledge of the insides of all subsystems.
> - Is it really significant whether we have <p> in Javadoc at the end
of
> one
> line or the beginning of the next? Do we really need to keep moving
these
> around? I don't think there's an Eclipse setting to control this, so
I'd
> really REALLY prefer to see wherever Eclipse puts them be a legal
option.
The javadoc is probably read more often in the code editor than as
generated javadoc. In the code editor, the html tags (<p>, <li>, <pre>,
...) are cluttering the view. They should be moved to where they do the
least cluttering possible. For the <p> tag, I think that the least
cluttering is last on a line followed by an empty line. The <p>-tag then
creates a new section in the generate html, the empty line is the new
section when viewing in the code editor.
If the <p> is put first on a line, the end of that section is not as
clearly marked. Especially (in some cases that I have fixed) where the
line with <p> is instead of the empty line.
> - Checkstyle (or the compiler?) is warning about missing serialization
> versions for classes that implement Serializable. My understanding is
> that
> without the version number, Java may think things are incompatible
when
> they
> really are still compatible. Personally I don't think that's a big
deal.
> I
> also doubt that we have the discipline to maintain the version #
> appropriately for changes to the public API. I've added some of these
to
> make the warnings go away, but I think a better option would be to
disable
> the check.
I think these are Eclipse warnings and not checkstyle warnings.
You are right. We don't use serialization anyway so why should we
bother. I have now turned it off in my Eclipse. I will add instructions
for turning this off in the Cookbook (and see what I can do about the
findbugs settings).
/Linus
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
