At 08:47 AM 1/25/2013, Vincent Ryan wrote:
>Please review this fix to correct a failing PKCS12 test:
>
>Webrev: 
><http://cr.openjdk.java.net/~vinnie/8006946/webrev.00/>http://cr.openjdk.java.net/~vinnie/8006946/webrev.00/
>
>Thanks. 

This first is not a comment on the change - but a "why can't java do ...." type 
of comment....


>+                 // skip friendlyName, localKeyId and trustedKeyUsage
>+                 if (CORE_ATTRIBUTES[0].equals(attributeName) ||
>+                     CORE_ATTRIBUTES[1].equals(attributeName) ||
>+                     CORE_ATTRIBUTES[2].equals(attributeName)) {
>+                     continue;
>+                 }
>                 


We allow arrays to be implicitly treated as collections only in the for () 
construct.  For the above, it would be really nice if you could just do:

if (CORE_ATTRIBUTES.contains(attributeName)) {}


For your version, it would be increase supportability if instead of:


>+     private static final String[] CORE_ATTRIBUTES = {
>+         "1.2.840.113549.1.9.20",
>+         "1.2.840.113549.1.9.21",
>+         "2.16.840.1.113894.746875.1.1"
>+     };
>+ 


you did

private static final String idAtPkcs9FriendlyName = "1.2.840.113549.1.9.20";

etc... and did "idAtPkcs9FriendlyName.equals(attributeName)" 

I can't see any reason why this is in an array as it's not referenced anywhere 
else in the code.

Mike


Reply via email to