On Fri, 1 Nov 2024 13:08:54 GMT, Andrey Turbanov <[email protected]> wrote:
> `Properties` doesn't allow `null` values. > We can replace containsKey+getProperty with getProperty+null check. > It's clearer and a bit faster. Anyway, it looks good to me. Messing around with `final` doesn't bring much value. However, it may make the code look more consistent. src/java.desktop/share/classes/java/awt/Cursor.java line 300: > 298: loadSystemCustomCursorProperties(); > 299: > 300: String prefix = CURSOR_DOT_PREFIX + name; I'd mark `prefix` as `final`, it's used throughout the method, which highlights the inconsistency in applying `final`. src/java.desktop/share/classes/java/awt/Cursor.java line 301: > 299: > 300: String prefix = CURSOR_DOT_PREFIX + name; > 301: String key = prefix + DOT_FILE_SUFFIX; After you removed `containsKey(key)`, `key` is used only once, and `prefix + DOT_FILE_SUFFIX` can be inline in the call to `systemCustomCursorProperties.getProperty` to get the file name. This would align with other usages of `prefix` in the method. ------------- Marked as reviewed by aivanov (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/21824#pullrequestreview-2478650911 PR Review Comment: https://git.openjdk.org/jdk/pull/21824#discussion_r1869526952 PR Review Comment: https://git.openjdk.org/jdk/pull/21824#discussion_r1869519299
