Calling an overridable method in constructor is bad. Are there exceptions?

176 Views Asked by At

I was wondering if there are cases where calling public or, in this case and particularly, protected methods in the constructor of an abstract class would be ok, or at least forgivable, given an adequate documentation of the intentions of the method.

My actual question concerns an abstract class IdentifiedConnection that looks like this:

public abstract class IdentifiedConnection extends Connection {

    private UUID uuid;

    public IdentifiedConnection(String serverAddress, int port) throws IOException {
        super(serverAddress, port);
        this.uuid = this.aquireIdentifier();
    }

    public UUID getIdentifier() {
        return this.uuid;
    }

    protected abstract UUID aquireIdentifier() throws IOException;

}

My intention is to get a UUID identifier from the server. If the server responds with a valid UUID, we set that UUID in the field of this class (named uuid). If the server responds with a non-UUID message, we assume that the server is unreachable for some reason and throw an IOException.

Generally I understand that calling an overridable method from the constructor is bad, since it leaves the class prone to various more or less hard-to-detect bugs. However, in this case I feel like doing this isn't actually too bad (as long as I javadoc the hell out of it).

What's your thoughts? Is this still a really bad idea? What alternative way of doing this do you suggest if you think this is bad?


I have left the code for the Connection class out of this question as I think it's irrelevant.

2

There are 2 best solutions below

3
On BEST ANSWER

I would recommend you to use a factory method in this case.

Write a static method called something like

public static IdentifiedConnection openConnection(String serverAddress, int port)
        throws IOException {
    ...
}

This allows for better control of the creation and avoids the potential problems with leaking references to uninitialized objects.


Another approach would be to take a Supplier<UUID> as argument to the constructor, as follows:

abstract class IdentifiedConnection extends Connection {

    private UUID uuid;

    public IdentifiedConnection(String serverAddress,
                                int port,
                                Supplier<UUID> uuidSupplier) throws IOException {
        super(serverAddress, port);
        this.uuid = uuidSupplier.get();
    }
}

and in a subclass use it as

class SomeConnection extends IdentifiedConnection {

    public SomeConnection(String serverAddress, int port) throws IOException {
        super(serverAddress, port, SomeConnection::createUUID);
    }

    public static UUID createUUID() {
        return ...;
    }

}
0
On

Having a base-class constructor invoke virtual or abstract methods is a perfectly fine and reasonable thing to do if the documentation for such methods expressly states that they will be called from the base-class constructor. Such calls can be very useful in cases where e.g. a subclass needs to affect the value to be stored in a final base-class field, or where establishment of the base-class invariants would entail exposing the newly-constructed object to outside code. Unfortunately, there is no nice way for a derived-class method which runs before the derived-class constructor to get access to any of the derived class constructor parameters. It would be possible for each layer of a class to define a constructor-parameter class, each of which derives from the next lower layer's constructor-parameter type, and then have the base class pass that object through to a virtual method which is called in the base constructor, but that's a bit hokey. Still, having a virtual init(ConstructorParams) method may make it possible for derived classes to initialize themselves before the base class constructor returns--something which is otherwise very difficult.