RFR: 8301302: Platform preferences API [v18]

[Michael Strauß]

On Tue, 31 Oct 2023 20:58:55 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Michael Strauß has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - formatting
>>  - Javadoc change
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/application/preferences/PlatformPreferences.java line 52:
> 
>> 50:  * by calling the {@link #update(Map)} method.
>> 51:  */
>> 52: public class PlatformPreferences extends AbstractMap<String, Object> implements Platform.Preferences {
> 
> Is there a need for this class to be public?  It seems to me that `Platform.Preferences` is public, and that in order to get the platform preferences you call `Platform.getPreferences()`, which returns the interface.
> 
> Otherwise, you need to document all `public` methods (not from the interface), including the constructor.

`PlatformPreferences` is in the `com.sun.javafx.application.preferences` package, but must be accessible from the `com.sun.javafx.application` package. That's why it needs to be public. The class doesn't have undocumented public methods (aside from the constructor), but then again this class is also not API.

> modules/javafx.graphics/src/main/java/com/sun/javafx/application/preferences/PlatformPreferences.java line 61:
> 
>> 59:     final Map<String, Object> effectivePreferences = new HashMap<>();
>> 60:     final Map<String, Object> unmodifiableEffectivePreferences = Collections.unmodifiableMap(effectivePreferences);
>> 61:     final PreferenceProperties properties = new PreferenceProperties(this);
> 
> minor: `this` escapes here before object is fully constructed

Yes, but `PreferenceProperties` doesn't do anything with `this`.

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

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