RFR: 8154231: Simplify access to System properties from JDK code

Claes Redestad claes.redestad at oracle.com
Wed Apr 20 16:12:23 UTC 2016


Thanks for looking at this, Ulf!

On 2016-04-20 17:57, Ulf Zibis wrote:
> Hi,
>
> here my comments:
>
> Am 20.04.2016 um 16:44 schrieb Claes Redestad:
>> Hello,
>>
>> now that the sun.security.action package is encapsulated we can 
>> simplify internal code to get System properties.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8154231
>> Webrev: http://cr.openjdk.java.net/~redestad/8154231/webrev.01/
>>
>> This adds a few convenience methods to GetPropertyAction[1] and 
>> GetIntegerAction[2].
>
> - In the original version of ProcessBuilder.java line 472 there was a 
> nice example using lambda expression. Wouldn't this be applicable also 
> for these new methods here?

While perhaps a nice example in isolation, creating a lambda when 
there's already a specialized convenience class available expressing the 
same thing doesn't add any value.

> - Isn't the "theProp" naming style something from an old usage where 
> all members have been named myXyz? I more would like "propName" or 
> just "property".

I chose to go with keeping names in line with existing methods. If 
anything is to be done about this I'd prefer changing all names in the 
class to be consistent and modern, and that'd be a separate RFE.

> - In Integer.getProperty the default substitution is done twice, 
> lines: 143, 148. So maybe return to the "immediate return in if...else 
> clause" style.

The call to Integer.getInteger on line 143 was supposed to be without 
the defaultVal param to avoid always boxing the defaultVal. Fixed in-place.

Thanks!

/Claes


More information about the security-dev mailing list