2.4 Single responsibility, and how to test for it

Objects that hold their shape · chapter 14 of 33

Single responsibility, and how to test for it

Chapter 2.4 · Part 2, Objects that hold their shape · about 30 minutes

What you need before this chapter: chapters 2.1 and 2.2, encapsulation and immutability. Also chapter 2.3, composition over inheritance.

When you finish this chapter you will be able to:

Watch a duplicated rule cause two operations to silently disagree, in real compiled output
Apply a one-sentence test that tells you whether a class is carrying more than one responsibility
Recognise a split that produces a class per method, and say why that split is worse than doing nothing
Use the same test to catch both failures: a class doing too much, and a class split too far

1. Working code, for one operation

An in-memory file system needs to reject a path that is not absolute before it does anything else with it. The first version puts the check exactly where the operation needs it.

public class MiniFileSystem {

    public void mkdir(String path) {
        if (path == null || !path.startsWith("/")) {
            throw new IllegalArgumentException("path must be absolute (start with '/'): " + path);
        }
        System.out.println("created directory " + path);
    }
}

For one operation, this is fine. The rule about what a path looks like lives next to the one place that currently needs it.

2. The new requirement, and the drift it allows

A second operation, write, needs to reject the same malformed paths mkdir does. The obvious move is to copy the check.

public class MiniFileSystem {

    public void mkdir(String path) {
        if (path == null || !path.startsWith("/")) {
            throw new IllegalArgumentException("path must be absolute (start with '/'): " + path);
        }
        System.out.println("created directory " + path);
    }

    public void write(String path, String content) {
        if (path == null || !path.startsWith("/")) {
            throw new IllegalArgumentException("path must be absolute (start with '/'): " + path);
        }
        System.out.println("wrote " + content.length() + " bytes to " + path);
    }
}

At two copies, keeping them in sync by hand costs little, and this still runs correctly. Then a security review lands a new rule: no path may contain .., because a caller could otherwise write /docs/../etc/passwd and reach outside the tree it was supposed to be confined to. Someone adds the check to mkdir, tests it, and moves on to the next ticket without noticing that write has its own, separate copy of the path rule.

public class MiniFileSystem {

    public void mkdir(String path) {
        if (path == null || !path.startsWith("/")) {
            throw new IllegalArgumentException("path must be absolute (start with '/'): " + path);
        }
        if (path.contains("..")) {
            throw new IllegalArgumentException("path must not contain '..': " + path);
        }
        System.out.println("created directory " + path);
    }

    // write() was not touched when the ".." rule was added to mkdir() above.
    public void write(String path, String content) {
        if (path == null || !path.startsWith("/")) {
            throw new IllegalArgumentException("path must be absolute (start with '/'): " + path);
        }
        System.out.println("wrote " + content.length() + " bytes to " + path);
    }
}

MiniFileSystem fs = new MiniFileSystem();

try {
    fs.mkdir("/docs/../etc");
} catch (IllegalArgumentException e) {
    System.out.println("mkdir rejected: " + e.getMessage());
}

fs.write("/docs/../etc/passwd", "root:x:0:0::/root:/bin/sh");

mkdir rejected: path must not contain '..': /docs/../etc
wrote 25 bytes to /docs/../etc/passwd

mkdir correctly refuses the traversal. write accepts the identical path and reports success. Both methods compile. Both methods pass whatever test was written for them in isolation. The two methods were never actually enforcing one rule. They were each enforcing their own copy of it, and the two copies stopped agreeing the moment only one of them got edited. Six methods carrying six copies of the same check is not six times the work of writing the check. It is six independent chances for this exact drift, and nothing in the language or the compiler notices when it happens.

3. The move, and the test that names it

Path syntax and directory methods change for two different reasons. Whether a path is well-formed does not depend on whether it is being handed to mkdir or to write. Give the check its own class, and have every method ask it instead of carrying its own copy.

final class PathSyntax {

    /** Throws if the path is not legal. Every caller asks here, and only here. */
    static void requireValid(String path) {
        if (path == null || !path.startsWith("/")) {
            throw new IllegalArgumentException("path must be absolute (start with '/'): " + path);
        }
        if (path.contains("..")) {
            throw new IllegalArgumentException("path must not contain '..': " + path);
        }
    }

    private PathSyntax() {}
}

public class MiniFileSystem {

    public void mkdir(String path) {
        PathSyntax.requireValid(path);
        System.out.println("created directory " + path);
    }

    public void write(String path, String content) {
        PathSyntax.requireValid(path);
        System.out.println("wrote " + content.length() + " bytes to " + path);
    }
}

mkdir rejected: path must not contain '..': /docs/../etc
write rejected: path must not contain '..': /docs/../etc/passwd

