Hi Max,
yes, comment states that this constructor is only for sub-classes so
I've made it protected. Compilation didn't broke (at least in jdk and
tests).
AFAIK extensionId should never be null, as for extensionValue there are
extensions without value (e.g. OCSPNocheck) but in that case value is
set to empty byte array.
I've reverted checks in equal and hashCode to let NPE be thrown:
http://cr.openjdk.java.net/~snikandrova/8054536/webrev.03/
<http://cr.openjdk.java.net/%7Esnikandrova/8054536/webrev.03/>
BTW, should I rename bug and review? Current title seems to be irrelevant.
Thank you,
Svetlana
On 18.07.2016 19:09, Wang Weijun wrote:
If the constructor is only meant to be called by a child class, can we just
change it to protected?
Also, if extensionId and extensionValue should never be null, we should focus
on not to create an Extension that has one of 2 fields as null. Inside this
class, we can add Objects::requireNonNull checks in the 3-arg constructor and
the static newExtension method.
In case we cannot guarantee them to be non-null (For example, a child class has not fill
in these 2 fields), I'd rather fail loudly instead of returning null or "null".
In my humble opinion, throwing NPE in hashCode and equals is not really a bad thing.
Thanks
Max
On Jul 18, 2016, at 11:40 PM, Svetlana Nikandrova
<svetlana.nikandr...@oracle.com> wrote:
Artem,
please see updated review:
http://cr.openjdk.java.net/~snikandrova/8054536/webrev.02/
<http://cr.openjdk.java.net/%7Esnikandrova/8054536/webrev.02/>
Thanks,
Svetlana
