I made a similar observation about the list of transitions. Looking at Duration, is it normal for a property to unbind itself when it is assigned an illegal value? The Duration property also looks more verbose because Node uses the SimpleObjectProperty concrete class, which is less memory efficient than using an anonymous subclass, and JavaFX tends to use the anonymous class pattern.
I think that interfaces for Node and Duration are fine. An abstract class might be too restrictive. We could make it a non-public class and that would give us more flexibility, but won't serve the user. I would have liked to have a generic interface NodeTransition<T extends Node> where implementing transitions can choose which Node they act on, like Shape, but the Shape transitions have a different name for their methods, so that doesn't work. On Tue, May 9, 2023 at 11:51 AM Marius Hanl <mariush...@web.de> wrote: > I had a look and made list of every transition with their properties: > Transition > -> FadeTransition (node, duration) > -> FillTransition (shape, duration) > -> ParallelTransition (node) > -> PathTransition (node, duration) > -> PauseTransition (duration) > -> RotateTransition (node, duration) > -> ScaleTransition (node, duration) > -> SequentialTransition (node) > -> StrokeTransition (shape, duration) > -> TranslateTransition (node, duration) > > We can save a lot of duplicated code for the duration property since it > always does the same thing, which is calling *setCycleDuration()*. > This also beneficial for other developers so they don't need to deal with > that when creating an own Transition class with a duration. > *setNode()* or *setShape()* are simple property setters. So maybe a mix > of an abstract class and interfaces makes sense here. > > E.g. > Abstract class: TimedTransition > Interface: NodeTransition > Interface: ShapeTransition > > For *SequentialTransition *and *ParallelTransition *we can also think of > an abstract superclass as they share a lot of code. > And this would also be useful for developers as this abstract superclass > deals already with e.g. all the *children* property stuff for you when > extending such superclass. > > -- Marius > > *Gesendet:* Montag, 08. Mai 2023 um 23:18 Uhr > *Von:* "Nir Lisker" <nlis...@gmail.com> > *An:* "Carl Carlec" <carl.car...@gmail.com> > *Cc:* openjfx-dev@openjdk.org > *Betreff:* Re: Define an intermediate superclass for Transitions with a > duration property > I am unconvinced yet that this is a good path. The reason is that > different transitions have different combinations of properties to them. > JDK-8091607 [1] asks for a common superclass for transitions that have a > Node property. JDK-8226456 [2] asks for a common superclass for parent > transitions (although this case is a bit different), Since only 1 > superclass is possible, it might be better to use interfaces for these. The > problem described in the ticket and in [1] is that many instanceof checks > are needed, and this can be relieved with an interface as well. Code > duplication will still be present though. > > I think that some prototyping is required first. > > - Nir > > [1] https://bugs.openjdk.org/browse/JDK-8091607 > [2] https://bugs.openjdk.org/browse/JDK-8226456 > > > On Mon, May 8, 2023 at 10:03 PM Carl Carlec <carl.car...@gmail.com> wrote: > >> Hello JavaFX community, >> I've created a draft PR for an enhancement to Transitions: >> https://github.com/openjdk/jfx/pull/1061 >> JavaFX contains many different Transitions that have a durationProperty() >> and related getter/setter. >> Those transitions benefit from an abstract super class which defines this >> once. >> This will therefore reduce a lot of code duplications or big switch-cases >> to handle all Transitions like the one linked in the ticket. >> >> >> I propose to create a new abstract class called TimedTransition, which >> extends from Transition and defines the durationProperty() + getter/setter. >> All Transitions which define a durationProperty() will now extend from it >> and do not need to define it. >> Those Transitions are affected: FadeTransition, FillTransition, >> PathTransition, PauseTransition, RotateTransition, ScaleTransition, >> StrokeTransition, TranslateTransition. >> >> >> Feedback welcome! >> >> >> Carl >> >
