On Thu, 10 Nov 2022 23:31:36 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> Michael Strauß has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Update PlatformPreferences when dark mode is enabled
>
> modules/javafx.graphics/src/main/java/javafx/application/PlatformPreferences.java
>  line 117:
> 
>> 115:  * @since 20
>> 116:  */
>> 117: public interface PlatformPreferences extends Map<String, Object> {
> 
> Are you sure it is a good idea to expose these as a `Map`?  It seems to me 
> that this isn't that good a practice any more to have classes implement or 
> extend lists/maps/sets as they pull in a huge amount of API surface that is 
> mostly useless or too general for the class's purpose.
> 
> The addition of the listener management methods also has me wondering, as 
> `ObservableMap` does something similar.

All of the mutating methods are useless, since the implementation always 
returns a read-only map. However, the alternative would be to duplicate APIs to 
enumerate and inspect the contents of the `PlatformPreferences` map 
(applications might want to show a list of available preferences). I'm not sure 
that's preferable, mostly because `PlatformPreferences` _does_ represent a 
mapping of keys to values.

It's true that listener management makes it look like an `ObservableMap`. The 
difference is that `ObservableMap` doesn't support batch change notifications, 
which the current implementation relies on to minimize the number of style 
theme resets. Of course, that could be solved in a different way.

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

PR: https://git.openjdk.org/jfx/pull/511

Reply via email to