Both operations refuse the traversal now, for the same reason: there is only one rule, and both methods ask it. Any future requirement about what a path may look like is one edit, in PathSyntax, and every caller picks it up because every caller was already going through it.

This is the idea behind the formal statement of the principle: a class should have one reason to change. The test that makes this checkable, rather than a matter of taste, is a sentence you fill in: <Class> changes when ____. PathSyntax changes when the definition of a legal path changes, and for nothing else. MiniFileSystem changes when its methods run in a new order, or a new method joins the list. Two different blanks, two different classes, and each one has exactly one.

4. The version of this that is wrong

Taken further than the blank actually asks for, "one reason to change" gets applied per method instead of per rule. PathSyntax's two checks get split apart, on the theory that a smaller class must be more disciplined.

final class PathValidator {
    static void requireAbsolute(String path) {
        if (path == null || !path.startsWith("/")) {
            throw new IllegalArgumentException("path must be absolute (start with '/'): " + path);
        }
    }
    private PathValidator() {}
}

final class ReservedCharacterCheck {
    static void requireNoTraversal(String path) {
        if (path.contains("..")) {
            throw new IllegalArgumentException("path must not contain '..': " + path);
        }
    }
    private ReservedCharacterCheck() {}
}

both checks passed for /docs

Run the fill-in-the-blank test on both new classes. PathValidator changes when the definition of a legal path changes. ReservedCharacterCheck changes for the identical reason: a rule about which characters a path may contain is a rule about what makes a path legal. One blank, two classes. They were one class already, and the split bought nothing but a second file to open the next time either rule changes. A minimal set of classes scores as well as a correct split of a real one. A speculative split with no second reason to exist scores below the single class it replaced.

Your turn

corpus/file-system's real InMemoryFileSystem has eight operations, not two, and every one of them calls PathSyntax.segments(path) rather than checking anything about path shape itself. Given the fill-in-the-blank test, explain in a sentence why mv, which needs to check that a destination is not inside the source it is moving, does not belong inside PathSyntax.

The answer. mv's check depends on comparing two resolved locations in the tree, not on whether either path is well-formed text. PathSyntax changes when the rules for legal path spelling change. The self-move check changes when the rule about moving something into itself changes, and that is a fact about tree shape, not spelling. Two different blanks again, correctly kept as two different pieces of code: one inside PathSyntax, one inside the method that walks the tree.

Going deeper

"A class should do one thing" is not a test. Almost any class can be described as doing one thing at a vague enough level, and almost any class can be described as doing several things at a precise enough one. The fill-in-the-blank sentence from section 3 is the version that actually works. It has a sharper form worth knowing: instead of asking what a class does, ask who would ask for it to change.

PathSyntax changes when whoever owns path rules decides the rules are different. MiniFileSystem changes when whoever owns the method list wants a method to behave differently. If two unrelated people, for two unrelated reasons, would both come to the same class asking for a change, that class is carrying more than one job. This is sometimes called the actor test. It holds up better than "does one thing" does, because "who asks for this" has a concrete answer and "what does this do" usually does not.

Here is the honest cost of the principle, stated plainly rather than left for you to find the hard way. Taken completely literally, "one reason to change" never stops at a sensible class. A method changes for its own reason, so a fully literal reading produces a class per method. That is exactly what section 4 built, and exactly what the fill-in-the-blank test exists to catch before you ship it. The principle is a question to ask about a boundary you are already drawing. It is not a licence to keep splitting until nothing is left.

Java 21 gives this a second, sharper tool for one common case. corpus/file-system's Node is sealed interface Node permits FileNode, DirectoryNode. The one place in the whole codebase that asks "what kind of node is this" is a switch expression, and javac checks it for completeness. Adding a third kind of node later is a compile error at that one switch, not a silently wrong answer at whichever method forgot to handle it. sealed does not replace the fill-in-the-blank test. It gives one specific shape of the same idea, knowing every kind of thing that exists, a shape the compiler can check for you.

Why this matters in an interview

Interviewers ask for SRP by name more than any other principle in this part. Most candidates answer with "a class should do one thing," and that is exactly the version this chapter showed you does not survive a follow-up question. Answering instead with the fill-in-the-blank test, ready to apply it live to a class the interviewer names, separates reciting a rule from demonstrating you can use one. The second half of this chapter matters just as much under a clock. A candidate who answers every review comment by extracting another single-method class is committing the mistake this chapter named, not avoiding it. A watching interviewer scores that as over-engineering, the same way STANDARD v1.0 does here.

Next: chapter 2.5, Open for extension, closed for modification — where a rule that varies gets a seam of its own, before a promotion or a pricing change forces it.

← 2.3 Composition over inheritance · All chapters · 2.5 Open for extension, closed for modification →