On Fri, 5 Jan 2024 01:32:04 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/java/javafx/css/SimpleSelector.java line 81:
>>
>>> 79: * @deprecated for future removal, use {@link
>>> #getStyleClassNames()} instead
>>> 80: */
>>> 81: public List<String> getStyleClasses() {
>>
>> Have you considered keeping this method (instead of adding a new
>> `getStyleClassNames`), and have `FixedCapacitySet` implement `List` to make
>> it work?
>
> This is possible, although `List#get` would not perform too well when it is
> implemented for `FixedCapacitySet.OpenAddressed` as the array used as hash
> table in this class can have gaps (so we'd need to iterate to find the index).
>
> However, I am very sure this method is not used anywhere (not even in 3rd
> party code as it requires casting to access), and I wouldn't encourage its
> use, so I'd be more inclined to remove it completely.
If this method is not used anywhere, why do we need to expose
`getStyleClassNames()` as new API to replace this one? I'm a bit puzzled by
that, especially since you're saying that the API shouldn't be used. Why create
something that shouldn't be used?
I'd rather just document that you shouldn't expect great performance from this
method, and be done with it. Changing API in a performance optimization PR
seems out of scope.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1451652964
