“Clean Code” Book Club: Chapter 3, Functions (Part 2)

Continuing with our book club, this week we’ve finished chapter 3 (“Functions”), covering pages 40–52. For boring reasons, I missed part of the discussion this time, so this will be a shorter than usual post. I’ll focus solely on the “Function Arguments” subsection, which inspired by far the most discussion, and skip the rest.

A few days ago, a colleague shared the post “It's probably time to stop recommending Clean Code” with our book club. I wrote last week’s notes before reading that post, so I found it encouraging to notice overlap between both—and sometimes quite cathartic to see vocal criticism of other issues in the chapter that subconsciously irked me but that I wasn’t able to articulate precisely.

Chapter 3: Functions

Function Arguments

The ideal number of arguments for a function is zero. Next comes one, followed closely by two. Three arguments should be avoided where possible. More than three requires very special justification—and then shouldn’t be used anyway. (p. 40)

As so often in this chapter, I think the basic point (too many arguments tend to correlate to messy code) is reasonable; but by focussing too much on a metric that’s easy to measure, but not very accurate, Martin once again gives advice that leads to bad code.

For example:

includeSetupPage() is easier to understand than includeSetupPageInto(newPageContent). The argument is at a different level of abstraction than the function name and forces you to know a detail that isn’t particularly important at that point. (p. 40)

Skipping the argument makes it easier to understand that one line, perhaps. But if I want to understand what the code does, knowing where that setup page gets included is pretty important. And leaving out that single argument means I have to instead look up and read the whole definition of the includeSetupPage() function.

Another example can be found in this extract from listing 3-7:

public class SetupTeardownIncluder {
  private PageData pageData;
  private boolean isSuite;
  private WikiPage testPage;
  private StringBuffer newPageContent;
  private PageCrawler pageCrawler;

  private String render(boolean isSuite) throws Exception {
    this.isSuite = isSuite;
    if (isTestPage()) // <-- this is the offending line
      includeSetupAndTeardownPages();
    return pageData.getHtml();
  }
}

If what is a test page?

It’s gotta be one of the five member variables listed above … but can you guess which of them? Defining isTestPage() without any arguments means I have to glance up at the list of member variables, which already makes the code harder to understand than simply having that one argument.

What makes this case particularly egregious, though, is that the naming is actively misleading: Based on both the class names and variable names, clearly only one of the five (the WikiPage testPage) is a “page”—so surely that would be the page that isTestPage() checks, right? Wrong.¹

In a later sub-section, Martin argues that flag arguments (i.e., any argument that is a boolean) are “a truly terrible practice”, because they mean that a function “does more than one thing” and should be split into two different functions. But often, flag arguments don’t determine whether to do one thing or another; they just specify one small aspect of how that one thing is done. For example: Should the function do a “dry run” and list the changes that would happen or actually modify the data? Should it modify an array in place or return results in a new array? Should it print verbose output or not?

Splitting such things up into separate functions would be clunky; especially if multiple flags could apply independently. (Imagine that API: performOperationVerboselyInPlace(), performOperationQuietlyInPlace(), performOperationVerboselyNondestructive(), performOperationQuietlyNondestructive(), … 😱)

Martin is correct that a context-less true or false in a function call is often confusing. But in other languages, like Python, that is easily solved by using keyword arguments. For example:

deleteOldFiles("/usr/local/", True)  # Confusing :(
deleteOldFiles("/usr/local/", dry_run=True)  # Perfectly clear :)

This also solves a problem Martin mentions for functions with multiple arguments that don’t have an obvious ordering (e.g., assertEquals(expected, actual) or assertEquals(actual, expected)).

Some languages even go one step further: Objective-C (example here) and Swift (example below) distinguish internal and external keyword names:

func divide(numerator: Int, byDenominator denominator: Int) -> Int? {
    if denominator == 0 {
        return nil
    }

    return numerator / denominator
}

So while the arguments are named numerator and denominator in the function body, the function can be called as divide(numerator: 42, byDenominator: 7), which sounds even closer to an English sentence.

1. The correct answer would have been pageData—and the only way to figure that out is, once again, to look up and read through the function definition. ↩