Re: Avoiding NPEs caused by indirect call

From:

Royan <romayankin@gmail.com>

Newsgroups:

comp.lang.java.programmer,comp.lang.java.gui

Date:

Mon, 4 Aug 2008 08:01:06 -0700 (PDT)

Message-ID:

<8d61e9e0-b62d-425a-9a95-ca7ac9e8ea49@d77g2000hsb.googlegroups.com>

On 4 =C1=D7=C7, 05:29, Mark Space <marksp...@sbc.global.net> wrote:

Royan wrote:

abstract class AbstractModel {
=9A =9A public AbstractModel() {
=9A =9AindirectCall();
=9A =9A }

=9A =9A private void indirectCall() {
=9A =9AsetSomeValue(new Integer(1));
=9A =9A }

=9A =9A public void setSomeValue(Integer value) {
=9A =9A =9A =9A firePropertyChange("someProperty", null, value);
=9A =9A }

As Lew said, the chained call from the constructor is the problem. Never
ever do this. =9ACalling any "foriegn" method too (like "indirectCall()" =

)

is broken because you don't know what they will call. =9AIf they call a
public, overriden method (which happens here), then you could be in a
lot of trouble.

Joshua Bloch describes almost the exact code you have with a big "Don't
Do This!" sign next to it. =9AIt's Item 17: Design and Document for
Inheritance or Else Prohibit It, in Effective Java 2nd edition.

A couple of classic solutions: =9Ause composition instead of inheritance.
=9A Use a static factory (maybe with a Strategy Pattern to "plug in" the
exact Model you want).

Composition would use something like the Decorator Pattern. =9AUse a
concrete class, DefaultModel, instead of an abstract class
AbstractModel, and wrap the new class around the default one.

But we kinda don't have enough info to help you out of this, it's very
much a design issue. =9A"indirectCall" is the problem, you can NEVER do
this and expect it to work well. =9AIt must go. =9AWhat are you really
trying to do here?

OK, sorry for making that too confusing, my only intention was to
create a small test case and get things crystal-clear. I have faced
that in two cases.

First was when I derived from DefaultTableModel and overridden a
couple of its methods that make use of internal vectors.

And the second one (which has made me to start this thread) was when I
derived from the JDialog and overridden addXxxx and fireXxx methods.
Because this case is somewhat easier to reproduce I've written a very
small test case that demonstrates my original problem:

import java.beans.PropertyChangeListener;
import java.beans.PropertyChangeSupport;

import javax.swing.JDialog;

class SampleDialog extends JDialog {
    private final PropertyChangeSupport propertyChangeSupport;

    public SampleDialog(JDialog owner) {
        super(owner);
        propertyChangeSupport = new PropertyChangeSupport(owner);
    }

    @Override
    public void addPropertyChangeListener(PropertyChangeListener
listener) {
        propertyChangeSupport.addPropertyChangeListener(listener);
    }

    @Override
    public void firePropertyChange(String propertyName, Object
oldValue, Object newValue) {
        propertyChangeSupport.firePropertyChange(propertyName,
oldValue, newValue);
    }
}

public class Test {
    public static void main(String[] args) {
        SampleDialog sd = new SampleDialog(new JDialog());
    }
}

The stack trace here:

Exception in thread "main" java.lang.NullPointerException
    at SampleDialog.firePropertyChange(Test.java:21)
    at java.awt.Container.setFocusTraversalPolicy(Container.java:3220)
    at sun.awt.SunToolkit.checkAndSetPolicy(SunToolkit.java:501)
    at java.awt.Dialog.<init>(Dialog.java:665)
    at java.awt.Dialog.<init>(Dialog.java:499)
    at javax.swing.JDialog.<init>(JDialog.java:409)
    at javax.swing.JDialog.<init>(JDialog.java:361)
    at javax.swing.JDialog.<init>(JDialog.java:336)
    at SampleDialog.<init>(Test.java:10)
    at Test.main(Test.java:28)

As you can see the root of the problem is that
SunToolkit#checkAndSetPolicy(SunToolkit.java:501) calls
setFocusTraversalPolicy which eventually causes NPE

I'm afraid that nothing can actually be done except for things that
have already been mentioned, but you might have another idea :)