0

I have the following situation:

        public abstract class A {
            private Object superMember;
            public A() {
                superMember = initializeSuperMember();
                // some additional checks and stuff based on the initialization of superMember (***)
            }

            protected abstract Object initializeSuperMember();
        }

        class B extends A {
            private Object subMember;
            public B(Object subMember) {
                super();
                subMember = subMember;
            }

            protected Object initializeSuperMember() {
                // doesn't matter what method is called on subMember, just that there is an access on it
                return subMember.get(); // => NPE
            }
        }

The problem is that I get a NPE on a new object B creation.
I know I can avoid this by calling an initializeSuperMember() after I assign the subMember content in the subclass constructor but it would mean I have to do this for each of the subclasses(marked * in the code).
And since I have to call super() as the first thing in the subclass constructor I can't initialize subMember before the call to super().

Anyone care to tell me if there's a better way to do this or if I am trying to do something alltogether wrong?

Deelazee
  • 493
  • 2
  • 9
  • 18

4 Answers4

2

Two problems:

First, you should never call an overrideable member function from a constructor, for just the reason you discovered. See this thread for a nice discussion of the issue, including alternative approaches.

Second, in the constructor for B, you need:

this.subMember = subMember;

The constructor parameter name masks the field name, so you need this. to refer to the field.

Community
  • 1
  • 1
Ted Hopp
  • 232,168
  • 48
  • 399
  • 521
  • So then the only solution is to have some duplicate code in all subclasses? – Deelazee Feb 13 '12 at 17:38
  • @Deelazee - No, that's no the only solution. You can use the [Builder pattern](http://en.wikipedia.org/wiki/Builder_pattern), as discussed in the thread I referenced. You can also use a [Factory method pattern](http://en.wikipedia.org/wiki/Factory_pattern), which is closely related. – Ted Hopp Feb 13 '12 at 17:49
  • Or, third and probably simplest option, pass arguments to the superclass constructor. – gustafc Feb 13 '12 at 18:25
  • 1
    @gustafc - That's not a good solution. OP specifically wanted to keep the subclass data out of the superclass. So what's the superclass going to do with the arguments? Pass them back to the subclass. This requires the superclass to know more about the subclass than it should (which is nothing). It also doesn't generalize: should we further pollute the superclass when another subclass requires different objects for successful initialization? We would then need to overload `initializeSuperMember()` to take different arguments...a mess. – Ted Hopp Feb 13 '12 at 19:09
  • I can't help but feel that the OP should tell us what they're _actually_ trying to do here, and then we should address that specific use case. – Louis Wasserman Feb 13 '12 at 19:19
  • @Ted Hopp - I don't follow you; why should there be any need to pass more arguments to the superclass? The OP has one field in the superclass which needs to be initialized with data supplied by the subclass. `A` gets a single-argument ctor - called by `B´ with `super(submember.get())` - and cna verify that `superMember` is OK. – gustafc Feb 14 '12 at 20:36
  • @gustafc - In OP's example, the subclass cannot pass an argument to the superclass constructor because the subclass member field (`subMember`) must be initialized in order to compute the data needed in the superclass. However, the superclass constructor must run before any subclass fields are properly initialized. The only way around this is to pass the data needed to initialize the subclass field to the superclass so that the superclass can, in turn, pass it back to the subclass in order to compute the appropriate value. – Ted Hopp Feb 15 '12 at 17:14
0

Follow the chain of invocation:

  1. You invoke the B() constructor.
  2. It invokes the A() constructor.
  3. The A() constructor invokes the overridden abstract methot
  4. The method B#initializeSuperMember() references subMember, which has not yet been initialized. NPE.

It is never valid to do what you have done.

Also, it is not clear what you are trying to accomplish. You should ask a separate question explaining what your goal is.

Jim Garrison
  • 85,615
  • 20
  • 155
  • 190
0

The problem is when you call super(), the subMember is not initialized yet. You need to pass subMemeber as a parameter.

public abstract class A {
    public A (Object subMember) {
        // initialize here
    }
}


class B extends A {
    public B (Object subMember) {
        super(subMember);
        // do your other things
    }
}

Since you don't want to have subMember in the abstract class, another approach is to override the getter.

public abstract class A {
    public abstract Object getSuperMember();
    protected void checkSuperMember() {
        // check if the supberMember is fine
    }
}

public class B extends A {
    private Object subMember;
    public B(Object subMember) {
        super();
        this.subMember = subMember;
        checkSuperMemeber();
    }

    @Override
    public Object getSuperMember() {
        return subMember.get();
    }
}

I hope this can remove your duplicate code as well.

Bob Wang
  • 606
  • 4
  • 8
0

Hum, this code does not look good and in all likelyhood this is a sign of a bad situation. But there are some tricks that can help you do what you want, using a factory method like this:

public static abstract class A {
    public abstract Object createObject();
}

public static abstract class B extends A {
    private Object member;

    public B(Object member) {
        super();
        this.member = member;
    }

}

public static B createB(final Object member) {
    return new B(member) {

        @Override
        public Object createObject() {
            return member.getClass();
        }
    };
}
Guillaume Polet
  • 47,259
  • 4
  • 83
  • 117