RFR: 8323706: Move SimpleSelector and CompoundSelector to internal packages [v3]

[Andy Goryachev]

On Fri, 19 Jan 2024 09:33:15 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> modules/javafx.graphics/src/main/java/javafx/css/Selector.java line 102:
>> 
>>> 100: 
>>> 101:             return new Match(this, s.getPseudoClassStates(), idCount, styleClassCount);
>>> 102:         }
>> 
>> I presume you are moving the implementations to this base class because some of the types, constructors (e.g., Match), or methods only have package visibility? Using the accessor pattern via a Helper class is usually how we deal with this. Have you considered that? It would allow the implementation to remain in the subclasses.
>
> Yes, correct, `CompoundSelector` accesses the package private fields `idCount` and `styleClassCount` of `Match` directly, which it can't do anymore after being moved to a different package; these lines:
> 
>                 idCount += match.idCount;
>                 styleClassCount += match.styleClassCount;
> 
> I'm aware of the Helper classes, but I feel that they are a much more invasive concept (and also harder to follow) to achieve this than doing a pattern match with `instanceof` (which can be replaced with pattern matches for switch once we can use Java 21).  However, if you think this is a requirement, I'm happy to change it -- that said, we're not locked in either choice as far as I can see.
> 
> Alternatively, with everything needed in `Selector` being publicly accessible, I'm not sure if the match creation really needed to be in `Selector` or its subtypes at all.  It feels like more a job for an external type to handle (like how you don't write serialization logic for JSON or XML in each subtype).  If it were up to me, I'd probably create a static method in `Match` which given a `Selector` creates the match.  That way, no `Match` internals need exposing at all.  I could still do this, as the method needed could be package private, and then all the match fields can be made fully private.

what do you think of

Match MatchHelper.create(SimpleSelector)
Match MatchHelper.create(CompoundSelector)

?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1333#discussion_r1670912593