Mostly looks good, just a few comments:
- for completeness, please add @Override tags to all existing methods
that are overridden
- you need to also override the new forEach and getOrDefault methods to
first check if the provider is initialized. See, for example, the get
method.
- nit, say either "security manager" or "Security Manager" but not both.
I would probably use the former since it was used already throughout.
- line 429, replace should be synchronized
- For the methods that require both put and remove permissions, can you
tweak the wording to say:
"... to see if it's ok to set this provider's property values and
remove this provider's properties."
and
"... denies access to set property values or remove properties."
- lines 681-684. I think this should call checkLegacy() like the other
impl methods otherwise legacyStrings might be null and throw an NPE.
- line 733,787 remove indentation and align
- line 749-750, shouldn't that be legacyStrings.compute and BiFunction?
--Sean
On 10/17/2013 06:14 PM, Anthony Scarpino wrote:
Hi,
I need a code review for changes regarding
8025763 Provider does not override new Hashtable methods
http://cr.openjdk.java.net/~ascarpino/8025763/
thanks
Tony
