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
