On Wed, 8 Sep 2021 09:32:55 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - adjust testcases to verify the new semantics
>>  - implement review suggestions:
>>     - Use doPriveleged instead of explicit permission checks, to reduce 
>> complexity
>>     - Use DateTimeFormatter and ZonedDateTime instead of Date.toString()
>>     - Use DateTimeFormatter.RFC_1123_DATE_TIME for formatting 
>> SOURCE_DATE_EPOCH dates
>>     - Use Arrays.sort instead of a TreeMap for ordering property keys
>
> src/java.base/share/classes/java/util/Properties.java line 963:
> 
>> 961:         synchronized (this) {
>> 962:             var entries = map.entrySet().toArray(new Map.Entry<?, 
>> ?>[0]);
>> 963:             Arrays.sort(entries, new Comparator<Map.Entry<?, ?>>() {
> 
> This part here, intentionally doesn't use a lambda, since from what I 
> remember seeing in some mail discussion, it was suggested that using lambda 
> in core parts which get used very early during JVM boostrap, should be 
> avoided. If that's not a concern here, do let me know and I can change it to 
> a lambda.

This is a fair concern, but writing out a properties file is a pretty 
high-level operation that doesn't seem likely to be used during bootstrap. 
Also, I observe that the `doPrivileged` block above uses a lambda. So I think 
we're probably ok to use a lambda here. But if you get an inexplicable error at 
build time or at startup time, this would be the reason why.

Assuming we're ok with lambdas, it might be easier to use collections instead 
of arrays in order to preserve generic types. Unfortunately the map is 
`Map<Object, Object>` so we have to do some fancy casting to get the right 
type. But then we can use `Map.Entry.comparingByKey()` as the comparator. (Note 
that this uses lambda internally.)

Something like this might work:


        @SuppressWarnings("unchecked")
        var entries = new ArrayList<>(((Map<String, 
String>)(Map)map).entrySet());
        entries.sort(Map.Entry.comparingByKey());

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

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

Reply via email to