{"id":609404,"date":"2023-02-18T07:49:00","date_gmt":"2023-02-18T13:49:00","guid":{"rendered":"https:\/\/news.sellorbuyhomefast.com\/index.php\/2023\/02\/18\/its-probably-time-to-stop-recommending-clean-code-2020\/"},"modified":"2023-02-18T07:49:00","modified_gmt":"2023-02-18T13:49:00","slug":"its-probably-time-to-stop-recommending-clean-code-2020","status":"publish","type":"post","link":"https:\/\/newsycanuse.com\/index.php\/2023\/02\/18\/its-probably-time-to-stop-recommending-clean-code-2020\/","title":{"rendered":"It's probably time to stop recommending Clean Code (2020)"},"content":{"rendered":"
\n

It may not be possible for us to ever reach empirical definitions of “good code” or “clean code”, which means that any one person’s opinions about another person’s opinions about “clean code” are necessarily highly subjective. I cannot review Robert C. Martin’s 2008 book Clean Code<\/i><\/a> from your perspective, only mine.<\/p>\n

That said, the major problem I have with Clean Code<\/i> is that a lot of the example code in the book is just dreadful<\/em>.<\/p>\n

*<\/h4>\n

In chapter 3, “Functions”, Martin gives a variety of advice for writing functions well. Probably the strongest single piece of advice in this chapter is that functions should not mix levels of abstraction<\/strong>; they should not perform both high-level and low-level tasks, because this is confusing and muddles the function’s responsibility. There’s other valid stuff in this chapter: Martin says that function names should be descriptive, and consistent, and should be verb phrases, and should be chosen carefully. He says that functions should do exactly one thing, and do it well, which I agree with… provided that we aren’t too dogmatic about how we define “one thing”, and we understand that in plenty of cases this can be highly impractical. He says that functions should not have side effects (and he provides a really great example), and that output arguments are to be avoided in favour of return values. He says that functions should generally either be commands, which do<\/em> something, or queries, which answer<\/em> something, but not both. This is all reasonable entry-level advice.<\/p>\n

But mixed into the chapter there are more questionable assertions. Martin says that Boolean flag arguments are bad practice, which I agree with, because an unadorned true<\/code> or false<\/code> in source code is opaque and unclear versus<\/i> an explicit IS_SUITE<\/code> or IS_NOT_SUITE<\/code>… but Martin’s reasoning is rather that a Boolean argument means that a function does more than one thing, which it shouldn’t.<\/p>\n

Martin says that it should be possible to read a single source file from top to bottom as narrative, with the level of abstraction in each function descending as we read on, each function calling out to others further down. This is far from universally relevant. Many source files, I would even say most source files, cannot be neatly hierarchised in this way. And even for the ones which can, an IDE lets us trivially jump from function call to function implementation and back, the same way that we browse websites.<\/p>\n

He says code duplication “may be the root of all evil in software” and fiercely advocates DRY<\/a>. At the time, this was quite standard advice. In more recent times, however, we generally understand that a little duplication isn’t the worst thing in the world<\/a>; it can be clearer, and it can be cheaper than the wrong abstraction<\/a>.<\/p>\n

And then it gets weird. Martin says that functions should not be large enough to hold nested<\/em> control structures (conditionals and loops); equivalently, they should not be indented to more than two levels. He says blocks should be one line long, consisting probably of a single function call. He says that an ideal function has zero arguments<\/em> (but still no side effects?), and that a function with just three arguments is confusing and difficult to test. Most bizarrely, Martin asserts that an ideal function is two to four lines of code long<\/em>. This piece of advice is actually placed at the start of the chapter. It’s the first and most important rule:<\/p>\n

\n

The first rule of functions is that they should be small. The second rule of functions is that they should be smaller than that<\/em>. This is not an assertion that I can justify. I can\u2019t provide any references to research that shows that very small functions are better. What I can tell you is that for nearly four decades I have written functions of all different sizes. I\u2019ve written several nasty 3,000-line abominations. I\u2019ve written scads of functions in the 100 to 300 line range. And I\u2019ve written functions that were 20 to 30 lines long. What this experience has taught me, through long trial and error, is that functions should be very small.<\/p>\n

[…]<\/p>\n

When Kent [Beck] showed me the code, I was struck by how small all the functions were. I was used to functions in Swing programs that took up miles of vertical space. Every function in this<\/em> program was just two, or three, or four lines long. Each was transparently obvious. Each told a story. And each led you to the next in a compelling order. That\u2019s<\/em> how short your functions should be!<\/p>\n<\/blockquote>\n

All of this sounds like hyperbole. A case for short functions instead of long ones can certainly be made, but we assume that Martin doesn’t literally<\/em> mean that every function in our entire application must<\/em> be four lines long or less.<\/p>\n

But the book is being absolutely serious about this. All of this advice culminates in the following source code listing at the end of chapter 3. This example code is Martin’s preferred refactoring<\/strong> of a pair of Java methods<\/a> originating in an open-source testing tool, FitNesse<\/a>.<\/p>\n

\npackage fitnesse.html;\n\nimport fitnesse.responders.run.SuiteResponder;\nimport fitnesse.wiki.*;\n\npublic class SetupTeardownIncluder {\n  private PageData pageData;\n  private boolean isSuite;\n  private WikiPage testPage;\n  private StringBuffer newPageContent;\n  private PageCrawler pageCrawler;\n\n\n  public static String render(PageData pageData) throws Exception {\n    return render(pageData, false);\n  }\n\n  public static String render(PageData pageData, boolean isSuite)\n    throws Exception {\n    return new SetupTeardownIncluder(pageData).render(isSuite);\n  }\n\n  private SetupTeardownIncluder(PageData pageData) {\n    this.pageData = pageData;\n    testPage = pageData.getWikiPage();\n    pageCrawler = testPage.getPageCrawler();\n    newPageContent = new StringBuffer();\n  }\n\n  private String render(boolean isSuite) throws Exception {\n     this.isSuite = isSuite;\n    if (isTestPage())\n      includeSetupAndTeardownPages();\n    return pageData.getHtml();\n  }\n\n  private boolean isTestPage() throws Exception {\n    return pageData.hasAttribute(\"Test\");\n  }\n\n  private void includeSetupAndTeardownPages() throws Exception {\n    includeSetupPages();\n    includePageContent();\n    includeTeardownPages();\n    updatePageContent();\n  }\n\n\n  private void includeSetupPages() throws Exception {\n    if (isSuite)\n      includeSuiteSetupPage();\n    includeSetupPage();\n  }\n\n  private void includeSuiteSetupPage() throws Exception {\n    include(SuiteResponder.SUITE_SETUP_NAME, \"-setup\");\n  }\n\n  private void includeSetupPage() throws Exception {\n    include(\"SetUp\", \"-setup\");\n  }\n\n  private void includePageContent() throws Exception {\n    newPageContent.append(pageData.getContent());\n  }\n\n  private void includeTeardownPages() throws Exception {\n    includeTeardownPage();\n    if (isSuite)\n      includeSuiteTeardownPage();\n  }\n\n  private void includeTeardownPage() throws Exception {\n    include(\"TearDown\", \"-teardown\");\n  }\n\n  private void includeSuiteTeardownPage() throws Exception {\n    include(SuiteResponder.SUITE_TEARDOWN_NAME, \"-teardown\");\n  }\n\n  private void updatePageContent() throws Exception {\n    pageData.setContent(newPageContent.toString());\n  }\n\n  private void include(String pageName, String arg) throws Exception {\n    WikiPage inheritedPage = findInheritedPage(pageName);\n    if (inheritedPage != null) {\n      String pagePathName = getPathNameForPage(inheritedPage);\n      buildIncludeDirective(pagePathName, arg);\n    }\n  }\n\n  private WikiPage findInheritedPage(String pageName) throws Exception {\n    return PageCrawlerImpl.getInheritedPage(pageName, testPage);\n  }\n\n  private String getPathNameForPage(WikiPage page) throws Exception {\n    WikiPagePath pagePath = pageCrawler.getFullPath(page);\n    return PathParser.render(pagePath);\n  }\n\n  private void buildIncludeDirective(String pagePathName, String arg) {\n    newPageContent\n      .append(\"n!include \")\n      .append(arg)\n      .append(\" .\")\n      .append(pagePathName)\n      .append(\"n\");\n  }\n}\n<\/pre>\n
I’ll say again: this is Martin’s own code, written to his personal standards. This is the ideal, presented to us as a learning example.<\/p>\n
I will confess at this stage that my Java skills are dated and rusty, almost as dated and rusty as this book, which is from 2008. But surely, even in 2008, this code was illegible trash?<\/p>\n
Let’s ignore the wildcard import<\/code>.<\/p>\n
First, the class name, SetupTeardownIncluder<\/code>, is dreadful. It is<\/em>, at least, a noun phrase, as all class names should be. But it’s a nouned verb phrase, the strangled kind of class name you invariably get when you’re working in strictly object-oriented code, where everything has to be a class, but sometimes the thing you really need is just one simple gosh-danged function<\/em>.<\/p>\n
Inside the class, we have:<\/p>\n