7

I have a class that extends JPanel. In its constructor I'm passing this to other methods, mainly to add the jpanel object as a listener to containers/controls within the jpanel (but also other objects). Since Netbeans shows a leaking this in constructor warning for those calls I've put them in an other method that is called from the constructor.

before:

class Foo ... {
    public Foo() {
      initComponents();
      tabX.addChangeListener(this); // <- netbeans complains here
    }

after:

class Foo ... {
    public Foo() {
      initComponents();
      initListeners();
    }

    protected void initListeners() {
      tabX.addChangeListener(this);
    }

That gets rid of the symptom. But I doubt it fixes the reason why netbeans shows the warning.
Where is the proper place to do this kind of initialization in a JPanel-derived class?

chendral
  • 694
  • 1
  • 6
  • 17

3 Answers3

2

I wonder if there's a bigger issue at play here -- that of asking your class to do too much. A class should have one main purpose, and a view should be responsible for view and that's it. To have it do model or control functions and you lose cohesion, may increase coupling and risk creating god objects that are difficult if not impossible to debug or extend. So to put it bluntly, your GUI or view classes should not also be listener classes. In other words, there is no good reason and a lot of bad reasons for a GUI class to also implement a listener interface.

The best solution: don't have your GUI classes implement listeners. Instead either use anonymous inner classes, or private inner classes, or if complex enough or you anticipate extending and/or modifying your code in the future, stand alone listener classes.

Hovercraft Full Of Eels
  • 283,665
  • 25
  • 256
  • 373
  • 1
    For stand-alone listeners, [*package-private*](http://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html) access may be useful. – trashgod Mar 19 '12 at 18:48
  • 2
    Anonymous inner classes don't solve the problem of the leaking `this`, though, if their instances are created in the constructor. It only makes the problem harder to spot since now `this` leaks implicitly. – Sergei Tachenov May 07 '15 at 17:43
0

I'm assuming you're probably adding your JPanel extension to some other component (e.g. JFrame, JApplet, another JPanel, etc.). You mentioned that you've got a bit of a mix between needing to add the panel to sub-components within that panel and "other objects" that the panel needs to listen to. It would probably be better to add the panel to those "other objects" near the place where you add your JPanel extension to its enclosing JFrame or other parent component, outside of your extension's class definition.

However, for the sub-components of your panel that your panel must listen to, I think what you're doing is fine, provided those sub-components aren't visible to objects outside of your JPanel extension class definition. The warning is simply there to indicate that what you're doing may be unsafe, but ultimately, when your panel gets garbage-collected, so will all of the sub-components that it owns, including any listener lists they maintain that point back to your JPanel extension. Because of this fact, I think that putting the add*Listener(this) call in an aptly-named private method of your JPanel extension and calling it from your constructor is fine.

The other option would be to use Eclipse so you don't get those warnings anymore... (totally joking ;).

CodeBlind
  • 4,519
  • 1
  • 24
  • 36
0

The reason of this warning is that you are passing this while the constructor is not finished and thus the object is not fully initialized. Even you use it at the end of your constructor, chances are that your class is extended and there is a constructor of subclass which is to be executed yet. In your case (registering the object as a listener) this is safe, as Swing is single-threaded and events will be passed to the listeners only after your object is initialized.

Alexei Kaigorodov
  • 13,189
  • 1
  • 21
  • 38