In my previous article I mentioned the book "Clean Code" - this article is a brief critique of it (in some cases you'll just have to read the book to see what I'm talking about because I'm not going to rewrite the book here!).
What I liked
The first chapter is excellent - particularly pages 4-6 (buy the book to find out what they contain). I got the same feeling of wanting to get everyone to read these pages as I had from the section about comments in Kent Beck's Smalltalk Best Practice Patterns. I wanted to shout (while shaking people by their lapels) "read this - it's what I've been trying to tell you all this time".
What I didn't like - examples (particularly Chapter 3)
There are some examples that aren't great. The one that stuck out particularly badly was in chapter 3 - HtmlUtil. The problem with HtmlUtil is that it is classic procedural style - which is OK as an example of bad style - but the refactored version does not address that aspect of it's badness.
This example is a method on HtmlUtil with signature:
public static String testableHtml(PageData pageData, boolean includeSuiteSetup) throws Exception
with lots of methods called on pageData. It gets nicely refactored into a short method of a different name but with the same signature.
My main problem with this example is that it looks (to someone who doesn't know the codebase that it is taken from) as if this should be a method on PageData. Maybe there is a good reason it is a static method on HtmlUtil but the author doesn't explain whether there is or not. To me it seems like an obvious question and by not mentioning anything about Utils with static methods that do things to objects that should be quite capable of doing stuff for themselves, it shows an example of arguably bad code, early on, with no explanation or apology for using such an example. I assume that they wanted to focus only on procedural refactoring for the example and deliberately wanted to avoid anything object-oriented - but if so, they should have said that very clearly.
The second red flag that this example waved at me was declaring that the method throws Exception. It isn't clear from reading the code that it needs to. Maybe it does, but again I think if it does then the author should have explained why and apologised for that aspect of the example.
What I didn't like - Structured Programming (Chapter 3)
In the (very short) section on Structured Programming there is a claim for the rules of structured programming; "It is only in larger functions that such rules provide significant benefit." I almost screamed at my fellow passengers on the train into work. My blood pressure is rising as I write this. (Imagine me screaming at the top of my voice - "NO NO NO").
I don't believe there is ANY benefit, let alone significant benefit, of applying the rules of structured programming to Java code. If there is, then the author should justify their comments rather than invoke the "proof by repeated assertion" that the cargo cult followers will no doubt spout in comments on this article.
What I liked - formatting (Chapter 5)
Lots of good stuff - I didn't agree with it all but some of it has definitely made me rethink (or in some cases just think) about what I'm doing with formatting more than before.
What I didn't like - Chapter 11 (Systems)
Any chapter which starts with an analogy for building a software system as building a city, has already got off to a bad start as far as I'm concerned. Whenever I hear of such analogies I wonder what Swiss Toni's analogy would be. "Building a system is very much like making love to a beautiful woman ..."
Apart from the analogy - the author then goes on to talk about Spring somewhat implying that it's a Good Thing. Just don't get me started. That's a whole other blog post.
Nevertheless, despite these things, there is also some good stuff in this chapter.
Overall
Great. Buy it. It's full of good stuff and only very few things that make my blood boil.
Copyright © 2008 Ivan Moore
1 comment:
As you say, the section on structured programming is very short. It only talks about one structured programming rule: 'every function, and every block within a function, should have one entry and one exit'. The author says that while the rule provides little benefit in small functions, it provides significant benefits in larger functions. I agree with you that they should justify their claim.
*** In Java Style - Patterns for Implementation, while dismissing the rule as an anachronism, Jeff Langr implicitly gives us some idea about why the rule is useful in long methods:
Somewhere [in Dijkstra's paper "Goto Statement Considered Harmful"] I also remember a platitude about not returning out of a routine in its middle. Of course that was back in the days when functions or subroutines were dozens of lines long...within the context of a brief method, then, a return statement is not nearly as obscured as one within a large nesting of other statements. Guard Clause, one of my favourite formatting patterns, promotes early exist from methods.
*** In Code Complete, Steve McConnel, does gives us at least some concrete justification for the rule:
Minimise the number of returns in each routine - it's harder to understand a routine when, reading it at the bottom, you are unaware of the possibility that it returned somewhere above. For that reason, use returns judiciously - only when they improve readability. - Btw, he does advocate using Guard Clause to enhance readability and simplify complex error processing)
*** In Implementation Patterns, Kent Beck provides a more high-level justification:
Back in the old days of programming, a commandment was issued: each routine shall have a single entry and a single exit. This was to prevent the confusion possible when jumping into and out of many locations in the same routine. It made good sense when applied to FORTRAN or assembly language programs written with lots of global data where even understanding which statements were executed was hard work. In Java, with small methods and mostly local data, it is needlessly conservative. However, this bit of programming folklore, thoughtlessly obeyed, prevents the use of guard clauses.
Cheers.
Post a Comment