hello Mark,

On Sunday 05 February 2006 00:03, Mark Wielaard wrote:
> On Sat, 2006-02-04 at 22:19 +1100, Raif S. Naffah wrote:
> > ...
> >   //
> > -------------------------------------------------------------------
> >-------
> >
> >   // 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.

noted.


> >     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...

yes we can do that: force a white space after a unary operator, but then 
we get:

    boolean condition1 = false;
    boolean condition2 = true;
    // ...
    if (condition1 & ! condition2)
      {
        // do something
      }

rather than 
    if (condition1 & !condition2)

if this is what we want then i'll update the style specs file 
accordingly.


> >     try
> >       {
> >         DERWriter.write(baos,
> >                         new DERValue(DER.CONSTRUCTED |
> > DER.SEQUENCE, spki)); result = baos.toByteArray();
> >       }
>
> Good...

the alternative uses a deep (2*indent) for continuation lines to avoid 
output like the one you see in 
gnu.java.security.key.DSSKeyPairGenerator; e.g. declaration of 
KEY_PARAMS_512.  i see from your later comments) though that this is 
not what we want.


> >     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.)

good point.  a good compiler does catch these things.  they were 
assigned values before all the code that sets them was written to 
silence that compiler.  your comment is a valid reminder to re-visit 
the code once all the logic has been implemented.


i didn't see any comments on

a/ the line-wrapping before extends and implements.  i assume the 
alternative is the accepted one?

a.1:

public class Main extends KeyGenerator implements java.io.Serializable

or a.2:

public class Main
    extends KeyGenerator
    implements java.io.Serializable


b/ Javadoc style; which is more acceptable:

b1:

   * @param input the byte array to unmarshall into a valid DSS
   *          [EMAIL PROTECTED] PublicKey} instance.

or b2:

   * @param input
   *          the byte array to unmarshall into a valid DSS
              [EMAIL PROTECTED] PublicKey} instance.


TIA + cheers;
rsn

Attachment: pgpdYCct1gRgY.pgp
Description: PGP signature

Reply via email to