Hi Raif, On Sat, 2006-02-04 at 22:19 +1100, Raif S. Naffah wrote: > > > Although I don't want to claim to be a GNU source formatting > > > expert. > > > > I am neither, but emacs is mostly right :) > > Normally we try to follow the generic GNU C formatting: > > http://www.gnu.org/prep/standards/html_node/Formatting.html > > The specific GNU Classpath java source code extensions are > > documented: > > http://www.gnu.org/software/classpath/docs/hacking.html#SEC7 > > since we're at it ;-) here is the output of the GNU > (srcipts/eclipse-gnu.xml) and GNU Classpath (attached > eclipse-gnu-classpath.xml) which one looks more acceptable?
I'll just go over it and say what looks wrong to me.
> //
> --------------------------------------------------------------------------
>
> // gnu.java.security.key.IKeyPairCodec interface implementation
> -------------
These lines look strange/wrong and are a pain to keep right (think
refactoring the class name and then having to keep these exactly the
same length). I would just remove them completely because the disturb
the reading flow.
> if (!(key instanceof DSSPublicKey))
> throw new InvalidParameterException("key");
Officially there is a space after each operator, so also for ! to make
it more clearly stand out. (This is not always/consistently done in our
sources though.) So write:
if (! (key instanceof DSSPublicKey))
throw new InvalidParameterException("key");
> byte[] result = null;
> // ----- begin from gnu.java.security.provider.GnuDSAPublicKey
What is this comment trying to say me? There are a couple of these that
I don't immediately understand the relevance of.
> try
> {
> DERWriter.write(baos,
> new DERValue(DER.CONSTRUCTED | DER.SEQUENCE, spki));
> result = baos.toByteArray();
> }
Good, but personally I would write this:
try
{
DERValue val = new DERValue(DER.CONSTRUCTED | DER.SEQUENCE, spki);
DERWriter.write(baos, val);
result = baos.toByteArray();
}
> BigInteger p = null;
> BigInteger q = null;
> BigInteger g = null;
> BigInteger y = null;
Style comment. Since all these values should be definitely assigned on a
normal return from the method where they are used I would not assign
them a value here so the compiler can catch when they are used
uninitialized. (Note to compiler writers: A compiler could warn on such
assignments to constants that are never used.)
> try
> {
> DERWriter.write(baos,
> new DERValue(DER.CONSTRUCTED | DER.SEQUENCE, spki));
> result = baos.toByteArray();
> }
See above. The "new DERValue" should align with the "baos" after the
opening bracket.
Cheers,
Mark
signature.asc
Description: This is a digitally signed message part
