RFR: 8313650: Add hasProperties method to MenuItem and Toggle [v3]

[Andy Goryachev]

On Tue, 22 Aug 2023 06:03:01 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Andy Goryachev has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision:
>> 
>>  - javadoc
>>  - contains properties interface
>>  - Merge remote-tracking branch 'origin/master' into 8313650.properties
>>  - review comments
>>  - test
>>  - has properties
>
> modules/javafx.graphics/src/main/java/javafx/scene/ContainsProperties.java line 36:
> 
>> 34:  * @since 22
>> 35:  */
>> 36: public interface ContainsProperties {
> 
> This name fails the "is-a" test "is a contains properties", like for example "is an `EventTarget`" or "is a `Styleable`". Suggestions:
> 
> `ApplicationPropertyProvider`
> `PropertyProvider`
> `ApplicationPropertyAccessor`
> `PropertyAccessor`
> 
> I'd personally go for the most descriptive one (the first one).  It's long, but it's unlikely to be ever encountered as a type for a variable.
> 
> Suggestion for the docs:
> 
>     /*
>      * Provides access to properties for use primarily by application developers.
>      */

Shouldn't it be `ApplicationPropertiesProvider` then, since multiple properties are involved?

> modules/javafx.graphics/src/main/java/javafx/scene/ContainsProperties.java line 48:
> 
>> 46:      * @return true if this object has properties.
>> 47:      */
>> 48:     public boolean hasProperties();
> 
> Suggestion:
> 
>     boolean hasProperties();

personal preference: prefer not to think when I see it in the "Declaration" window in Eclipse.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1215#discussion_r1301825295
PR Review Comment: https://git.openjdk.org/jfx/pull/1215#discussion_r1301838535