Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Howto design for extension

Tags:

There is a Checkstyle rule DesignForExtension. It says: if you have a public/protected method which is not abstract nor final nor empty it is not "designed for extension". Read the description for this rule on the Checkstyle page for the rationale.

Imagine this case. I have an abstract class which defines some fields and a validate method for those fields:

public abstract class Plant {
    private String roots;
    private String trunk;

    // setters go here

    protected void validate() {
        if (roots == null) throw new IllegalArgumentException("No roots!");
        if (trunk == null) throw new IllegalArgumentException("No trunk!");
    }

    public abstract void grow();
}

I have also a subclass of Plant:

public class Tree extends Plant {
    private List<String> leaves;

    // setters go here

    @Overrides
    protected void validate() {
        super.validate();
        if (leaves == null) throw new IllegalArgumentException("No leaves!");
    }

    public void grow() {
        validate();
        // grow process
    }
}

Following the Checkstyle rule the Plant.validate() method is not designed for extension. But how do I design for extension in this case?

like image 386
Eduard Wirch Avatar asked Mar 19 '09 15:03

Eduard Wirch


2 Answers

The rule is complaining because it is possible for a deriving (extending) class to completely replace the functionality you provided without telling you about it. It's a strong indication that you haven't fully considered how the type might be extended. What it wants you to do instead is something like this:

public abstract class Plant {
    private String roots;
    private String trunk;

    // setters go here

    private void validate() {
        if (roots == null) throw new IllegalArgumentException("No roots!");
        if (trunk == null) throw new IllegalArgumentException("No trunk!");
        validateEx();
    }

    protected void validateEx() { }

    public abstract void grow();
}

Note that now someone can still supply their own validation code, but they can't replace your pre-written code. Depending on how you meant to use the validate method you could also make it public final instead.

like image 157
Joel Coehoorn Avatar answered Sep 20 '22 13:09

Joel Coehoorn


Although the answer by Joel Coehoorn explains how to overcome the concrete problem posted by the OP, I’d like to suggest an approach which takes a broader view on ‘how to design for extension?’ As the OP points out in one of his comments, the given solution does not scale well with a growing (class) inheritance depth. Also, anticipating in the base class the need to validate possible child classes (validateTreeEx()) is problematic for obvious reasons.

Proposal: Check a plants properties at construction time and remove validate() altogether (along with possible setters; see also http://www.javaworld.com/article/2073723/core-java/why-getter-and-setter-methods-are-evil.html). The original code suggests that validate() is an invariant, which has to be true before each grow() operation. I doubt that this design is intentional. If there is no operation, which can ‘break’ a plant after construction, there is no need to re-check the validity over and over again.

Going even further, I’d question the soundness of the initial inheritance design. Without additional (possibly polymorphic) operations, Tree just reuses some properties of Plant. I hold the strong opinion, that class inheritance should not be used for code reuse. Josh Bloch has this to say (from Effective Java, 2nd Edition, chapter 4):

If you use inheritance where composition is appropriate, you needlessly expose implementation details. The resulting API ties you to the original implementation, forever limiting the performance of your class. More seriously, by exposing the internals you let the client access them directly.

Also check out 'Item 17: Design and document for inheritance or else prohibit it' (also chapter 4 for the same book)

like image 39
Horst Dehmer Avatar answered Sep 22 '22 13:09

Horst Dehmer