On Mon, 12 Apr 2021 14:09:55 GMT, Conor Cleary <ccle...@openjdk.org> wrote:

>> ### Description
>> This fix is part of a previous effort to both cleanup/modernise JNDI code, 
>> the details of which can be seen in 
>> [JDK-8048091](https://bugs.openjdk.java.net/browse/JDK-8048091). A number 
>> JNDI methods under `java.naming` use Anonymous Inner Classes in cases where 
>> only a single object unique to the requirements of the method is used. The 
>> issues these occurrences of AICs cause are highlighted below.
>> 
>> - AICs, in the cases concerned with this fix, are used where only one 
>> operation is required. While AICs can be useful for more complex 
>> implementations (using interfaces, multiple methods needed, local fields 
>> etc.), Lambdas are better suited here as they result in a more readable and 
>> concise solution.
>> 
>> ### Fixes
>> - Where applicable, occurrences of AICs were replaced with lambdas to 
>> address the issues above resulting primarily in more readable/concise code.
>
> Conor Cleary has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Update copyright headers
>  - Tidied up lambdas

LGTM

src/java.naming/share/classes/com/sun/jndi/ldap/LdapPoolManager.java line 400:

> 398:     private static final String getProperty(final String propName, final 
> String defVal) {
> 399:         PrivilegedAction<String> pa = () -> System.getProperty(propName, 
> defVal);
> 400:         return AccessController.doPrivileged(pa);

Hmmm... This is not strictly equivalent but will work because java.naming is 
loaded by the boot loader and has the permission to read all system properties. 
I guess the code on the left-hand side was written at a time where JNDI was 
still in a stand-alone library?

-------------

Marked as reviewed by dfuchs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3416

Reply via email to