Common OOP Code Smells

The most common smells in Java code are a switch or if chain on the runtime type of an object, a method that does several unrelated things, an anemic class that holds data while the rules live elsewhere, and a subclass that overrides a parent method only to throw an exception.

public class Main {
    public static void main(String[] args) {
        Renderer r = new HtmlRenderer();
        System.out.println(r.render("hi"));
        Renderer m = new MarkdownRenderer();
        System.out.println(m.render("hi"));
    }
}

interface Renderer {
    String render(String text);
}

class HtmlRenderer implements Renderer {
    public String render(String text) { return "<p>" + text + "</p>"; }
}

class MarkdownRenderer implements Renderer {
    public String render(String text) { return "**" + text + "**"; }
}

The type switch is the one worth showing first, because the code above is its replacement. The old version asked what kind of renderer it had and branched; the new version asks the renderer to render itself. The caller no longer has to know the set of implementations, so adding a format means adding a class and nothing else. The output is the same either way, which is what makes the refactoring safe.

The other three have standard fixes. A long method with unrelated sections is split by responsibility, usually by extracting the sections into named methods or collaborators. An anemic class gains behaviour, and the service that was operating on it loses the rules that belong to the data. A refused bequest, where a subclass throws from an inherited method, means the parent promised too much, so the contract is split into a narrower interface that only the relevant subclasses implement.

None of these is a rule the compiler enforces, which is why they survive review. The way to catch them is to ask two questions of every method: what would I name this, and does the name describe one thing? A method that needs a conjunction in its name is the usual starting point for a refactor.

A refused bequest is when a subclass inherits a method from its parent and overrides it only to throw an exception or return nothing useful. The subclass is saying it cannot honour the parent's contract, which is the direct violation of the substitutability promise that inheritance is supposed to keep.

It usually appears when a parent class was designed as a catch-all. A manager class that inherits from a generic worker and cannot report, or a repository base that assumes a table it does not have, both produce subclasses full of unsupported operation exceptions. The exceptions are a design note that says this relationship should not exist.

The clean fix is to split the contract. Instead of one parent with everything, define narrow interfaces for the capabilities that vary, such as a reporting capability and a supervising capability, and have the parent implement only what genuinely applies to all of them. The subclasses then implement the extra interface when they support it, and the exception methods disappear because nobody inherits them.

The incremental version of that refactoring is gentle. Leave the parent class alone for now, introduce the narrow interface alongside it, move the subclasses over one at a time, and only then retire the parts of the parent that nothing uses. The tests keep the behaviour honest throughout, and no caller has to change at once.

The judgement to explain in an interview is that a subclass which cannot support its parent's behaviour is evidence about the hierarchy, not about the subclass. The parent was modelled as the union of everything its children can do, and the fix is to model the parent as the intersection instead.