My takeaways from "Clean Code"
medium.com
medium.com
It's been nearly 10 years since the 2nd edition, which sat uncomfortably athwart the tectonic shift to agile practices.
If I had a magic wand there'd be a 3rd edition based on what's been learned since 2004 (and a 2nd edition of Rapid Development, which he is at least considering[2]).
[1] Yes, I know most software engineering research sucks. But a lot of it is better than stuff found in The Journal of Gross Oversimplifications Conveyed in 140 Characters or Less.
- Minimize the number of arguments
- Avoid output arguments
Basically, you're arguing against some principles of functional programming, advocating the use of state. There's no good or wrong with state, I think, there's only a trade-off. Heavy use of state = easier/faster to code, harder to debug/read.
IMHO these kinds of generalizations are pretty dangerous. We should always try to understand the problems lying underneath in order to correctly assess the trade-offs. A quote from Elon Musk that goes something like "don't rely on analogies, try to understand the problem" popped up on HN some other day, but I'm too lazy to go look for it now. :-)
- Comments are fails
Sometimes you rewrite code for performance, making it fast but awful to understand. Comments are very useful in this case.
I've also found that comment tend to be both easier to maintain (due to the smaller amount of code) and more necessary (due to the higher density of information) when using more powerful languages (eg, Haskell).
There’s a lot of good advice in the book, but it is written by and for Java programmers, and it comes with its share of risks if you take its advice. If you’re developing in a dynamically typed OO language like Ruby, it’s almost always bad advice to follow Clean Code. There are performance considerations even with Java to following some of the advice given. With Objective C, the suggestions are just silly. With languages that emphasize some level of functional programming (including Ruby and even C# to a degree), the suggestions are counter-productive and produce code that is unnecessarily obfuscatory.
Yes, it’s good to have short, readable, meaningful methods. Reducing method size too much, though, can result in code that you have to follow through many objects in order to understand the logic. If you follow Martin’s advice enough, you will end up with lots of very short methods on lots of very small objects—and have a hard time understanding just how your program fits together.
If Martin were a better writer, or had expanded his horizons beyond Java when he had written this book, he may have written something profound—still dangerous, but profound. Instead, he wrote Clean Java and either he or his publisher thought it would sell more as Clean Code. My advice is to ignore the book—unless Martin comes out with a second edition that takes what he has learned in the last few years since leaving behind Java and applies it to really make a book that describes clean code.
I have had to try and clean up functions that were two thousand lines long (in Java), had lots of side effects, and took in seven parameters. Oh boy did I wish the developer had read Clean Code.
I agree with your premise that the book is not relevant to those who work in dynamic or functional programming languages. Frankly, I don't think they are not the intended audience. I think it is a little hyperbolic to call it the "one of the most dangerous programming books released in the last decade".
I don't understand how a book that professes to be top-down advice about software craftsmanship as a discipline can be intended only for a specific subset of programmers. Shouldn't choosing a language be as much a part of software craftsmanship as how you use that language?
It's like writing a book entitled "Good Woodworking" which implicitly assumes you'll be using only double-clawed hammers[1]. Wouldn't you expect a book called "Good Woodworking" to start with a chapter on good tools?
[1] http://me.veekun.com/blog/2012/04/09/php-a-fractal-of-bad-de...
It wouldn’t sell as many copies that way, though. Probably.
Generally speaking, the earlier chapters were better, but my advice would echo yours: give it a miss, especially if you're not working in Java.
I have never found this to be the case. Quite the contrary, in fact. Far too many objects that do way to much, or functions that do X and Y.
Indeed, I'm puzzled by your statement:
> Yes, it’s good to have short, readable, meaningful methods. Reducing method size too much...
Which directly contracts this:
> If you follow Martin’s advice
I guess the key word is 'enough', which is really just bad wording. "If you don't follow Martin's advice" is better. The qualifier here is you don't make short methods for the sake of having short methods. You make methods as short as they need to be to do that thing they promise to do.
So then we come to it: either you support having your methods do more than they should, doing more than one thing and having side effects, or you support Martin's suggestions. And yes, it really is that black and white. Sure, you could suggest that following his advice "enough" leads you to nothing but one line methods, but that would be silly.
I distinctly remember arguments with people who were reading Clean Code at the time who really believed that one-line methods really were the ideal.
The trick is knowing what does “doing more than they should” actually mean? If you follow Clean Code as I’ve seen some people follow it (including the Ruby dev shop I mentioned elsecomment), you will end up having substantially larger interfaces at the cost of understanding just what it is that your program is supposed to do in the first place.
I’ve dealt with programs written the Clean Code way (an explosion of methods, classes, and objects), as well as programs written with too much going on in one place (e.g., 2+kloc functions). Neither is fun to deal with, but it’s generally easier to assimilate the basic business purpose of the 2+kloc function than something that has a larger interface surface than it needs to have.
Having recently read Clean Code as a C# dev, I feel the missing piece in this conversation is that the advice of small methods needs to be taken together with the idea that things should do one thing, and should be composed of items at the same level of abstraction. I feel the idea is hard to defend on its own, but with the other ideas quite easily defended.
I'm also curious about the performance problems you've mentioned above.
The problems with Clean Code are legion. It makes axiomatic arguments and then drives them further in the name of purity—which is anathema to shipping code, clean or otherwise. It is deeply rooted in the Java available in 2007–2008 and shows no awareness of just how broken Java was as a programming language at the time in comparison with many other languages. It makes prescriptions of things that are merely good advice.
By all means, simplify your code. That does not mean making lots of little objects and lots of little methods. It means that your code should be as simple as possible and no simpler. Sometimes this means that you’re going to have methods that are a few dozen lines long, because splitting them apart increases the state you need to carry around between those methods. Sometimes it means taking advantage of features in your language that Clean Code (being an Old Java book, despite its grandiose title) exhibits no awareness of (even though, with generics, it could).
It’s been a few years since did this, but to introduce the C# developers at a previous job to the ideas provided by C#’s functional constructs, I wrote a particular piece of code four or five times and wrote a substantial email about it describing the simplifications that the constructs provided at each step of the way, and ended it with the equivalent Ruby code, which was more or less:
return container.any? { |e|
e.subcontainer.any? { |f|
f.boolean and f.subcontainer.any? { |g|
g.boolean
}
}
}
The above code clearly violates the rules of Clean Code; your method at object level D ('container') knows something about objects at level F and G, but making (and naming) methods at levels D, E, F, and G to return truth when you do not need those methods anywhere else? Foolish inconsistency, and Clean Code is full of examples like that where, given a better language (or even a better use of the Java language and generics), you don’t need or want to do those things.
Clean Code is a dangerous book because it takes generally good ideas and applies them as Truth, and many of the people who should be reading and learning from a book like Clean Code simply don’t know any better and fall into even worse habits than if they had never read the book. Given a choice between following complex business logic across lots of little classes and files and a few larger functions, I know which I would choose.
Is that true? "Minimize the number of arguments" nudges you towards datatypes or maps instead of lists of implicitly related variables. No state there, just keeping functions relatively simple in terms of their inputs.
In the latter case, "avoid output arguments" is absolutely a principle of functional programming; it says use arguments for input and return values for output. You can't do this in Haskell. I don't think you can do it in Clojure, either, or if you can, it's not really idiomatic b/c immutable. I can't speak to other Lisps with any certainty, but I've never seen it as an idiom.
Yes, yes! I definitely agree but what I meant is "principles of functional programming outside functional languages". When you understand the trade-offs that these principles bring to the table, you can apply them in OO languages -- in some cases. But here he's banning it for good with that statement!
There is one which explicitly encourages side effects -- the one which says "write bar.doFoo() instead of doFoo(bar)" -- but I think it's orthogonal; you can keep your inputs and outputs simple without rewriting your code to have a lot of state.
"Anyway, it is always a good programming advice to minimize the number of arguments. Zero or one argument is easiest to understand and maintain."
How is one to minimize the number or arguments in a purely functional language with immutable state? How can I do tail recursion without any arguments?
In reality, Uncle Bob's stuff is actually better when taken holistically and when applied together, but it's hard to do and when taken to their logical conclusion, it is so outside the norms of OOP, that people freak out about it not being "proper OOP" as they were taught in university and in mainstream things.
> Have No Side Effects
Can someone explain how you use zero argument functions that doesn't have side effects? I am trying to wrap my head around these two statements.
2. Some functions are queries that don't require an arguments. Eg. getVariable().
jQuery.ajax is a 33 parameter function. No one complains, and it is nice that it doesn't have side effects.
It's when the rules conflict that mere rule-following ends and professional judgement begins.
Basically, as noted legal scholar Captain Barbossa observed, they're less like laws and more like guidelines.
You could certainly argue that a class's fields are a form of input arguments to a method, and I would agree - which is why another good rule of thumb is to not have too many of those either!
Obviously, it's a balance, so none of the rules can be taken without context to the situation - they are all just smells that you should "back away from" if you're finding yourself breaking any given rule too much.
Anyway, in the context of the rest of the post, it looks like object oriented programming is being discussed, in which case I think the implicit 'self' does not count as a parameter.
I definitely agree with you saying its suggestions, not hard rules. Unfortunately the author of the page keeps saying 'always' as if it's hard and fast rules.
Understand why and when first, then use where appropriate.
Isn't that what I said?
storeItem.makeAvailable();
In any case, it's fine for two rules such as "avoid side effects" and "prefer zero- or single-argument functions" to be in tension with one another. I don't buy that zero-or-one idea, though. A function should have as many arguments as it makes sense for it to have.
In jasonlotito's example, he's definitely modifying state.
(Mind you, I don't think side effects should be banned outright or anything like that. But minimizing them is frequently a nice idea.)
I think instead of "side effect" the author should have used the more traditional term of cohesion:
As the article mentions, I agree that it's good to make sure that you're naming conventions and code itself is expressive. Furthermore, if you approach coding with this philosophy you will most likely write much better code the first time around. Still some comments, written in plain (INSERT YOUR NATIVE LANGUAGE HERE), which give an executive summary of the class or method can be useful.
> Avoid output arguments / Have No Side Effects
This can be difficult to do in a language like C. It is convention to pass pointers to arrays, structs, etc and allow the method to change it or operate on it. Also it is normal for a method to adjust a pointer's position. With all the grief this caused me though, I do think that this is great advice for a language like Java, Ruby, etc.
There’s a Ruby shop in Toronto that I interviewed with a couple of years back. As soon as I learned that they considered Clean Code a good book to be learned from, I knew that I didn’t want anything to do with their software development process (and I write very good, clean, and readable software, thank you very much).
One of the things that stands out in my mind is a few related pieces of advice, most of which are fairly good in isolation:
- prefer composition to inheritance
- prefer more smaller methods (as I said elsewhere, I believe that there’s even a point where it’s suggested that a one-line method is “ideal”)
- prefer smaller objects
In isolation, each of these things is fairly good—but when combined, you change from a codebase of large, unreadable functions, to an unreadable codebase of lots of little, tightly-coupled, smaller objects with smaller methods all composed into doing something that would be simpler to understand as a few medium-size objects and medium-size methods.
In Ruby, all of this is…fungible is the best word I can think of for this. Because of Ruby’s duck-typing, mixins, and most importantly, lambdas and blocks, the best place for your logic to reside is as close as possible to where it’s being used. This doesn’t mean write large methods—it means that all of the advice that’s given for Java written before late 2008 (when lambdas were barely a twinkle in the JSR process and Sun still mattered) doesn’t apply when you have anonymous blocks of code that you can apply immediately and functionally.
The best description I’ve got for this ultimately is related to many of the things said about Gamma, et al.’s Design Patterns: in other languages, design patterns are just features that the language gives you, rather than something you have to implement. Iterators? Built into Ruby, and better because of blocks. In Ruby, you don’t need to learn the lessons given to Java programmers in 2008, because the language gives you more power and expressiveness already. Add a little bit of metaprogramming (in the same way that attr_reader/attr_writer/attr_accessor is implemented, for example) and you’ve got programs that express your intent clearly, but are nearly impossible to apply Clean Code to because you’re doing something smarter (not clever, but smart).
To address the three points that I mentioned a couple of paragraphs ago:
- prefer composition to inheritance: good advice, but composition in Ruby also means considering how blocks and mixins can help you understand your code better.
- prefer more smaller methods: I’m not as sold on this; the more methods you have, even if they’re private, the harder it is to understand just how your code works because you now have a larger API to understand. (And, like someone else in this conversation, I’ve had to deal with 2+kloc functions in other languages; I just haven’t had that happen in Ruby, where the largest functions that I’ve dealt with are in the low hundreds—and those are very rare but focussed functions (e.g., a lot of error handling where you don’t necessarily want to introduce state that lives outside of the function).
- prefer smaller objects: As with smaller methods, I’m not as sold on this. The more objects you have, the more interface you have to remember, the harder it is to remember how to compose everything back into the logic you’re trying to implement.
By all means, extract code—DHH’s advice on what they’re calling “concerns” in Rails is very good (I’ve already done that with more than a small bit of our current Rails application; I’ve also done it with other code over time).
Robert ("Uncle Bob") Martin's book has been really useful for us. We have been going through the book section by section and comparing the ideas in "Clean Code" to our implementations. It has been a most effective way of getting the team up to speed.
I would recommend anyone who is getting started on their first software engineering job to go through the book. It is excellent study material for an inexperienced team.
Flag arguments are ugly
Perhaps the only exception is for specific setters that
directly set the value of an object property (flag?)
itself. But I have to agree that flags implicitly mean
that the method is probably doing too much (e.g. there is
no Command Query Separation).
The sheer number of flags can lead to weird issues (what does it mean to open a file for read-only and truncate it? The man page I'm reading leads me to believe it does something other than return an error) and hard to test (I count fifteen flags, which gives 32,768 paths through the code).
Null is perfectly fine in place of an object. The author mentions this saves you debugging time. No, it will cause you pain later because you won't see errors that should happen. Instead they are masked by operating on some dummy object that you don't give a rats ass about.
For modules I write in C, I only have one function that can return NULL---the function that creates a new structure. All other functions that work with that structure assume (backed by an assert()) that the passed in pointer will not be NULL. For the code I write, there is no reason for functions to accept a NULL pointer. And it's less painful that it sounds. I got the idea from _Writing Solid Code_, one of only two books that fundamentally changed how I write code (the other being _Thinking Forth_).
The right way to work around null for scalar variables is to use Maybe/Option, preferably in a language with pattern matching, as opposed to using "magic values" like 0 or the empty string. This means removing null pointer errors by construction.