2

Is it a good practice to provide an abstract method to help the developer to initialize a complex object in the superclass constructor?

Consider the following classes:

// Parent class
abstract public class A() {
    protected Person[] people;

    public A(Person[] people) {
        this.people = people;
    }

    abstract protected Person[] initPeople();
}

// Child class
public class B extends A {
    public B() {
        super(initPeople());
    }

    @Override
    protected Person[] initPeople() {
        return new Person[] {
            new Person("James"),
            new Person("Mary"),
            new Person("Elizabeth")
        };
    }
}

If there was no initPeople method, the developer of the child class would probably add it himself because creating the people object take several lines.

Is there a better way to do it? Thank you!

Chetan Kinger
  • 15,069
  • 6
  • 45
  • 82
Eneelk
  • 97
  • 1
  • 7
  • Yes: just remove that abstract method, and let the subclass decide how it calls the superclass constructor. – JB Nizet Oct 20 '17 at 06:15
  • If you add abstract in the superclass, the subclass must override it. By the way, you must return `Person[]` for the `initPeople` method – Xin Meng Oct 20 '17 at 06:24
  • Firstly, your code does not work, `initPeople()` method is void but you are returning the void. @Override protected Person[] initPeople() { return new Person[] { new Person("James"), new Person("Mary"), new Person("Elizabeth") } – Oguz Ozcan Oct 20 '17 at 06:26
  • Thank you, I edit the code. – Eneelk Oct 20 '17 at 06:27
  • @Raedwald I don't think this question is a duplicate of the one marked. There is a subtle difference between the two. – Chetan Kinger Oct 20 '17 at 07:11
  • @CKing if they are different, the text of the question should clearly indicate how they are different, with a link to the other question. Edit the question, and let the voters on the Reopen queue decide. – Raedwald Oct 20 '17 at 07:20
  • And another: https://stackoverflow.com/questions/3404301/whats-wrong-with-overridable-method-calls-in-constructors – Raedwald Oct 20 '17 at 07:24
  • 1
    @Raedwald Unless I missed something, the OP clearly asks *"Is there a better way to do it*" indicating that the question is not about calling `abstract` methods from a constructor but asking for a way to force subclasses to provide an initialization method? The calling of `abstract` method in the constructor is one of the solutions that the OP could think of but that is not the real question here is it? – Chetan Kinger Oct 20 '17 at 07:49
  • Initialization methods are a bad udea anyway. See https://stackoverflow.com/a/3786967/545127 – Raedwald Oct 20 '17 at 08:38
  • 2
    @Raedwald What has that got to do with marking this as a duplicate? Something being a bad idea (I don't agree that it's a bad idea but that is off topic) doesn't make it a candidate for a duplicate I am sure? – Chetan Kinger Oct 20 '17 at 09:01

1 Answers1

2

The primary problem with this approach is that even though the subclass is required to implement the initPeople method, there is nothing forcing the subclass to call it in the constructor.

A better approach would be to use the Template design pattern :

public abstract class A 
{
    protected Person[] people;

    public final void execute() 
    {
         initPeople();
         performOperations();
    } 
    protected abstract void initPeople();
    protected abstract void performOperations();
}
Chetan Kinger
  • 15,069
  • 6
  • 45
  • 82
  • Thank you for the idea. I have a question about it: is it safe to call `execute()` in the constructor of `A`? – Eneelk Oct 20 '17 at 06:35
  • That is not recommended. The purpose of a constructor is to initialize an object and not call methods that perform operations on the data. The idea here is that after instantiating a subclass of `A`, the client will call the `execute` method. The execute method will initialize the person array first through `initPeople` and then call `performOperations`. The way the code is structured, subclasses will always have to provide a way to initialize person and then a way to perform some operations on it. – Chetan Kinger Oct 20 '17 at 06:37
  • why go for a `RuntimeException` instead of an `UnsupportedOperationException` ? – FrankK Oct 20 '17 at 06:47
  • @FrankK An `UnsupportedOperationException` by definition indicates an optional operation that is not supported; however, in this particular case, `initPeople` is central to the correctness of the `execute` method. If the `people` array is not intialized, none of the subclasses can do their job correctly. – Chetan Kinger Oct 20 '17 at 06:58
  • in that case, why not just make it abstract? – FrankK Oct 20 '17 at 07:00
  • @FrankK I here you. This answer was typed form my mobile phone. Point taken. – Chetan Kinger Oct 20 '17 at 07:02