RFR: 8345398: Avoid redundant Properties.containsKey call in Cursor.getSystemCustomCursor

Alexey Ivanov aivanov at openjdk.org
Wed Dec 4 13:47:42 UTC 2024


On Fri, 1 Nov 2024 13:08:54 GMT, Andrey Turbanov <aturbanov at openjdk.org> 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


More information about the client-libs-dev mailing list