On Wed, 28 Aug 2024 08:54:39 GMT, Abhishek Kumar <[email protected]> wrote:
> `static final Map<String,GTKStockIcon> ICONS_MAP` is modified only in > `static` block. Then > [com.sun.java.swing.plaf.gtk.GTKStyle#get](https://github.com/kumarabhi006/jdk/blob/73f7a5f15dbba54a98f3916ff1190520ac07874d/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKStyle.java#L892) > method read from it within `synchronized (ICONS_MAP)` block. As this map is > read-only and all its content is initialized in static block we can safely > remove synchronized from get call. It looks good to me as it is. However, I'd go further and use either `Map.of` or `Collection.unmodifiableMap` to emphasise the fact that the map is read-only. src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKStyle.java line 893: > 891: GTKStockIcon stockIcon = ICONS_MAP.get(key); > 892: > 893: if (stockIcon != null) { Suggestion: GTKStockIcon stockIcon = ICONS_MAP.get(key); if (stockIcon != null) { Maybe remove the blank line? It used to separate a block where `stockIcon` was initialised and the condition. Now it's better to keep these two lines closer. ------------- PR Review: https://git.openjdk.org/jdk/pull/20741#pullrequestreview-2268627382 PR Review Comment: https://git.openjdk.org/jdk/pull/20741#discussion_r1736137111
