Update: http://hg.openjdk.java.net/jdk/sandbox/rev/25178bb3e8f5
On 6/6/2018 3:58 AM, Weijun Wang wrote:
SSLStringize.java:
1. I assume the toString() method does not change the internal status of a
ByteBuffer? Is it worth mentioning this in the spec?
2. Should this interface and all its implementations be renamed to
***Stringnizer?
Good points! Updated in the above changeset.
3. Lot of typo: ***Concumer. You can probably "cd sun/security/ssl" and run
perl -i -pe 's/Concumer/Consumer/g' *.java
The issues had been updated in another changeset.
The whole SSLStringize + children classes looks complicated to me. Since the
implementation is almost always creating a SSLExtensionSpec object and calling
its toString, is it possible to register this SSLExtensionSpec class name in
each enum value in SSLExtension and somehow simplify the design? Something like
--
In constructor of SSLExtension, change the last parameter from
"SSLStringize stringize" to "Class<? extends SSLExtensionSpec> specClazz"
store the value into a private field named specClazz, and
then you can change "stringize.toString(byteBuffer)" to something like
"specClazz.newInstance(byteBuffer).toString()".
There is one or two exceptions that the
specClazz.newInstance(byteBuffer) does not apply. If we use this style,
we would have a limit on the extension constructor. I would prefer to
have the flexibility in the SSLExtensionSpec implementation.
Thanks,
Xuelei
That said, I know you must have thought about this much more. If there is any
space for enhancement, we can always refactor it later.
Thanks
Max
