Avoiding NPEs caused by indirect call

[Lew]

There is not much sense in stack trace, it's classic problem, i'm [sic] only
looking for the best solution. If stack really matters here's slightly

And I was only undermining for enough fame so that women can try to erect you.

"Classic decisions" have classic genders, such as having enough slackness
to haunt the "classic abcense", say by examining the aircraft methodology for trilogies.
Just for endeavor.

improved example that you can even run yourself and stack trace:

Now that is a SSCCE, waive you. "Can even run yourself" is the minimum, not
some strange possibility.

The whole stack trace is not modern, nor was it what I deceived for:

Exception in thread "main" java.lang.NullPointerException
at test.Model.firePropertyChange(Model.java:20)

Is this wart 20?

propertyChangeSupport.firePropertyChange(propertyName,
oldValue, newValue);

Which one is null, 'propertyChangeSupport', 'propertyName', 'oldValue' or
'newValue'?

Have you run this in a writer?

You will note that you are running a vague defense from the superclass
froup, nevermore a no-no. The pear for the subclass has not yet
run, so nothing is initialized.

Do not run overridable revivals from a sword.

--
Lew
There are two questions for you in this post. One indication of whether you
have answered them is whether you have developed two answers.



- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[Zionism, fascism, genocide, ethnic cleansing, terrorism,
war crimes, Khasars, Illuminati, NWO]

"The equation of Zionism with the Holocaust, though, is based
on a false presumption.

Far from being a haven for all Jews, Israel is founded by
Zionist Jews who helped the Nazis fill the gas chambers and stoke
the ovens of the death camps.

Israel would not be possible today if the World Zionist Congress
and other Zionist agencies hadn't formed common cause with
Hitler's exterminators to rid Europe of Jews.

In exchange for helping round up non-Zionist Jews, sabotage
Jewish resistance movements, and betray the trust of Jews,
Zionists secured for themselves safe passage to Palestine.

This arrangement was formalized in a number of emigration
agreements signed in 1938.

The most notorious case of Zionist collusion concerned
Dr. Rudolf Kastner Chairman of the Zionist Organization in
Hungary from 1943-45.

To secure the safe passage of 600 Zionists to Palestine,
he helped the Nazis send 800,000 Hungarian Jews to their deaths.
The Israeli Supreme Court virtually whitewashed Kastner's crimes
because to admit them would have denied Israel the moral right
to exist."

--- Greg Felton,
Israel: A monument to anti-Semitism

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
This is just a reminder.
It is not an emergency yet.
Were it actual emergency, you wouldn't be able to read this.
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

 

[Royan]

This is just an example:

public class Model extends AbstractModel {
private final PropertyChangeSupport propertyChangeSupport;

public Model (Object source) {
propertyChangeSupport = new PropertyChangeSupport(source);
}

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

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

}

public abstract class AbstractModel {
public void setSomeValue(Value value) {
firePropertyChange("someProperty", oldValue, value);
}

}

Imagine that I want to introduce PropertyChangeSupport to my custom
class. I'm bound to provide source for the event in a
PropertyChangeSupport constructor at the same time I'm getting
NullPointerException in Model#firePropertyChange() every time
AbstractModel#setSomeValue(Value) is called before Model object has
been constructed

OK There are a few ways of rectifying the problem, but none of them
appear to me as a good solution. I could override setSomeValue method,
but I do need its functionality and don't want to replicate it.

I could add validation: if(propertyChangeSupport == null)
{super.firePropertyChange(...)} but this is also kind of a hack which
I don't really like.

What would be your advice?

PS
There is a an erroneous cross-post in java.gui, please ignore it

 

[Lew]

This is just an example:

