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