<div dir="ltr">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.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Feb 1, 2023 at 9:37 PM Kevin Rushforth <<a href="mailto:kevin.rushforth@oracle.com">kevin.rushforth@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
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.<br>
<br>
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.<br>
<br>
-- Kevin<br>
<br>
<br>
<div>On 2/1/2023 9:45 AM, Nir Lisker wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">I'll add that internal classes, mostly NG___ peers,
can also benefit from sealing. NGLightBase is an example.
<div><br>
</div>
<div>Material is another public class that can be sealed.</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Wed, Feb 1, 2023 at 7:37 PM
Kevin Rushforth <<a href="mailto:kevin.rushforth@oracle.com" target="_blank">kevin.rushforth@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div> 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.<br>
<br>
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.<br>
<br>
-- Kevin<br>
<br>
<br>
<div>On 2/1/2023 9:06 AM, Philip Race wrote:<br>
</div>
<blockquote type="cite"> In the JDK we've only sealed
existing classes which provably could not have been
extended by application classes,<br>
so I'm not sure about this .. <br>
<br>
also I think that might be the first change that
absolutely means FX 21 can only be built with JDK 17 and
later ..<br>
<br>
-phil<br>
<br>
<div>On 2/1/23 8:59 AM, Thiago Milczarek Sayão wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div>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.</div>
<div><br>
</div>
<div><br>
</div>
<div>-- Thiago<br>
</div>
<br>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">Em qua., 1 de fev.
de 2023 às 13:48, Michael Strauß <<a href="mailto:michaelstrau2@gmail.com" target="_blank">michaelstrau2@gmail.com</a>>
escreveu:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I don't think
that's what Thiago is proposing. Only `Node` would
be sealed.<br>
The following subclasses would be non-sealed:
Parent, SubScene,<br>
Camera, LightBase, Shape, Shape3D, Canvas,
ImageView.<br>
And then there are additional subclasses, which
don't fit into this<br>
idea since they are in other modules: SwingNode (in
javafx.swing),<br>
MediaView (in javafx.media), Printable (in
javafx.web).<br>
<br>
<br>
<br>
On Wed, Feb 1, 2023 at 5:39 PM John Hendrikx <<a href="mailto:john.hendrikx@gmail.com" target="_blank">john.hendrikx@gmail.com</a>>
wrote:<br>
><br>
> 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:<br>
><br>
> * An application should not extend the Node
class directly. Doing so may lead to<br>
> * an UnsupportedOperationException being
thrown.<br>
><br>
> Currently this is enforced at runtime in
NodeHelper.<br>
><br>
> --John<br>
><br>
> On 01/02/2023 15:47, Thiago Milczarek Sayão
wrote:<br>
><br>
> Hi,<br>
><br>
> NodeHelper.java has this:<br>
><br>
> throw new UnsupportedOperationException(<br>
> "Applications should not extend the "<br>
> + nodeType + " class directly.");<br>
><br>
><br>
> I think it's replaceable with selead classes.
Am I right?<br>
><br>
> The benefit will be compile time error instead
of runtime.<br>
><br>
><br>
> -- Thiago.<br>
><br>
</blockquote>
</div>
</blockquote>
<br>
</blockquote>
<br>
</div>
</blockquote>
</div>
</blockquote>
<br>
</div>
</blockquote></div>