Robert C. Martin's Clean Code Tip of the Week #2: The Inverse Scope Law of Function Names
Like this article? We recommend
Like this article? We recommend
Like this article? We recommend
I have a simple rule. The longer the scope of a function, the shorter its name should be. Functions that are called from far and wide should have short evocative names like push, close, read, or kill. Functions that are called locally from a few nearby places should have long descriptive names like parseColumnHeader or callScenarioForRow. Indeed, the longest function names should be given to those functions that are called from just one place.
Consider a function like parseColumnHeader below:
protected void parseColumnHeader() {
headerColumns = table.getColumnCountInRow(1);
for (int col = 0; col < headerColumns; col++) {
String cell = table.getCellContents(col, 1);
if (cell.endsWith("?"))
funcs.put(cell.substring(0, cell.length() - 1), col);
else
vars.put(cell, col);
}
}
This function has a very small scope. It's called from two places. Once from production code, and once more from a unit test. Here is the production code that calls it:
private void invokeRows() {
parseColumnHeader();
for (int row = 2; row < table.getRowCount(); row++)
invokeRow(row);
}
What does the function name tell us about its behavior? What does it mean to "parse" a "column header". One of my guidelines for clean code is that you should not have to look inside a function to know what it does. Instead the names of each called function should give you enough information so that you can continue to read without jumping to their implementation. In this regard the name parseColumnHeaders fails.
So let's look at the implementation of parseColumnHeader and see if we can come up with a better name. The first line tells us that the first row of the table contains column headers. (perhaps the name of the variable should be changed from headerColumns to columnHeaders). The next line tells us that we are looping through each column of the column headers. Inside the loop we check the column header to see if it ends with a ?. If so we put the column header into a list named funcs; otherwise we put it into a list named vars.
So, what this function is doing is allocating the column headers between the funcs and vars lists. Can we say this in an evocative function name? How about gatherFunctionsAndVariablesFromColumnHeader? That certainly tells us the intent of the function. It also makes invokeRow much easier to understand.
private void invokeRows() {
gatherFunctionsAndVariablesFromColumnHeader();
for (int row = 2; row < table.getRowCount(); row++)
invokeRow(row);
}
Now we can see that before we invoke a row we figure out which columns are functions and which are variables.
A long function name like this is much better than a comment because its documentary value appears both where the function is defined, and also where the function is used. Moreover, a long and precise name like this is much more likely to be kept long and precise, than a name like parseColumnHeaders. For example, if a new kind of column were added, we'd likely change the name of gatherFunctionsAndVariablesFromColumnHeader but we'd likely not change the name of parseColumnHeaders.
You might think that changing the name of the function is a bad idea, and that we should choose names that are not likely to change. Indeed, isn't that what the Open Closed Principle is all about? Yes and no. You see there is another rule. As behavior gets more complex, the function name should become more generic and the complexity should be managed by extracting simpler functions. So if we added a new column type, we might change the name of the function to gatherInvocationParametersFromColumnHeaders, and then we might break the function into several smaller functions.
If the name of the function remained parseColumnHeaders then we would not be so tempted to break the function into smaller functions. We could just live with the imprecise name and let the function get ever more complicated. But with a name like gatherFunctionsAndVariablesFromColumnHeader we are almost forced to break the function up and change the name to something more generic, but not less precise.
Indeed the gatherFunctionsAndVariablesFromColumnHeader function is probably already too large. We could easily split it as follows:
protected void gatherFunctionsAndVariablesFromColumnHeader() {
columnHeaders = table.getColumnCountInRow(1);
for (int col = 0; col < columnHeaders; col++)
putColumnHeaderInFunctionOrVariableList(col);
}
private void putColumnHeaderInFunctionOrVariableList(int col) {
String cell = table.getCellContents(col, 1);
if (cell.endsWith("?"))
funcs.put(cell.substring(0, cell.length() - 1), col);
else
vars.put(cell, col);
}
The morals of this story are:
-
Functions in small scopes should have long and precise names.
-
You should not have to read the body of a function to know what it does. It's name should tell you.
-
The more complex the behavior of a function, the more generic its name, and the more sub functions should be extracted from it.