On 10/06/2012 01:30 PM, Alan Bateman wrote:
On 05/10/2012 22:31, Remi Forax wrote:
Hi Alan,
just some minor nits,
in Properties.XmlSupport, 'provider' should be PROVIDER because it's
a constant and
instead of using loadProvider(), you should initialize PROVIDER in a
static block.
line 1172, you declare a provider and line 1174, you initialize it,
I think line 1174 should do the declaration and the initialization on
the same line.
Thanks Rémi. You're right, it probably should be PROVIDER rather than
"provider", I'll change that before I push this. I think I'll leave
the initialization in the 3 methods rather than a jolly giant static
initializer, just to keep it readable. I can move the declaration of
provider to L1174, that is a bit nicer.
Thanks Alan,
I'm fine with methods loadProviderAsService and
loadProviderFromProperty, but I think you can remove loadProvider
because PROVIDER can be initialized in a static block.
private static final XmlPropertiesProvider PROVIDER;
static {
PROVIDER = AccessController.doPrivileged(
new PrivilegedAction<XmlPropertiesProvider>() {
...
}});
}
-Alan.
cheers,
Rémi
