8

This doubt is about function doing one thing from Chapter 3: Functions of the book named Clean Code

Here Uncle Bob is talking about this function:

public static String renderPageWithSetupsAndTeardowns(
  PageData pageData, boolean isSuite) throws Exception{
  if (isTestPage(pageData))
    includeSetupAndTeardownPages(pageData, isSuite);
  return pageData.getHtml();
}

In this function I can see, it's doing three different things:

  • Determining whether the page is a test page.
  • If so, including setups and teardowns.
  • Rendering the page in HTML.

But Uncle Bob says:

If a function does only those steps that are one level below the stated name of function, then the function is doing one thing.

What does that means? How does this statement proves that the above function is doing one thing?

1
  • I would agree that the example shown is not a particular good example since it mixes test apects with code, but given its name it does one function by delegating one level deeper. – eckes Jun 5 at 13:06
19

The fact that you were able to so easily make that list of bullet points is of relevance here :)

This question might be considered opinion based, but this is about levels of abstraction. The concept may appear too academic, almost intimidating, but it just refers to the level of detail with which you express things, and the choice of particular detail you include - and this depends on who are you talking to and why.

E.g., in an informal scenario, if someone asks you "When will you be available?", there's a difference between you answering "I'll get back to you in a week." and "Oh, man, I'm traveling, I'm going to this rock concert, the stage is within this medieval fortress, and I've heard so many good things from people, it's going to be such a blast, you're never going to believe who's playing [etc., etc.]!". In the second case, the person who asked the question might be able to extract the information relevant to them eventually, but the first level of abstraction is likely preferred - e.g., in a business scenario (where the person is busy and they just want to understand when you'll be available). Similarly, if someone asks you "Where are you going?", but from the context, it's clear to you that they aren't interested in all the detail, you could just say "To a rock concert in Hungary" - thus making a different choice of the relevant details.

In software, you're expressing behavior using functions, and you're faced with similar choices, creating conceptual levels of abstraction. This is in part for the benefit of the readers of your code (other programmers, or future you), and in part for organization and maintainability. So, the exact natures of "level of abstraction", and "one thing", or "single responsibility" are necessarily somewhat up to you and depend on the particular problem you're trying to solve with your program. You try to identify different axes of change and divide responsibilities according to that, on a multiple hierarchical levels of abstraction (you may need to continue to refactor towards this as you work on the project - this is not something you're going to get completely right from the very start).

Now to the core of your question:

In this function I can see, it doing three different things:

  • Determining whether the page is a test page.
  • If so, including setups and teardowns.
  • Rendering the page in HTML.

What this function is doing is orchestrating those other functions which are one level of abstraction below. Its job is to put them together and decide what gets called when. Those other functions are doing the actual, individual things.

The same idea applies recursively, within those lower-level functions as well.

But Uncle Bob says:
"If a function does only those steps that are one level below the stated name of function, then the function is doing one thing."

I'm pretty sure that's what he means here. As a rule of thumb, if a function is only orchestrating functions that are one abstraction level below it, then there's a good chance it's doing a single thing in this sense. Also, the code is more declarative - you can almost read it as a sentence, as you've demonstrated by creating your list of bullet points to which it maps almost 1-to-1. And it doesn't dig down into the lower level concerns that are not its business. Otherwise you'd have to mentally parse a bunch of if-s or loops and strangely named variables to even figure out what is the high-level thing the function is trying to do - this scenario corresponds to the situation described at the start, where a blabbering answer is given to a higher-level question.

For the most part, when someone looks inside a function, they generally want to know what's it for, what its job. It's a high level question. Be deliberate about levels of abstraction and choose good names, and refactor to make sure that they can just read the answer off of your implementation. Don't settle for blabbering functions; your readers should be able to make a sensible bulleted list, just like you have done. If they need more detail, they can dig in, and if your names are good, they'll know there to look.

On the next page, Uncle Bob says: "We want the code to read like a top-down narrative". This is, more or less, the idea he's describing there.

