11

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?

Malady
  • 109

5 Answers5

22

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.

7

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.

Doc Brown
  • 218,378
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 ;-)

Christophe
  • 81,699
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.

Kilian Foth
  • 110,899
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.

chepner
  • 183