[External] : Re: Some classes could be sealed

[Nir Lisker]

For Material and LightBase it's because they are just facades whose
implementation is in native code. You can't extend these without tampering
with internals. I think that Camera and Shape3D also requires modifying
internal stuff, though not at the native level.

On Thu, Feb 2, 2023 at 12:38 AM John Hendrikx <john.hendrikx at gmail.com>
wrote:

> I'm curious to know why these classes are not allowed to be subclassed
> directly, as that may be important in order to decide whether these classes
> should really be sealed.
>
> --John
> On 01/02/2023 20:37, Kevin Rushforth wrote:
>
> I read the spec for sealed classes more carefully, and it turns out we
> can't make Node sealed. At least not without API changes to SwingNode and
> MediaView (and implementation changes to Printable in the javafx.web
> module). All of the classes listed in the "permits" clause must be in the
> same module, and SwingNode (javafx.swing) and MediaView (javafx.media)
> extend Node directly they would need to be "permitted" subtypes, but there
> is no way to specify that. We would also need to do something about the
> tests that extend Node and run in the unnamed module. So this doesn't seem
> feasible.
>
> We could still seal Shape, Shape3D, LightBase, and Material, since all
> permitted implementation are in the javafx.graphics module. It may or may
> not be worth doing that.
>
> -- Kevin
>
>
> On 2/1/2023 9:45 AM, Nir Lisker wrote:
>
> I'll add that internal classes, mostly NG___ peers, can also benefit from
> sealing. NGLightBase is an example.
>
> Material is another public class that can be sealed.
>
> On Wed, Feb 1, 2023 at 7:37 PM Kevin Rushforth <kevin.rushforth at oracle.com>
> wrote:
>
>> I agree that we should only seal existing classes that could not have
>> been extended by application classes. The ones I listed in my previous
>> email fit that bill, since an attempt to subclass them will throw an
>> exception when it is used in a scene graph. Each documents that subclassing
>> is disallowed.
>>
>> Btw, we've already started making use of pattern-matching instanceof in
>> the implementation anyway. It would be the first API change that relies on
>> a JDK 17 feature, but for JavaFX 21, I see no problem in doing that.
>>
>> -- Kevin
>>
>>
>> On 2/1/2023 9:06 AM, Philip Race wrote:
>>
>> In the JDK we've only sealed  existing classes which provably could not
>> have been extended by application classes,
>> so I'm not sure about this ..
>>
>> also I think that might be the first change that absolutely means FX 21
>> can only be built with JDK 17 and later ..
>>
>> -phil
>>
>> On 2/1/23 8:59 AM, Thiago Milczarek Sayão wrote:
>>
>> Yes, sorry, I made the email title in plural, but I meant what Michael
>> said, Node would be sealed permitting only what is needed for JavaFx
>> internally.
>>
>>
>> -- Thiago
>>
>>
>> Em qua., 1 de fev. de 2023 às 13:48, Michael Strauß <
>> michaelstrau2 at gmail.com> escreveu:
>>
>>> I don't think that's what Thiago is proposing. Only `Node` would be
>>> sealed.
>>> The following subclasses would be non-sealed: Parent, SubScene,
>>> Camera, LightBase, Shape, Shape3D, Canvas, ImageView.
>>> And then there are additional subclasses, which don't fit into this
>>> idea since they are in other modules: SwingNode (in javafx.swing),
>>> MediaView (in javafx.media), Printable (in javafx.web).
>>>
>>>
>>>
>>> On Wed, Feb 1, 2023 at 5:39 PM John Hendrikx <john.hendrikx at gmail.com>
>>> wrote:
>>> >
>>> > I think this may be a bit unclear from this post, but you're proposing
>>> I think to make `Node`, `Shape` and `Shape3D` sealed.  For those unaware,
>>> you're not allowed to extend these classes (despite being public).  For
>>> example Node says in its documentation:
>>> >
>>> >    * An application should not extend the Node class directly. Doing
>>> so may lead to
>>> >    * an UnsupportedOperationException being thrown.
>>> >
>>> > Currently this is enforced at runtime in NodeHelper.
>>> >
>>> > --John
>>> >
>>> > On 01/02/2023 15:47, Thiago Milczarek Sayão wrote:
>>> >
>>> > Hi,
>>> >
>>> > NodeHelper.java has this:
>>> >
>>> > throw new UnsupportedOperationException(
>>> >         "Applications should not extend the "
>>> >         + nodeType + " class directly.");
>>> >
>>> >
>>> > I think it's replaceable with selead classes. Am I right?
>>> >
>>> > The benefit will be compile time error instead of runtime.
>>> >
>>> >
>>> > -- Thiago.
>>> >
>>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20230202/5f5bf300/attachment.htm>