5
  • 1
    includeSetupAndTeardownPages() is one level of abstraction below renderPageWithSetupsAndTeardowns() right? So if there was a function like setup() inside the includeSetupAndTeardownPages() then the setup() will be two level of abstraction below renderPageWithSetupsAndTeardowns() right? Is the level of abstraction is measured by how we hide the low level details ? renderPageWithSetupsAndTeardowns() has higher level of abstraction than the setup() because renderPageWithSetupsAndTeardowns() indirectly abstracts or hides the setup() function so do its definition. – Chinky Sight Jun 5 at 9:51
  • 1
    @ChinkySight Yes, but note that it's not necessarily so mechanical. And it's not merely about containment; what you do is you strive to form an understandable narrative for whoever is reading the code, constrained by other things you need to do (like the need to invert certain dependencies, and so on). The process is not necessarily straightforward. You might start by writing everything in terms of details. Then you put a comment on top of a group of lines to explain what they do. Then you realize that maybe you should pull them out into a function and explicitly name the concept. 1/4 – Filip Milovanović Jun 5 at 11:19
  • 4
    Two programmers might come up with different concepts and different levels of abstractions. That's fine. What's problematic is when the code doesn't have any. Not only because it's hard to read, but because the lack of explicit structuring leads to all kinds of accidental/nonobvious coupling between different parts of the code; this will come to bite you back later. Separating things into functions imposes clear boundaries, and explicitly defines ways of crossing those boundaries (via function calls and parameter lists) - if you come up with cohesive and loosely coupled functions. 2/4 – Filip Milovanović Jun 5 at 11:19
  • 4
    Since you have access to the book, go to the section, find "Listing 4-7: GeneratePrimes.java". That's an example of "blabbering" code (everyone writes code like that, even the best programmers out there - it's just that we should all strive to make our code better). It's just a wall of instructions; after some carefull reading, you can figure out the major steps. However, the author knows it's a bit of a tedious task, so they have sprinkled the code with comments, trying to tell you what the code itself doesn't tell you directly (in a straightforward way). 3/4 – Filip Milovanović Jun 5 at 11:19
  • 5
    In real-world applications you may run into functions that are 10x longer, much harder to understand, and in a dire need for cleanup. Listing 4-8 is the same code after refactoring; the generatePrimes method now directly tells you what the major steps are (assuming you understand how the Sieve of Eratosthenes algorithm works, which you can look up). Same the level below - take a look, for example, at crossOutMultiples() - again, it tells you what it does, wheres it was originally this cryptic loop under the //sieve comment. 4/4 – Filip Milovanović Jun 5 at 11:19
6

Some good answers already here, but I would like to highlight some aspects here even more:

"I can see, it's doing three different things"*

That is not what I see, quite the opposite:

  • "Determining whether the page is a test page" is done by a different function (isTestPage).

  • "including setups and teardowns" is done by includeSetupAndTeardownPages

  • "Rendering the page in HTML" is done by pageData.getHtml()

The "one thing" renderPageWithSetupsAndTeardowns does is to chain these calls together.

There are two things to care for here when trying to write "clean code" (or maintainable, evolvable code):

  • all these steps should be on the same level of abstraction - this is also known as the SLA principle.

  • it makes sense to group them under a common name, so whenever one calls it, one can "forget" about some or most of the internal details

Of course, there is no hard-and-fast rule to decide when two code sections are on the same level of abstraction, or on different ones. Developers and analyst usually make a mental model of what they understand as "one step" inside a function, and what kind of steps are "equally complex, to a comparable degree". You did this, for example, by enumerating those three bullet points. When now one of these bullet points would be expressed by a single function call, and another one by a code section complex enough not to be understandable without an introductory comment line, then I would usually perceive this as not at the same level of abstraction.

Let me finally add, you will be better off to take all of Bob Martin's advice with a grain of salt, "Clean Code" is not a religous books and Bob Martin not a prophet.

6
  • 1
    What does he meant by "one level of abstraction"? – Chinky Sight Jun 5 at 10:59
  • By curiosity, may I ask: How do you recognize a “level of abstraction”? What objective criteria determine that it’s the “same level” of abstraction? – Christophe Jun 5 at 11:26
  • @Christophe IMO, there aren't necessarily objective criteria (although some things work better then others). I guess it depends in part on your communication style (how you communicate ideas using code), and in part on how you approach design. I think there are aspects that are commonly regarded as jumping levels of abs. - e.g. huge jumps in granularity of tasks, or mixing business logic and code that calls some low-level library. Different people will come up with different levels of abstraction, and associated concepts; the real problem is when the code doesn't make any effort to have any. – Filip Milovanović Jun 5 at 11:40
  • @FilipMilovanović What meant to suggest with ly question, is that “level of abstraction” is very fuzzy and difficult to grasp for people who do not have a sufficient experience and a source of subjective arguments for others. Moreover Uncle Bob describes (you made the point in your own answer) in reality a simple level in an arbitrary top down decomposition. Each of these levels may be unrelated to the concept of “abstraction”: objectively, “rendering a page” is not more abstract than “checking if a page is a test page”. And factorial is not less abstract than factorial ;-) – Christophe Jun 5 at 12:54
  • 1
    @ChinkySight: see my edit – Doc Brown Jun 5 at 20:18
5

Can we count things a function does?

Usually function perform many operations and statements. Even the very simplest function like:

int f(int a, int b) { return a*a+2*a*b+b*b; }
// one statement, but at least 6 operation

But there are different ways to do the same thing. For our example we could have implemented it like:

