Single Responsibility Principle promotes greater reuse

Speaking to the design around a piece of code he had written, an aspiring programmer asked me “but how do I make it more generic?”

He had the right desire to reuse code, but the approach to keep adding parameters to his methods was having the opposite effect of promoting their reuse.

At times like this, it’s helpful to focus on the Single Responsibility Principle — the ‘S’ in SOLID object oriented design.

Case in question

Imagine a class that takes a List of text and replaces all occurrences of a given word with another.  Something like:

class TextReplacer {
    private String replace;
    private String with;

    public TextReplacer(String replace, String with) {
        this.replace = replace;
        this.with = with;
    }

    public List<String> replace(List<String> text) {
        List<String> results = new ArrayList<>(text.size());
        for (String textToCheck : text) {
            String result = textToCheck.equals(replace) ? with : textToCheck;
            results.add(result);
        }
        return results;
    }
}

The use of this class is pretty straightforward:

TextReplacer replacer = new TextReplacer("foo", "fu");
    List<String> results = replacer.replace(Arrays.asList("foo", "bar", "foo"));
    assertEquals(Arrays.asList("fu", "bar", "fu"), results);

Getting more complicated

The discussion at work focused on an added requirement — we wanted a UUID added to the end of each line.  (The real situation was more complex, it’s easier to understand with a simpler example).

In this case the programmer wanted to update the existing class with the code.  This made sense in his mind.  This class was where the text was already being transformed.  Go to this spot, add the work needed, and you’re done.

This is what how the resultant class looked:

class TextReplacer {
    private String replace;
    private String with;
    private boolean addUuid;

    public TextReplacer(String replace, String with, boolean addUuid) {
        this.replace = replace;
        this.with = with;
        this.addUuid = addUuid;
    }

    public List<String> replace(List<String> text) {
        List<String> results = new ArrayList<>(text.size());
        for (String textToCheck : text) {
            String result = textToCheck.equals(replace) ? with : textToCheck;
            results.add(result);
        }
        if (addUuid) {
            results.add(UUID.randomUUID().toString());
        }
        return results;
    }
}

A small change, yet the complexity has just been magnified. Additionally there’s a fundamental change being made to the class.

It started off as TextReplacer. Now, it would be more accurately labeled TextReplacerAndUuidAppender.   Any time there’s an ‘and’ in a class’s name, it’s usually a code smell.  Rather than focused on one task, the class is now trying to do two things, and is a good sign it should be broken into two separate classes.

Reasons against updating this class in place

It hurts reuse in a couple of ways.

For starters, it discourages a later developer from using the class if they have no need for appending a UUID at the end of their text… which is the one true requirement this class was originally created to solve.

Even if the class was updated with an overridden constructor and only appends the UUID where a flag is true — there’s the issue of its single responsibility being violated.   There’s no reason for a class whose only concern is replacing text to have any concerns outside of replacing text.

Even if the add-on functionality were made optional, updating this class would introduce the risk of introducing a NullPointerException in existing code that has already been working.

Perhaps the biggest impact, although hard to accurately measure, is how much reuse would be limited by the added confusion.

The best APIs are intuitive and easy to use.  Any later developer to use this code would have to take a step back and wonder why he’s being asked about UUIDs if all he wants is to replace some text.

Even if the developer reads the documentation or source for the class and understands the ins and outs, it’s time he shouldn’t have had to invest.  And in a more complicated scenario, unless the developer is confident he knows the ins and outs, he may rightly choose not to use the class at all due to the risk of unknown consequences.

Lastly, keeping classes simple and focused aids unit testing.  Every additional conditional that gets added doubles the number of edge cases that should rightly be checked.

Best bet

For our real-life scenario, the best option was to leave the existent class alone and make a new one to handle the new requirement, then pass the data through the two classes in sequence.  Like so:

/** Use original TextReplacer and add this class */
class UuidAdder {
    public List<String> add(List<String> text) {
        List<String> results = new ArrayList<>(text.size() + 1);
        results.addAll(text);
        results.add(UUID.randomUUID().toString());
        return results;
    }
}
/** Use in control flow changed like so */
    TextReplacer replacer = new TextReplacer("foo", "fu");
    UuidAdder adder = new UuidAdder();

    List<String> results = replacer.replace(Arrays.asList("foo", "bar", "foo"));
    results = adder.add(results);

    assertEquals(4, results.size());
    assertEquals(Arrays.asList("fu", "bar", "fu"), results.subList(0, 3));
    UUID.fromString(results.get(3));

This is a better design b/c the ‘decision making’ of what should be done is occurring at the level of the program’s control flow.

Moreover, the system has now gained another, highly-reusable class that can be applied anywhere a UUID is needed, to go along with the easily-tested text-replacing class we already had, and without risk of introducing bugs in any existing code, where we only need to test the simple class we’ve added and the small change we made to the control flow logic.