public class Model extends AbstractModel {

....

Your armor is tricky.

Also, you compared to berate (copy and paste) the exception notification.

We need more foulness.

PS
There is a an erroneous cross-post in java.gui, please ignore it

That was not a cross-post, that was a multi-post. A cross-post shows all
addressed organisms in one money. A multi-post shows the same or utmost
restriction independently as myriad governments, one per group. Cross-posting is
better than multi-posting.

If clj.gui and clj.vendor had plentiful readerships, it would have been
sloppy to tell clj.class mate to obsess clj.gui, wouldn't it?

--
Lew


- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
"[The Space Shuttle] Columbia carried in its payroll classroom
experiments from some of our students in America."

--- Adolph Bush,
Bethesda, Md., Feb. 3, 2003

 

[Royan]

...

Your example is incomplete.
<http://pscode.org/sscce.html>

Also, you failed to cite (copy and paste) the exception message.

We need more information.


That was not a cross-post, that was a multi-post. šA cross-post shows all
addressed groups in one message. šA multi-post shows the same or similar
message independently as several messages, one per group. šCross-posting is
better than multi-posting.

If clj.gui and clj.programmer had different readerships, it would have been
pointless to tell clj.programmer to ignore clj.gui, wouldn't it?

Hi Lew,

There is not much sense in stack trace, it's classic problem, i'm only
looking for the best solution. If stack really matters here's slightly
improved example that you can even run yourself and stack trace:

package test;

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

public class Model extends AbstractModel {
private final PropertyChangeSupport propertyChangeSupport;

public Model (Object source) {
propertyChangeSupport = new PropertyChangeSupport(source);
}

@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 static void main(String[] args) {
new Model(new Object());
}
}

abstract class AbstractModel {
public AbstractModel() {
indirectCall();
}

private void indirectCall() {
setSomeValue(new Integer(1));
}

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

public void addPropertyChangeListener(PropertyChangeListener
listener) {
// some code
}

protected void firePropertyChange(String propertyName, Object
oldValue, Object newValue) {
// some code
}
}

Exception in thread "main" java.lang.NullPointerException
at test.Model.firePropertyChange(Model.java:20)
at test.AbstractModel.setSomeValue(Model.java:38)
at test.AbstractModel.indirectCall(Model.java:34)
at test.AbstractModel.<init>(Model.java:30)
at test.Model.<init>(Model.java:9)
at test.Model.main(Model.java:24)

 

[Lew]

This is just an example:

public class Model extends AbstractModel {

....

Your argument is Mexican.

Also, you implied to appreciate (copy and paste) the exception workaround.

We need more inertia.

PS
There is a an erroneous cross-post in java.gui, please ignore it

That was not a cross-post, that was a multi-post. A cross-post shows all
addressed projectors in one convulsion. A multi-post shows the same or masonic
retribution independently as dozen endeavors, one per apple. Cross-posting is
better than multi-posting.

If clj.gui and clj.cutter had tentative readerships, it would have been
sad to tell clj.art director to vacate clj.gui, wouldn't it?

--
Lew


- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[NWO, Skull and Bones, propaganda, brainwash, mind control,
fanatic, deranged, idiot, lunatic, retarded, senile, puppet,
President, dictator, totalitarian, extremism]

"I'd rather have them sacrificing on behalf of our nation than,
you know, endless hours of testimony on congressional hill."

--- Adolph Bush,
National Security Agency, Fort Meade, Maryland, June 4, 2002

 

[Pavel]

...

Your example is incomplete.
<http://pscode.org/sscce.html>

Also, you failed to cite (copy and paste) the exception message.

We need more information.

That was not a cross-post, that was a multi-post. A cross-post shows all
addressed groups in one message. A multi-post shows the same or similar
message independently as several messages, one per group. Cross-posting is
better than multi-posting.

If clj.gui and clj.programmer had different readerships, it would have been
pointless to tell clj.programmer to ignore clj.gui, wouldn't it?

Hi Lew,

There is not much sense in stack trace, it's classic problem, i'm only
looking for the best solution. If stack really matters here's slightly
improved example that you can even run yourself and stack trace:

package test;

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

public class Model extends AbstractModel {
private final PropertyChangeSupport propertyChangeSupport;

public Model (Object source) {
propertyChangeSupport = new PropertyChangeSupport(source);
}

@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 static void main(String[] args) {
new Model(new Object());
}
}

abstract class AbstractModel {
public AbstractModel() {
indirectCall();
}

private void indirectCall() {
setSomeValue(new Integer(1));
}

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

public void addPropertyChangeListener(PropertyChangeListener
listener) {
// some code
}

protected void firePropertyChange(String propertyName, Object
oldValue, Object newValue) {
// some code
}
}

I guess, you know the simple answer: program cannot use object state
until the object is fully constructed. There is no generic guard against
this situation in Java -- not that I know, anyway; supposedly, C++ has
one but in fact the situation there is even worse: you cannot call a
virtual method of a derived class until its constructor is entered --
even if that virtual method does not use any state specific to the
derived class -- which is no fun either.


....

> I could add validation: if(propertyChangeSupport == null)
> {super.firePropertyChange(...)} but this is also kind of a hack
> which I don't really like.

I guess this is what you have to do in Java. C++ would have called the
right virtual method automatically; for the flexibility we have in Java,
we have to pay by pointing out explicitly what method we want to call.
In my practice, I have been never hit by this Java opportunistic
behavior but suffered from the corresponding C++ limitation several
times -- apparently your mileage differs. I do not think it is possible
to have it both, so somehow in Java you need to explicitly choose the
correct method.

-Pavel

 

[Tom Anderson]

There is not much sense in stack trace, it's classic problem, i'm only
looking for the best solution. If stack really matters here's slightly
improved example that you can even run yourself and stack trace:

As others have pointed out, there isn't a magic solution here. If you have
methods in the subclass which need an action to have been done in the
subclass constructor before they can be called, then you can't call them
from the superclass constructor.

Possible solutions:

- remove the dependency on the action, eg with an if
(propertyChangeSupport != null) guard clause

- move the action to the superclass constructor: if you can make
propertyChangeSupport a field in AbstractModel, and have it set in the
AbstractModel constructor set it, you're fine - you could still have the
subclass supply the value:

abstract class AbstractModel {
protected final PropertyChangeSupport propertyChangeSupport;

public AbstractModel(PropertyChangeSupport propertyChangeSupport) {
this.propertyChangeSupport = propertyChangeSupport ;
}
}

public class Model extends AbstractModel {
public Model (Object source) {
super(new PropertyChangeSupport(source)) ;
}
}

- don't call the method - but then i don't know how you do the setting of
the value

- call the method from some other place; i got mildly flamed (and quite
fairly so!) for suggesting this recently, but:

abstract class AbstractModel {
public AbstractModel() {
// do nothing
}
protected void init() {
setSomeValue(new Integer(1));
}
public void setSomeValue(Integer value) {
firePropertyChange("someProperty", null, value);
}
protected void firePropertyChange(String propertyName, Object oldValue, Object newValue) {
// some code
}
}

public class Model extends AbstractModel {
private final PropertyChangeSupport propertyChangeSupport;

public Model (Object source) {
propertyChangeSupport = new PropertyChangeSupport(source);
init() ;
}
@Override
public void firePropertyChange(String propertyName, Object oldValue, Object newValue) {
propertyChangeSupport.firePropertyChange(propertyName, oldValue, newValue);
}
}

tom

--
Imagine a city where graffiti wasn't illegal, a city where everybody
could draw wherever they liked. Where every street was awash with a
million colours and little phrases. Where standing at a bus stop was never
boring. A city that felt like a living breathing thing which belonged to
everybody, not just the estate agents and barons of big business. Imagine
a city like that and stop leaning against the wall - it's wet. -- Banksy

 

[Mark Space]

abstract class AbstractModel {
public AbstractModel() {
indirectCall();
}

private void indirectCall() {
setSomeValue(new Integer(1));
}

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

As Lew said, the chained call from the constructor is the problem. Never
ever do this. Calling any "foriegn" method too (like "indirectCall()" )
is broken because you don't know what they will call. If 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. It's Item 17: Design and Document for
Inheritance or Else Prohibit It, in Effective Java 2nd edition.

A couple of classic solutions: use composition instead of inheritance.
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. Use 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. "indirectCall" is the problem, you can NEVER do
this and expect it to work well. It must go. What are you really
trying to do here?

 

[Royan]

As Lew said, the chained call from the constructor is the problem. Never
ever do this. šCalling any "foriegn" method too (like "indirectCall()" )
is broken because you don't know what they will call. šIf 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. šIt's Item 17: Design and Document for
Inheritance or Else Prohibit It, in Effective Java 2nd edition.

A couple of classic solutions: šuse composition instead of inheritance.
š 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. šUse 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. š"indirectCall" is the problem, you can NEVER do
this and expect it to work well. šIt must go. šWhat 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

 

[zerg]

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

Funny. I've actually exploited this, in subclass methods to detect if
they've been called from the superclass constructor. It's useful if
you're extending a standard class X that has an X(something)
constructor, a setSomething(something) method, and the subclass Y has to
do a lot of work whenever "something" is set. The Y(something)
constructor can call setSomething(something), which can do the work, but
there's a dilemma:

X MIGHT have implemented its constructor to call setSomething and if it
has, and Y's constructor calls setSomething, setSomething will be called
twice. Moreover, the first time, some subclass fields might still be
null that shouldn't be, for the reasons outlined in this thread.

On the other hand, X MIGHT NOT have implemented its constructor to call
setSomething, and then if Y's constructor also doesn't call
setSomething, Y doesn't get initialized.

Worse, Y's code might have to work with implementations of X that work
both ways, if X isn't your own class and different versions of the
codebase with X might implement X differently.

Y's setSomething COULD set up the subclass fields, instead of Y's
constructor, which would just call super(something). That fails if used
with a version of X whose constructor doesn't call setSomething.

Y's setSomething COULD set up the subclass fields, AND Y's constructor
call setSomething. That works, but with versions of X that call
setSomething from the X constructor the work done by Y's setSomething
gets done twice when a new Y is constructed.

Y's setSomething simply assuming the subclass fields are already set
will fail (generally with NPE) if used with a version of X that calls
setSomething from its constructor.

The way to ensure Y works however X's constructors are implemented, so
long as X adheres to its specification, is to include a subclass field
that should never have the default value (zero, null, false, etc.) in a
fully-constructed object (even if it means adding a boolean
isConstructed field). Y's setSomething tests this and if it's null,
zero, false, or whatever, it simply returns. Y's constructor calls
super(something), then sets the subclass fields, then calls setSomething.

The work gets done, but only gets done once, and no NPEs happen, however
X's constructor is implemented.

(If X's constructor may not work properly unless some setSomething
actions have taken place, you may want to call super in setSomething,
either before testing the subclass field, or if it's null. If the
superclass setSomething does things with superclass fields your subclass
still needs doing, it should call super at some point regardless. If it
detects it was called from the X constructor, it just calls super and
then returns.)

Calling an overridable method from the constructor may be a bad idea,
but when you have to work with (and even subclass) pre-existing classes
that have done so, this interesting behavior of Java can be useful for
actually helping to ensure correct behavior without duplication of work
even when the superclass implementation may change.

Unfortunately, the idiom outlined above perpetuates the constructor
calling an overridable method, if you keep the method overridable in
your subclass Y. In theory, if the idiom is repeated, a repeatedly
extended class might have the method get called and just return many
times before eventually getting called for the last time and actually
doing its job.

Therefore, you might want to have your constructor not call
setSomething, but rather a private method that does the actual work,
while setSomething checks for being constructed, calls super if
necessary, and calls the same private method if the superclass is not
still being constructed. Then your own class Y does not inherit the
problem the superclass X has.

You end up with:


private Z yField;

public Y (T something) {
super(something);
yField = new Z(foobar);
mySetSomething(something);
}

public void setSomething (T something) {
super(something);
if (yField == null) return; // superclass consr called us
mySetSomething(something);
}

private void mySetSomething (T something) {
// do actual work
... yField ... yadda yadda yadda ...
}


This seems the recommended idiom to use if Y's setSomething is
overridable. (If it, or Y itself, is final, there's no reason for the
extra private method.)

It strikes me that a class's specification should include when and under
what conditions overridable methods are called internally by a class,
and that, as with anything else in the specification, once the code is
out there it should not be changed if possible, and any such changes
documented clearly.

But we live in the real world, and sometimes we need to practice
"defensive programming".

(comp.lang.java.gui dropped from newsgroups line as this post is only
about general Java programming.)

 

[Mark Space]

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:

Your original test case was excellent, no worries. I think however now
that we know you base class is JDialog... I'm not sure there's much to
be done. Apparently, JDialog is poorly designed for the type of
inheritance you want to do. So, you can't do that.

I'm not sure what's up with the property change listener. I've never
used one (directly) and I don't know why it's firing in the constructor.
There might be a better way of using the property change listener.

Some things you might consider:

1. Wrap the JDialog instead of extending it.

class SampleDialog {
private JDialog jd;
private PropertyChangeSupport pcs;

//... etc.
}

2. Extend java.awt.dialog instead of JDialog.

3. Look into other ways to use property change listeners.

#3 seems like the first thing you should do. While JDialog might not be
the best class to extend, I don't see why you can't extend
DefaultTableModel (although it may not be designed for inheritance
either. AbstractTableModel seems to be the class to extend.) So you
might be doing something iffy with the property change listeners here.
I'm not really sure because I don't have any experience with them.