Reputation: 13859
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
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