Re: Avoiding NPEs caused by indirect call

Royan wrote:


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.)