Hello.
> On Tue, Jan 11, 2011 at 7:38 AM, sebb <seb...@gmail.com> wrote:
>
> > On 11 January 2011 15:15, Gilles Sadowski <gil...@harfang.homelinux.org>
> > wrote:
> > > Hi.
> > >
> > > Is there a document presenting best practices for writing code for CM?
> >
> > Sun/Oracle have Javadoc conventions which should be followed.
> >
>
> See
> http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#styleguide
>
> <http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#styleguide>Note
> that several items there conflict with what Gilles suggested.
>
>
> > > * All exceptions that could be thrown must be documented in a "@throws"
> > tag,
> > > indicating all the possible causes of the exception (such as the
> > > assumed preconditions)
> >
> > +1
> >
>
> Certainly all checked exceptions. Unchecked exceptions are a bit more hazy.
> Should all code that has any arithmetic or pointer dereferencing document
> the related exceptions? Probably not.
We agreed that in CM we would aim at documenting _all_ exceptions (i.e.
null references too) caused by CM code.
> Clarity can be compromised by too
> much goo just as much as with too little.
>
I totally agree. (But see below also.)
>
> > > * The "@param" tag should not contain redundant information (such as
> > > preconditions since they must be documented in a "@throws" tag).
> >
> > -1
> >
> > It's a lot easier to read the preconditions in the param description,
> > rather than wading through all the throws descriptions to see if the
> > parameter is mentioned.
> >
> > But where there are only one or two params and throws the duplication
> > may be unnecessary.
> >
>
> Also -1 for exactly the reasons stated.
(1)
I agree that in the absolute, the best place for a parameter precondition is
in the "@param" tag description.
(2)
If the Javadoc format were perfect, we would be able to specify a
(hypothetical) "@precondition" together with an associated "@throws" if
violated. But we can't.
(3)
We must (CM policy, cf. above) have a "@throws" for each exception. And thus
we must also say when (i.e. describe the precondition) this particular
exception is thrown.
(4)
Hence the description of the precondition is redundant in the "@param" tag.
Another case is when a precondition is formed from a combination of several
parameters. Then it certainly makes more sense to describe it _once_ in the
"@throws" rather than several times, in each of the "@param" tags.
>From another standpoint, redundant information might be welcome if the
preconditions are not obvious. An additional explanation within the main
description could mention the "caveat"s.
However I'm against repeating the obvious in the "@param" tag. IMHO, the
example that triggered this discussion falls in this category: You have a
class named
BaseMultiStartMultivariateRealOptimizer
and you have a constructor parameter named
starts
Then the description
* @param starts Number of starts to perform.
* @throws NotStrictlyPositiveException if {...@code starts < 1}.
is clear and complete (as far as the parameter "starts" is concerned).
While in this one:
* @param starts Number of starts to perform, must be >=1.
* Multi-start is disabled if {...@code starts == 1}.
* @throws NotStrictlyPositiveException if {...@code starts < 1}.
the same information is unnecessarily stated multiple times (obviously, the
number of starts must be positive, and, when "starts" is equal to 1, you
will have a single start).
Another example within CM is the repetition of something like "cannot be
null", at the "@param" or main description level, whereas I tend to think
that only the case "may be null" is unusual in CM (and thus worth
mentioning).
>
>
> >
> > > * Javadoc comments must be composed of full sentences (except for the
> > > definition of a "@param"), including punctuation and uppercase at the
> > > start of the sentence. E.g. write
> > > /**
> > > * @param x Value of the parameter. A value of -1 indicates that
> > > * the parameter is not used.
> > > */
> > > but not
> > > /**
> > > * @param x Value of the parameter. -1 for "not used".
> > > */
> >
> > I think the Javadoc standards may have useful guidance here too.
> >
>
> -1. Specifically the style standards suggest that sentence fragments
> *should* be used in some cases.
If you refer to
Okay to use phrases instead of complete sentences, in the interests of
brevity. This holds especially in the initial summary and in @param tag
descriptions.
I agree with that, as I said above "[...] full sentences (except for the
definition of a "@param"), [...]".
However I don't agree to sacrify clarity (in the sense of easy reading)
for the sake of saving a few typing strokes (cf. "Write once, read many
times").
>
> > > * When a description starts with a verb, write it in the imperative form
> > > (not the at the third person of the present tense). E.g. write
> > > /**
> > > * Compute the value of the function.
> > > */
> > > rather than
> > > /**
> > > * Computes the value of the function.
> > > */
> >
>
> Again, this is in direct contradiction with the style standards.
I know. But there is no best choice; it depends on how you interpret the
description (and how often you read the code vs the generated docs).
E.g. with the Java standard (3rd person indicative), you can read the
generated doc as e.g.
[The] "remove" [method] Removes the element [...]
Whereas I tend to prefer keeping the doc in sync with the actual "action"
(cf. "message passing"/imperative) of the method name:
/**
* Remove the element ...
*/
public E remove(int index) {
// ...
}
The generated doc would thus read
[Use the method] "remove" [in order to] Remove the element [...]
>
> > The class Javadoc needs to mention thread-safety (or otherwise).
> >
> > If user code needs to apply synch. to ensure thread-safety, then the
> > Javadoc should say what lock(s) need to be held.
> >
> > @since tags must be used where relevant
> > @deprecated/@Deprecated tags/annotations should say when the item was
> > deprecated, and when it is due to be removed (if known)
> >
> > @SuppressWarnings should be applied as close as possible to the code
> > that causes the warning (this may involve creating a temporary
> > variable) and should have a comment that explains why the warning is
> > safe to ignore.
> >
>
> These are all good if downgraded to a very strongly emphasized should
> instead of must.
Well this "Guidelines" document should in fact state what *must* be done.
If a developer does not know how to comply with all the rules, this list
should then help _before_ the commit. This will be less tedious than after
the fact clean-up (especially if the initial committer is not the one who
makes the clean-up...).
Regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
