RFR: 8301302: Platform preferences API [v23]

[Michael Strauß]

On Fri, 17 Nov 2023 19:43:58 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Rename Appearance to ColorScheme
>
> modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 630:
> 
>> 628:          * @throws NullPointerException if {@code key} is null
>> 629:          * @throws IllegalArgumentException if a mapping exists, but the key is not mapped to an {@code Integer}
>> 630:          * @return the optional {@code Integer} to which the key is mapped
> 
> I presume that it returns `Optional.empty()` if the mapping is not present? This could lead to a situation where calling this method with a particular key will return an empty optional in some cases and throw an exception in others. Might it be better to specify that it will throw IAE if the type of the key is known to not be an Integer, whether or not there exists a mapping for that key? Alternatively, might it be better to always return an empty Optional unless there exists a mapping and the value is an integer? In the latter case, we would get rid of "IAE" entirely.

Alternative 2 (return empty value if the key maps to a different type) is attractive because it doesn't require additional type validation for mappings that are not present, and gets rid of the sometimes unexpected IAE.

However, there's a major downside: the preferences map is, first and foremost, a `Map`. The typed getters are just a convenience to make it easier to work with this map. When a native toolkit reports a key-value mapping, it will be included in the map, and its value can be unconditionally retrieved with `Map.get(String)`.

When we use a typed getter, it should return `Optional.empty()` exactly if `get(String)` would return `null`. Returning an empty value when the wrong typed getter is called (which is a bug) is unexpected and basically hides what would be the equivalent of a `ClassCastException`. This rules out this approach.

Alternative 1 (always throw IAE if we call the wrong typed getter, whether or not we have a mapping) is not unexpected, and does not violate the `Optional.empty() <=> get(String)==null` invariant. This alternative is a bit heavier on the implementation side, as we must now keep well-known lists of key-type mappings around, and more importantly, keep them in sync with the native toolkit.

I've implemented this alternative and updated the specification of the typed getters similar to this:

/**
 * Returns an optional {@code Integer} to which the specified key is mapped.
 *
 * @param key the key
 * @throws NullPointerException if {@code key} is null
 * @throws IllegalArgumentException if the key is not mappable to an {@code Integer}
 * @return the optional {@code Integer} to which the key is mapped
 */

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1398099493