SedJ601
SedJ601

Reputation: 13859

The correct way to handle and extend Node in JavaFx

In my quest to have better coding practices, I have come across a coding idea that I have been doing for a long time and I even see programmers from this site, who I consider experts, use the same coding idea.

For example in the code below I extend Pane. To get the class' children I have always called this.getChildren() in this situation. Today I noticed Netbeans gives a warning message, so I looked it up and found that to get rid of the warning I should use super.getChildren(). Is the recommend way I should handle this situation in the future or is this.getChildren() just as good?

public class FlashCard extends Pane
{    
    FlashCard(int num1, int num2, int answer)
    {

        Label lblNum1 = new Label(Integer.toString(num1));
        Label lblNum2 = new Label(Integer.toString(num2));
        Label lblAnswer = new Label(Integer.toString(answer));

        VBox vbox = new VBox();
        vbox.getChildren().addAll(lblNum1, lblNum2, lblAnswer);          
        super.getChildren().add(vbox);
    }
}

Upvotes: 3

Views: 3017

Answers (1)

James_D
James_D

Reputation: 209553

This is Item 17 from Josh Bloch's "Effective Java" (Second Edition): "Design and document for inheritance or else prohibit it". (I strongly recommend reading this book, if you haven't already.) Specifically:

There are a few more restrictions that a class must obey to allow inheritance. Constructors must not invoke overridable methods.

The basic problem is that if you were to subclass FlashCard and override getChildren() in a way that depended on initialization performed in the subclass constructor, your code would break. The FlashCard constructor would be invoked before the subclass constructor, so it would invoke getChildren() before the initialization had taken place. In a somewhat artificial example:

public class SpecialFlashCard extends FlashCard {

    private ObservableList<Node> subclassChildren ;

    public SpecialFlashCard(int num1, int num2, int answer) {
        super(num1, num2, answer);
        subclassChildren = FXCollections.observableArrayList();
    }

    @Override
    public ObservableList<Node> getChildren() {
        return subclassChildren ;
    }
}

If the FlashCard constructor invoked this.getChildren(), then invoking the subclass constructor would throw a NullPointerException, because the superclass constructor would attempt to add elements to subclassChildren before it was initialized. On the other hand, if the FlashCard constructor invoked super.getChildren(), then the subclass would not behave as expected (because getChildren() would return a list that did not include the labels).

The simplest approaches here are either to prohibit subclassing of FlashCard or to avoid subclassing Pane in the first place.

To prohibit subclassing, make FlashCard final:

public final class FlashCard extends Pane {

    public FlashCard(int num1, int num2, int answer) {

        Label lblNum1 = new Label(Integer.toString(num1));
        Label lblNum2 = new Label(Integer.toString(num2));
        Label lblAnswer = new Label(Integer.toString(answer));

        VBox vbox = new VBox();
        vbox.getChildren().addAll(lblNum1, lblNum2, lblAnswer);          
        this.getChildren().add(vbox);
    }
}

This completely avoids the problem, because now you cannot subclass FlashCard, and so you cannot override its getChildren() method (you are no longer calling an overridable method from the constructor).

An alternative approach (which I tend to prefer), is not to subclass Pane in the first place. This is Item 16 from Effective Java: "Favor composition over inheritance".

public class FlashCard {

    private final Pane pane ;

    public FlashCard(int num1, int num2, int answer) {
        Label lblNum1 = new Label(Integer.toString(num1));
        Label lblNum2 = new Label(Integer.toString(num2));
        Label lblAnswer = new Label(Integer.toString(answer));

        VBox vbox = new VBox();
        vbox.getChildren().addAll(lblNum1, lblNum2, lblAnswer); 

        this.pane = new Pane();         
        this.pane.getChildren().add(vbox);
    }

    public Pane asPane() {
        return pane ;
    }
}

(Note that the constructor does not invoke any overridable methods.)

This allows essentially the same functionality as your existing FlashCard class, but with a slightly modified API. E.g. instead of

FlashCard flashCard = new FlashCard(6, 9, 42);
someContainer.getChildren().add(flashCard);

you do

FlashCard flashCard = new FlashCard(6, 9, 42);
someContainer.getChildren().add(flashCard.asPane());

Upvotes: 5

Related Questions