19May

Review: Clean Code, by Robert Martin

Posted by Elf Sternberg as programming

It might seem like I’ve been harsh on Robert Martin’s Clean Code for the past couple of posts, and that’s valid. I have been. It’s such a good book, full of strong advice on any number of topics.

It’s just that it feels old. Programming is a young discipline in the world, probably one of the youngest, and one of the most consequential. It changes with absurd speed, and everyone in it struggles to keep up. Clean Code came out in 2006 and already there are dusty corners within that feel out of date, even irresponsible.

So here’s what’s really great about the book:

The first chapter is, as necessary, an introduction. Martin introduces the idea of low-level programming discipline as a necessary precursor to writing large systems, and recommends the Boy Scout Rule: Always leave code better than when you found it. “Better” is a judgment call, and not everyone has good judgment, but it can be taught, and Martin has guidelines as to what “better” should be in the long run. And 90% of the time, I think he’s right.

Naming Things.

The second chapter is on naming things. Naming things is one of the hardest topics in software development, and there have been whole books just about how to pick a good name. Good names are about good taste, and while that can be taught it’s one of those things the learner has to want to learn. It has to be conscious.

Unfortunately, Martin’s book is wrapped deep into the Java / C# paradigm of object oriented programming, and his advice comes from that world, and only that world. This means that his section on naming methods is based, as he admits, on “the javabeans standard.” A better lesson is from Smalltalk: a method is a message to the object, which should be treated like a black box; what messages can you send it, what responses do you expect to receive back, and what state do you expect to find the system in after this request/response cycle is complete?

Functions

The chapter on functions made me queasy. He’s right that functions should be small (no, smaller than that), and that you should trust a good compiler to inline when it’s wise to do so.

His argument against switch statements makes sense until you see his response: prefer polymorphism over switching based on types. Given how much pain we’ve discovered in the years since due to polymorphism and inheritance in general, I’d rip both polymorphism and switch out and replace that sort of programming with a lookup table: cheap, easy, direct, comprehensible.

The Anti-If Patterns have more to teach about this topic, and I highly recommend them.

One thing that really made me mad, though, was the sentence “The ideal number of arguments for a function is zero.” No. Gods, no, this is horrible. A function of zero arguments is a mystery. You don’t know where the data is coming from. In the functional languages, functions of zero arguments are considered downright evil, and their type is often “TheWorld,” because that’s where the data they return comes from: the world outside your program. getLine() is a function of zero arguments, and it has to be wrapped in layers and layers of paranoia because you have no idea what your users are going to send you. Functions of zero arguments are where your security holes live. Whenever you write a function of zero arguments you should feel a cold chill run up your spine. (This is why I think Haskell is a valuable language to learn.)

(Methods of zero arguments… aren’t. They have an argument: the object with which they’re associated. .getWidth() is a message to the object to return a width value it contains (whether by storage or calculation), and .clear() can be a message to a collection to drop all of its contents. This is why I believe Smalltalk is a valuable language to learn.)

The section “a function should have no side effects” confused me until I realized that his definition of side effects was different from mine. A functional programmer knows that a side effect is anything that changes the outside world and has unpredictable consequences; Martin’s definition is that of a function that does two things, one of which isn’t clear from the function’s signature.

I’ve already discussed how DRY can sometimes lead to problematic obfuscation. DRY is great advice, but again, this is an issue of good sense and good taste, and an example of “debugging takes more smarts than developing; if you were at your limit writing the code, you don’t have the smarts to debug it.” The problem is that hacking a VM I’ve been using for 20 years isn’t the limit of my skills, but it’s already beyond the limits of the people who will maintain it. Downgrade your cleverness accordingly.

Comments

The comments section is just good advice. All of it. Comments are often clutter. Use them judiciously.

Formatting

Again, this is solid advice. Modern languages (Python, Rust, Go) have default formats, and good developers stick with them. It is a little funny that Martin has a long not-quite-sneer at Knuth for Literate Programming, and then tries hard to convince you that the layout of a source code file should “read like a book.”

Objects and Data

This chapter was pretty good, but again, as someone who has learned a lot from Haskell and Rust, I feel like this is an old chapter, well past its due date. Trait-based systems and pure functional languages have superseded much of this advice, and I genuinely feel sorry for anyone that has to work in a language like Java or C#.

Exception Handling

Again, this is one of those chapters that’s wedded to a single language paradigm, the one the big boys (Sun, Oracle, Microsoft) all tried to force upon us in the late 90s through the 00’s. We’ve seen through it. There are better worlds. Golang has adopted the “handle errors locally, or just die and let Kubernetes fix it for you,” Rust and F# and Haskell all have railway-oriented programming and scoped errors that make you think in onion architectures about what your system is doing.

Boundaries

The chapter on boundaries is pretty good. (Except… Jesus, Bob, what is it with the sexist illustrations? This stuff wasn’t acceptable when Weinberg wrote similar crap in 1974.) Keeping separate functionality separate is a keyword of all good design, which may explain why this chapter is short.

Testing