int f(int a, int b) { 
    int a2 = pow(a,2);
    int b2 = pow(b,2);
    int t = 2*a*b;
    return a2+t+b2;
}
// 4 statements 4 operation and 2 function calls

Regardless of its implementation, this example function does only one thing: calculating the square of a sum of two terms. This shows that “things” are difficult to measure: it’s subjective.

More over, the thing a function shall do is often defined before the function is even implemented. So shouldn’t counting “things” a function does be more about its purpose, goal or intent?

“one thing” = “one goal” ?

Real life functions, do much more complex things. The strategy to address complexity is functional decomposition:

  • a complex function with a goal is decomposed into a combination simpler functions
  • each of the simpler function has a sub-goal (i.e. a smaller goal that contributes to the larger goal),
  • each of the simpler functions can itself be decomposed further until the sub-goal is elementary.

The second bullet is one level below the first. Since the sub-goal is necessarily related to the original goal, it contributes to the same thing.

Conclusion

Uncle Bob’s statement says that doing “one thing” allows functional decomposition, under the condition that levels are not short-circuited (i.e some more elementary tasks would be left out of the decomposition).

I suspect however that this specific quote does by accident not work with recursive functions, even if those do one thing and do it well, because they reuse a step at the same level (and not only “one step below”). But that is an idea for your next question ;-)

3
  • 1
    Re: The recursion issue at the end; nicely, usable recursion essentially does need to call itself with one level simpler input (or more), to avoid the recursion being an infinite loop. – Daniel R. Collins Jun 5 at 13:46
  • @DanielR.Collins Thank you! I love clean code, but I’m also fascinated by recursion. You got a point: In every recursive function you’d indeed have a starting point. Which means that you express the function with one lower level of abstraction (e.g factorial(1)->1) and the same level (e.g factorial(n)->n*factorial(n-1)). I think the function nevertheless does only one thing and correspond to clean code, despite it doesn’t fit in the quote. Would you agree? :-) – Christophe Jun 5 at 13:57
  • 4
    It's certainly clean and does one thing. Frankly the quote is itself unclear, and I find "Uncle Bob" to be likewise usually unclear. Nonetheless, if one is hunting for a "one level below" criterion, then I think that's much clearer in the recursive case. – Daniel R. Collins Jun 5 at 16:14
3

The entire idea of abstraction rests on levels. "Render the HTML of page X" is a pretty high-level task.

One level below there are tasks like "render the header"/"render the body". Below that you would have tasks like "find the content for this user" and "turn it into HTML".

"Turn it into HTML" again requires the handling of tags of many different kinds. At some point this task requires handling string variables and literals. This in turn will require calling into the standard library (the standard library also contains code on various levels, although this is not normally visible to the caller).

The software engineering principle in question states that a function should fulfill a task at level X by calling operations from level X-1, not level X-2 or even level 1. According to this principle, this would be very bad code:

public static String renderPageWithSetupsAndTeardowns(
  PageData pageData, boolean isSuite) throws Exception{
    // BAD!
     String html = setupAndTeardownPages(pageData, isSuite);
    html += "</html>"
    return html;
}

The example function keeps its calls on the same level instead, therefore according to the principle it is okay.

0

I'm going to argue that this function does two things, rather than one. The name implies that it will unconditionally preprocess the page and then render it; I accept that as "one thing". However, it also has to decide whether to preprocess the page, something not implied by the name.

Instead, the function should receive a preprocessor that will always be applied first; however, that preprocessor may not actually do anything. I don't know Java well enough anymore to demonstrate in that language, so I will switch to Python. The idea should be translatable.

First, we'll define a function that takes the page and a preprocessor function as arguments:

def renderPageWithSetupsAndTeardowns(page_data, preprocessor):
    preprocessor(pageData)
    return pageData.getHtml()

It always calls the preprocessor on the page, no matter what. Also note that isSuite is no longer an argument, because it only applied to a particular preprocessing step, not to the rendering process.

Now, it will be up to the caller to decide whether includeSetupAndTeardownPages is the preprocessor, or if a no-op preprocessor will be applied.

if isTestData(pageData):
    p = lambda pd: includeSetupAndTearDownPages(pd, isSuite)
else:
    p = lambda pd: None

renderPageWithSetupsAndTeardowns(page_data, p)

p is always a function that receives only the page; it is up to the caller to define an appropriate function. In the case of test data, the function will call includeSetupAndTearDownPages with the appropriate isSuite. (As a Python detail, isSuite is a free variable, but Python is lexically scoped, so it will lookup the same variable that was previously being used as the argument.)

If it is not test data, we don't do anything; we'll just return None, ignoring the given page, having the same effect as not calling p at all.

Once p is defined, we call renderPage... with the page and p, and the function will do exactly what it says: preprocess the page with p, then render the resulting page.

Not the answer you're looking for? Browse other questions tagged or ask your own question.