Re: [External] : Re: Some classes could be sealed

2023-02-01 Thread Kevin Rushforth



You can't extend these without tampering with internals.


Pretty much, yes.

Node is an abstract class that requires a concrete implementation to be 
useful. The set of subclasses that can be used in describing and 
rendering the scene graph is a finite and known set. The rendering of 
the scene graph is an implementation detail; each node in the scene 
graph has a corresponding peer (an NGNode subclass) that is needed to 
implement various node types (shapes, images, etc).


So Node, as well as its abstract subclasses, like Shape, Shape3D, 
Camera, and LightBase, needs a known concrete subclass in order to do 
anything. Similarly, Material (which is not a Node) is abstract and has 
implementation that cannot be provided by an application class.


By contrast, Parent can be usefully subclassed. It is a concrete class 
that is used as a container for other nodes, and has implementation of 
layout, traversal, bounds computation, etc.


--- Kevin

On 2/1/2023 2:48 PM, Nir Lisker wrote:
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 
 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
 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ß
 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
 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

Re: [External] : Re: Some classes could be sealed

2023-02-01 Thread 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 
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 
> 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ß <
>> michaelstr...@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 
>>> 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.
>>> >
>>>
>>
>>
>>
>


Re: [External] : Re: Some classes could be sealed

2023-02-01 Thread John Hendrikx
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 
 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ß
 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
 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.
>







Re: [External] : Re: Some classes could be sealed

2023-02-01 Thread Nir Lisker
I held on proposing sealed class changes because they will be more
beneficial once switch patterns are introduced. I also held on refactoring
to records (of which I have a branch in my fork) for the reason that record
patterns are not out yet. I think that once we have more pieces of the
algebraic data types puzzle it will be very beneficial to do a decent
amount of refactoring. There's nothing stopping us from starting with what
we have for the purpose of upgrading errors from runtime to compile time,
but we will need a second iteration anyway in some of these cases.

On Wed, Feb 1, 2023 at 9:37 PM 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 
> 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ß <
>> michaelstr...@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 
>>> 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.
>>> >
>>>
>>
>>
>>
>


Re: [External] : Re: Some classes could be sealed

2023-02-01 Thread Kevin Rushforth
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 
 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ß
 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
 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.
>







Re: Some classes could be sealed

2023-02-01 Thread Nir Lisker
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 
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ß <
> michaelstr...@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 
>> 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.
>> >
>>
>
>
>


Re: Some classes could be sealed

2023-02-01 Thread Kevin Rushforth
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ß 
 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
 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.
>





Re: Some classes could be sealed

2023-02-01 Thread Kevin Rushforth
Yes, now that JDK 17 is the minimum we can consider doing this. As you 
mentioned, it would provide earlier notification of the error: 
compile-time versus runtime.


One thing to add is that in addition to sealing Node, we would leave at 
least Shape, Shape3D, Camera, and LightBase sealed, since they are also 
not extensible and throw a similar runtime exception.


-- Kevin


On 2/1/2023 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ß 
 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
 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.
>



Re: Some classes could be sealed

2023-02-01 Thread Philip Race
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ß 
 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
 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.
>



Re: Some classes could be sealed

2023-02-01 Thread Thiago Milczarek Sayão
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ß 
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 
> 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.
> >
>


Re: Some classes could be sealed

2023-02-01 Thread Michael Strauß
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  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.
>


Re: Some classes could be sealed

2023-02-01 Thread John Hendrikx
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.


Some classes could be sealed

2023-02-01 Thread Thiago Milczarek Sayão
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.