This is one of those chapters where the book really shines. I’m a huge fan of Test Driven Development. I really do believe in it, and my last project, in which I wrote the tests first, really did work well and I finished quickly. I even experimented with Martin’s Transformation Priority Premise and it worked pretty well! Tests must be in the required format and follow the required rules. I was taught test driven development back in the early 90’s, thanks to my Systems Design mentor, and I’ve been a fan of it ever since. Not every gist I’ve ever published has been wrapped in a TDD shell (they’re gists, after all), but I do prefer having tests to not.

The one thing I disagree with here is Fast. Tests should be no faster than the system requires; anything else gives you a false sense of reassurance.

Classes

The chapter on Classes is more or less a repeat of the Functions and Objects chapters. It’s okay, but it could have been distributed with some responsibility. This chapter is WET.

Three chapters: Systems, Emergence, and JUnit Internals.

These three chapters are, frankly, not very informative. The chapter on Systems is about designing in the large, but it tries too hard to convince you that aspect-oriented programming is a responsible development model, and that cross-cutting concerns can be dealt with in this manner without obfuscating the intent of the code.

The chapter on Emergence is basically “big systems behave in weird ways,” which is not surprising to anyone who’s ever worked on a large system. Much of this chapter has been superseded by the development of microservices, the discipline of system observability, and the understanding that very large systems live in a state of constant degradation; the only question is, how do you handle it, compensate for it, and structure your code to recover from it?

The JUnit Internals is… well, it is what it is.

Concurrency

This is a straightforward guide on handling multi-threaded environment, but again, its age shows through. So many pixels have been spent on this, and yet the biggest problem is that Martin (well, the contributor here, Brett Schuchert) does a terrible job of explaining how “clean code” interacts with concurrency.

If you want examples of how to do concurrency well, F#, Rust, Go, and Haskell all have powerful abstractions that help users develop concurrent code without pain. We know what the real pain is in concurrent development: multiple mutators having access to a single resource, and the associated locking nightmares that come with trying to control that access. Go and Rust have handled it in two different ways: Go by having a message-passing architecture, and Rust by having a model that forbids multiple references to a mutable resource by default, only enabling it with a permissive model that enforces locking. Python’s nursery model is another good way to make concurrency work.

Code Smells

The “code smells” in Bob’s document come across as good advice, language specific advice, outdated advice, and just generally annoying.

His section on comments is fine and expected. You don’t want comments that lie about code, get outdated, repeat the code, or are just badly written.

The section on environments is, well, I agree with him, a lot. If your build tool can’t handle everything in one step on the first go, it’s not really a build tool (I’m looking at you, npm and yarn). If you’ve downloaded a package written in C# but don’t have dotnet installed, that’s not the package’s fault. But once it’s in, there should be one obvious command you issue to build the thing, and that command should not require that you have an IDE up and running to build it. If the command is esoteric, wrap that in a Makefile. The user shouldn’t have to know the full command to build a simple, standalone C# command-line program for production, because that command is:

dotnet publish -c release --self-contained --runtime linux-x64 /p:PublishSingleFile=true /p:PublishTrimmed=true

The general guidelines are necessarily good; a program that doesn’t do the obvious thing implied by its name and placement by default is a bad program. Boundary tests are essential. Code safety is important.

Martin also says, “You shouldn’t have multiple languages in one source file.” Well, define multiple languages: do you have SQL embedded in your source code? LINQ? How about a few regular expressions? Heck, every C# file is four different languages in one file: the preprocessor, the templating language, the C-based semantics at the bottom and, if you really need it, a little in-line assembly language– and this doesn’t even scratch issues with programming peripherals like GPUs, something a systems programming language is supposed to do well. Every compiler in the world violates Martin’s “no multiple languages” rule, intermingling lexing, parsing, and semantic analysis. At the tip of the spear, JSX is popular, intermingling Javascript, CSS and HTML into coherent units of deployable objects that can be assembled like tinkertoys into fairly reasonable UIs for interactions that aren’t terribly performance-bound.

We’ve decided that “multiple languages in a source file” is okay.

There are some good guidelines in there: I especially liked the one on surfacing temporal couplings.

Conclusion

Clean Code is a book of advice, not gospel. The people who treat it as gospel miss the point, and often end up applying the rules either arbitrarily or with too much determination. The book itself is a prime example of inappropriate levels of abstraction, with some chapters sprawling all over the discipline, trying to do too much. Strong advice about naming, function design, system design, and source repository organization is intermingled with extremely language-specific advice about writing in Java; some advice is delivered as if they were rules of law that apply everywhere, even as the programming language community has, in the past 15 years since publication, moved on and wrapped many of the headaches he describes in syntax and semantics that alleviate most of the pain.

Read it and take its advice only the way the Buddha taught: “When you know for yourselves that these qualities are skillful; these qualities are blameless; these qualities are praised by the wise; these qualities, when adopted and carried out, lead to well-being and to happiness – then you should enter and remain in them.”

Robert Martin has a lot of experience. But his wisdom goes only so far. Think for yourself before adopting any programming advice– even mine.

Test everything. Even the advice you’re given.

Comment Form

Subscribe to Feed

Categories

Calendar

May 2020
M T W T F S S
« Jan   Jun »
 123
45678910
11121314151617
18192021222324
25262728293031