Skip to content

Chapter 9: Better Together Or Better Apart?

Whether two pieces of functionality belong together or apart is decided by complexity, not size. Subdividing adds components, interfaces, management code, separation, and duplication, so split only when the pieces are truly independent, and join them when they share information, simplify an interface, or remove duplication.

  • The costs of subdivision: smaller pieces look simpler, but subdividing adds:
    1. More components to track and more interfaces, each adding complexity.
    2. Extra code to manage the components (one object becomes several).
    3. Separation: related code ends up in different classes or files. That is good if the pieces are independent and bad if they depend on each other, because developers flip between them or miss the dependency.
    4. Duplication of code that each piece now needs.
  • Signs that two pieces of code are related (bring them together):
    • They share information, such as the syntax of a document type.
    • They are used together, and only if this holds in both directions. A block cache almost always uses a hash table, but hash tables are used everywhere, so keep them separate.
    • They overlap conceptually under a simple higher-level category (substring search and case conversion are both string manipulation).
    • It is hard to understand one without looking at the other.
  • Four rules:
    1. Bring together if information is shared (the HTTP read and parse classes).
    2. Bring together if it will simplify the interface: intermediate interfaces disappear, and some functions can happen automatically (buffering by default in a combined FileInputStream).
    3. Bring together to eliminate duplication: factor repeated code into a method if the snippet is long and the method’s signature is simple, or restructure so the snippet runs in one place.
    4. Separate general-purpose and special-purpose code: a general mechanism should contain just that mechanism. Pull special-purpose code upward into higher layers.
  • Splitting methods (Figure 9.3):
    • (b) Factor out a subtask (the best split): the parent keeps the original interface and calls a child. It works only if the child can be read without knowing the parent, and the parent without knowing the child’s implementation. The child is usually somewhat general-purpose.
    • (c) Split into two methods visible to callers: only when the original interface was overly complex because it did unrelated things. Each new method needs a simpler interface, and most callers should need only one. The book marks this “rarely works.”
    • (d) Avoid: splitting into several shallow methods that callers must call in turn, passing state between them.
  • Join methods when it replaces two shallow methods with one deeper one, removes duplication, removes dependencies or intermediate data structures, isolates knowledge in one place, or simplifies the interface.
  • Red Flag: Repetition: the same or nearly the same code appears again and again, so you haven’t found the right abstractions.
  • Red Flag: Special-General Mixture: a general mechanism contains code for one particular use, which complicates it and leaks information between it and the use case.
  • Red Flag: Conjoined Methods: you can’t understand one method without understanding another. This also applies to any two physically separated pieces of code that only make sense together.
  • Fences: markers in an undo history list that group related actions into one user-level undo.
  • Mechanism vs. policy split for undo: general history mechanism, specific action classes, and grouping policy in the UI.
  • “Length by itself is rarely a good reason for splitting up a method.” Methods of hundreds of lines are fine if they have a simple signature and are easy to read, because they are deep.
  • “Each method should do one thing and do it completely.” If it does, and its interface is simple and deep, its length hardly matters.
  • Think of lower layers as general-purpose and upper layers as special-purpose. When one class mixes both for the same abstraction, split it into a general class and a special-purpose class layered on top.
  • The general/special rule is per mechanism. Special-purpose undo code for text insertion belongs in the text class (with the other text code), not in the general History class.
  • Use goto to a shared cleanup block at the end of a method when the language allows it. It is acceptable for escaping nested code.
  • “Split up any method longer than 20 lines!”: rigid length rules make shallow, conjoined methods.
  • A separate logging class with one shallow method per call site (see worked example).
  • Combining loosely related things into one object: one team stored the selection and cursor together as two positions plus booleans for which end was the cursor and whether a selection existed. Higher-level code still treated them separately, and getting the cursor meant testing a boolean. Separating them, each built on a new Position class (line number plus character in line), simplified both use and implementation.
  • Putting the whole undo mechanism in the text class: the general history list, plus undo handlers for selection and cursor that called back into the UI. Each new undoable entity required changes to the text class.
public class History {
public interface Action {
public void redo();
public void undo();
}
History() {...}
void addAction(Action action) {...}
void addFence() {...}
void undo() {...}
void redo() {...}
}
  • What it demonstrates: a general-purpose mechanism that knows nothing about what its actions do. The text class creates UndoableInsert and UndoableDelete actions; the UI creates UndoableSelection and UndoableCursor actions. An undo steps back through the list until it reaches a fence, and high-level code decides where fences go by calling addFence. (The book’s text says “Each call to History.redo walks backwards” here, which appears to be a slip for undo.)

Figures 9.1 and 9.2 (C++ packet handling): in 9.1, a switch (common->opcode) has case DATA, case GRANT, and case RESEND. Each case calls received->getStart<...Header>() and, if it returns NULL, repeats the same multi-line LOG(WARNING, "%s packet from %s too short (%u bytes)", ...) statement before returning. In 9.2, each case does goto packetTooShort;, and a single packetTooShort: label after the switch holds the one LOG call and the return.

The separate logging class. A student class had several sequences like this:

try {
rpcConn = connectionPool.getConnection(dest);
} catch (IOException e) {
NetworkErrorLogger.logRpcOpenError(req, dest, e);
return null;
}

At the end of the same file, a private static class NetworkErrorLogger held methods such as logRpcOpenError, logRpcSendError, and logRpcReceiveError. Each method was one logger.log(Level.WARNING, ...) line with a long documentation comment, and each was called from only one place.

  1. Diagnosis. The methods were shallow and conjoined. Someone reading the call would flip to the logger to check what was logged, and someone reading the logger would flip back to the call site to see why.
  2. Fix. Delete the logging methods and write the logging statement where the error is detected. The code becomes easier to read and the logging interfaces disappear.
  3. Rule applied. These pieces were related in every sense on the checklist: shared information, always used together, and hard to understand apart. So they belong together.
  1. Before splitting, count what the split adds: interfaces, management code, separation, and duplication.
  2. Join code that shares information, simplifies an interface when combined, or is repeated.
  3. Keep general mechanisms free of special-purpose code, and push the special parts up a layer.
  4. Split a method only to factor out a cleanly separable subtask, or to divide an interface that does unrelated things.
  5. Never split into shallow methods that callers must chain together.
  6. Treat two methods that can’t be understood without reading each other as a signal to join (Conjoined Methods).
  • Ch 4: depth, classitis, and why small is not the goal.
  • Ch 5: shared information between classes is leakage.
  • Ch 6: general-purpose interfaces; the text class and Position.
  • Ch 7: pass-through methods are a symptom of poor division.
  • Ch 10: the empty selection removes another special case.