Sergey Tachenov Sergey Tachenov - 6 months ago 9
Java Question

Calling overridable methods like Swing's add() in constructor

I know that calling overridable methods from constructors is a bad idea. But I also see that it's being done everywhere with Swing, where code like

add(new JLabel("Something"));
occurs in constructors all the time.

Take NetBeans IDE, for example. It is very picky about overridable calls in constructors. And yet, when it generates Swing code, it puts all those
add()
method calls into an
initializeComponents()
method... which is then called from the constructor! A nice way to hide a problem and disable the warning (NetBeans doesn't have a “a private method that calls overridable methods is called from a constructor” warning). But not really a way to solve the problem.

What's going on here? I've been doing it for ages, but always had an uneasy feeling about this. Is there a better way of initializing Swing containers, except for making an additional
init()
method (and not forgetting to call it every time, which is kind of boring)?

Example

Here is an extremely contrived example of how things can go wrong:

public class MyBasePanel extends JPanel {
public MyBasePanel() {
initializeComponents();
}

private void initializeComponents() {
// layout setup omitted
// overridable call
add(new JLabel("My label"), BorderLayout.CENTER);
}
}

public class MyDerivedPanel extends MyBasePanel {
private final List<JLabel> addedLabels = new ArrayList<>();

@Override
public void add(Component comp, Object constraints) {
super.add(comp);
if (comp instanceof JLabel) {
JLabel label = (JLabel) comp;
addedLabels.add(label); // NPE here
}
}
}

Answer

To avoid wiring Swing components together in the constructor, you could simply give the responsibility of the wiring to another object. For instance, you could give wiring duties to a Factory:

public class MyPanelFactory {
    public MyBasePanel myBasePanel() {
        MyBasePanel myBasePanel = new MyBasePanel();
        initMyBasePanel(myBasePanel);
        return myBasePanel;
    }

    public MyDerivedPanel myDerivedPanel() {
        MyDerivedPanel myDerivedPanel = new MyDerivedPanel();
        initMyBasePanel(myDerivedPanel);
        return myDerivedPanel;
    }

    private void initMyBasePanel(MyBasePanel myBasePanel) {
        myBasePanel.add(new JLabel("My label"), BorderLayout.CENTER);
    }
}

Or you could go all out and instantiate all your Swing components with a dependency injection container and have the container trigger the wiring. Here's an example with Dagger:

@Module
public class MyPanelModule {
    static class MyBasePanel extends JPanel {
        private final JLabel myLabel;

        MyBasePanel(JLabel myLabel) {
            this.myLabel = myLabel;
        }

        void initComponents() {
            this.add(myLabel, BorderLayout.CENTER);
        }
    }

    static class MyDerivedPanel extends MyBasePanel {
        private final List<JLabel> addedLabels = new ArrayList<>();

        MyDerivedPanel(JLabel myLabel) {
            super(myLabel);
        }

        @Override
        public void add(Component comp, Object constraints) {
            super.add(comp);
            if (comp instanceof JLabel) {
                JLabel label = (JLabel) comp;
                addedLabels.add(label);
            }
        }
    }

    @Provides MyBasePanel myBasePanel(@Named("myLabel") JLabel myLabel) {
        MyBasePanel myBasePanel = new MyBasePanel(myLabel);
        myBasePanel.initComponents();
        return myBasePanel;
    }

    @Provides MyDerivedPanel myDerivedPanel(@Named("myLabel") JLabel myLabel) {
        MyDerivedPanel myDerivedPanel = new MyDerivedPanel(myLabel);
        myDerivedPanel.initComponents();
        return myDerivedPanel;
    }

    @Provides @Named("myLabel") JLabel myLabel() {
        return new JLabel("My label");
    }
}