On 07/09/2014 10:02 AM, Wang Weijun wrote:
On Jul 9, 2014, at 20:00, Sean Mullan <sean.mul...@oracle.com> wrote:
Looks good, just one comment in GssKrb5Base - I would change
getNegotiatedProperty to call the superclass method first, and then
if that returns null, check for the gss inquiretype properties.
This way you don't check for IllegalStateExc twice, and it seems
cleaner to me.
My understanding is that if getNegotiatedProperty() methods of both
parent class and child class return non-null, then the value from the
child class should shadow the one from the parent.
That behavior is unspecified as far as I can see. In any case, if you choose
that behavior, you could still call the superclass first and then override if
necessary.
It is unspecified because the SASL API has not assigned any default value for
getNegotiatedProperty() in SaslClient and SaslServer classes, and it's our
implementation that defines AbstractSaslImpl and GssKrb5Base, and both have
getNegotiatedProperty(). I choose child class shadowing the parent not only
because it looks natural but also the other subclasses of AbstractSaslImpl
(DigestMD5Base) does the same.
Now, if I call the super class method first, it means if the name is really defined in
both classes, codes in both sides will be executed and it's a negative performance
impact. If we call child class first, "if (!completed)" is checked twice but
its impact is much lighter.
This seems kind of odd though in that an abstract superclass might have
properties with the same name when these are specific to the GSS/KRB5
subclass. Anyway, this was mostly a minor comment, whatever you decide
here is ok.
--Sean
