Howto design for extension

Question

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?

Naif

You shouldn't throw an IllegalArgumentException in a method that takes no arguments... – Gentlewoman

@Gentlewoman Let's pretend it's IllegalStateException for the sake of the "argument" :) – Prepense

Answer 1

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.

Intransitive

I see the point, but I'm still missing a way to implement the validate() method "good" and "practical" at the same time. If Tree implements a final validateEx() and calls validate(), a class extending Tree (lets say Forest) will not be able to override validateEx().Solution?Implement validateExEx()? – Naif

Use a better name, then. Maybe ValidateTreeEx(). – Intransitive

Sure - they can't replace your code.. but how can validate() ever get called ? – Gentlewoman

Maybe - or even final public. But if you'll notice, validate is called by the grow() function. It's part of an internal operation of the class. – Intransitive

@markt, likely something else would be instantiating a plant and growing it. That other thing, whatever it is, is part of the contract that must be followed when implementing a Plant. Side note: static analysis tools like Checkstyle and PMD are most helpful when implemented thoughtfully and intentionally. – Nothingness

I have used the proposed solution, but now I have bigger problem: empty non-abstract method (PMD rule violation), in abstract class. Is there any way to solve it? – Goggin

I'm still confused as to how the implementing classes are aware that they MUST call the super validate() function as oppose to its own implementing validateEx() function. Is this just up to the developer to be aware? – Schulte

@Schulte implemented classes don't call validate(). That function would be called by existing code in the application, whenever the object needs to be validated. This way, an implementing class can't replace or hide existing validation logic for the code in my answer: only extend it. – Intransitive

But what shall I do if I want to make inheritance deeper. For example: A extends Plant, B extends A and so on? – Beilul

Answer 2

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)

Hamm

Thanks Horst, although your answer discusses the design of the sample code and not the original question: how to design for extension. It's really just sample code, not perfect probably. – Naif