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

You are building an in-memory file system. Before it touches a path, it has to reject one that is not absolute. Here is the first version. The check sits right 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. Leave it alone. The rule about what a path looks like lives next to the only place that currently cares.

2. The new requirement, and the drift it allows

Now a second operation arrives. write has to reject the same malformed paths that mkdir rejects. You copy the check. Everybody does.

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);
    }
}

Two copies. Keeping them in sync by hand costs almost nothing, and the code still runs correctly. So far so good.

Then a security review lands a new rule. No path may contain ... Without that rule a caller can write to /docs/../etc/passwd and escape the tree you meant to confine them to.

Someone picks up the ticket. They add the check to mkdir. They test it. It passes. They move on.

Nobody notices that write is carrying 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);
    }
}

Run both operations against the same hostile path and watch what you get.

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

Look at those two lines carefully.

mkdir refuses the traversal. write accepts the identical path and reports success. Both methods compile. Both methods pass whatever test was written for them on their own.

Here is the part worth slowing down for. These two methods were never enforcing one rule. Each was enforcing its own private copy. The copies agreed right up until the moment somebody edited one of them.

Now scale it. Six methods, six copies. That is not six times the typing. It is six independent chances for this drift, and the compiler will never mention any of them.

3. The move, and the test that names it

Ask yourself why each piece changes.

Path syntax changes when the rules for a legal path change. Directory methods change when the method list changes. Those are two different clocks. Whether a path is well-formed has nothing to do with whether mkdir or write is the one asking.

So give the check its own class. Every method asks it. Nobody carries a 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 refuse it now. There is one rule, and both methods ask it.

The next requirement about path shape is a single edit inside PathSyntax. Every caller picks it up for free, because every caller was already going through it.

That is the idea behind the formal wording you will hear in an interview: a class should have one reason to change.

On its own that sentence is too soft to act on. Here is the version you can actually check. Fill in the blank:

<Class> changes when \_\_\_\_.

Try it on both classes.

PathSyntax changes when the definition of a legal path changes. Nothing else.

MiniFileSystem changes when its methods run in a new order, or when a new method joins the list.

Two different blanks. Two different classes. Each one has exactly one. That is what you are looking for.

4. The version of this that is wrong

Now watch the same principle go too far.

Push "one reason to change" hard enough and it stops meaning one rule and starts meaning one method. So somebody splits PathSyntax in half. Smaller classes must be more disciplined, surely.

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 the two new classes. Do it before you read the next paragraph.

PathValidator changes when the definition of a legal path changes.

ReservedCharacterCheck changes when the definition of a legal path changes. A rule about which characters a path may contain is a rule about what makes a path legal.

One blank. Two classes. They were already one class, and the split bought you a second file to open every time either rule moves.

This costs you marks. 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 compares two resolved locations in the tree. It says nothing about whether either path is spelled correctly. 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. 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

You have probably met the phrase "a class should do one thing". Be careful with it. Describe any class vaguely enough and it does one thing. Describe it precisely enough and it does five. The sentence cannot be applied, so it gets recited instead.

The fill-in-the-blank test is the version that works. And it has a sharper form worth carrying into an interview. Stop asking what the class does. Ask who would come and 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.

Two unrelated people, arriving with two unrelated requests, both walking up to the same class? Then that class is holding two jobs. This is sometimes called the actor test. It survives follow-up questions because "who asks for this" has a concrete answer, and "what does this do" usually does not.

Now the honest cost, so you do not find it the hard way.

Read "one reason to change" completely literally and it never stops. A method changes for its own reason, so a literal reading ends at one class per method. Section 4 built exactly that. The fill-in-the-blank test is what catches it before you ship.

Use the principle as a question about a boundary you are already drawing. It does not license you to keep splitting until nothing is left.

Java 21 hands you a sharper tool for one common case. In corpus/file-system, Node is declared sealed interface Node permits FileNode, DirectoryNode. Exactly one place in the codebase asks what kind of node it has: a switch expression, which javac checks for completeness.

Add a third kind of node later and you get a compile error at that one switch. You do not get a silently wrong answer from whichever method forgot to handle it.

sealed will not replace the fill-in-the-blank test. It gives you one shape of the same idea — knowing every kind of thing that exists — in a form the compiler can check for you.

Why this matters in an interview

Interviewers ask for SRP by name more than anything else in this part.

Most candidates answer with "a class should do one thing". You now know what happens to that answer on the first follow-up question.

Answer with the fill-in-the-blank test instead, and be ready to apply it live to whichever class the interviewer points at. Reciting a rule and using one look very different from the other side of the table.

The second half of this chapter counts just as much under a clock. Watch out for this trap: you take a review comment, and you answer it by extracting another single-method class. Then another. An interviewer watching that scores it as over-engineering, exactly the way STANDARD v1.0 scores it here. You would be committing the mistake this chapter named while believing you were avoiding it.

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 →