7 ms·
The Uncle Bob Martin definition of "clean code" from his book "Clean Code: A Handbook of Agile Software Craftsmanship" is a set of rules that absolutely are not
by onion2k 5y ago
The Uncle Bob Martin definition of "clean code" from his book "Clean Code: A Handbook of Agile Software Craftsmanship" is a set of rules that absolutely are not at odds with one another. If you follow them you will end up with code that's really nice to read and easier to maintain, and, most importantly, that you can confidently change.
There's a decent summary here - https://gist.github.com/wojteklu/73c6914cc446146b8b533c0988cf8d29 https://gist.github.com/wojteklu/73c6914cc446146b8b533c0988c...
- davedx 5y agoYeah I agree with almost all of these. IMO the most important is “keep it simple”. That is always always my coding North Star and it’s served me well over the many years I’ve been programming.
- omegalulw 5y ago+1 when I write code I always think about how my reviewers would perceive it before sending it over and I refactor based on that. You should aim to write code that's easy for others to follow.
- raverbashing 5y ago> is a set of rules that absolutely are not at odds with one another Yes, if you take them as general guidelines, not dogmatic rules with variable interpretations The latter is exactly what his fans do > 2 .Keep it simple stupid > 2. Prefer polymorphism to if/else or switch/case. No, not at odds with one another...
- mpweiher 5y agoHmm...polymorphism is almost invariably simpler than if/else or switch/case, so not seeing the contradiction.
- fortylove 5y agoIs that view not subjective though? Might someone think that if/else is simpler?
- qayxc 5y agoDepends entirely on the metric used. Cyclomatic complexity would put if/else behind polymorphism while using lines of code and depth of inheritance would favour the if/else way. That's the crux with software - there's not always an objectively better solution in terms readability/comprehensibility and some metrics that try to capture these qualities are inherently at odds with each other.
- ajuc 5y agoI've seen this happening a lot: - you have the same if-then-else in 3 places in code with 3 different effects - you refactor it to polimorphism (Enum with a few overloaded methods works well in java) - now one of these places need additional condition so you make it a little more complex - year passes - you have eldritch abomination of OOP instead of 3 if-then-elses with slightly different conditions because at no point someone was brave enough to refactor back into if-then-elses as the special cases piled up
- ziml77 5y agoExactly my experience with polymorphism. It starts out looking super clean but then the business requirements shift and each has its own edge cases that turn it into a giant mess, often ending up as a mix of both polymorphism and convoluted if statements.
- deleted 5y ago[deleted]
- juped 5y agoThey aren't. This is why pattern matching and typeclasses (to use the Haskell words, though I know other words are in use as well) have become increasingly popular features in languages.
- beebmam 5y agoYeah, polymorphism is in no way simple in lots of languages/frameworks. It's actually quite complex in many common languages/frameworks. This is absolutely contradictory in my opinion, but I welcome disagreement :)
- mcv 5y agoAnd then there's a language like Scala where switch/case is so naturally integrated that it actually becomes the cleaner solution and you basically have to use it. Whatever approach is clean or not can vary wildly by language. Or framework. I do like clean code, but I think the article has a point: it's a very vague concept.
- onion2k 5y agoPolymorphism is more complex than a switch/case in a single instance. Across a large codebase using polymorphism instead of hundreds of different switch/cases is simpler, especially when you need to change something. Encapsulating what a class should do within the class instead of in an external code block is much simpler when you're working on something big.
- _0w8t 5y agoPolymorphism is a switch, but it is the switch over object kind, not action. In fact if there are only few subclasses compilers for some languages where a global analysis is possible implement polymorphic calls as switches over the object tag. And notice how if one needs to add a new action, all objects are required to implement it. In typical OOP languages that touches many files even if the action is used only once. So polymorphism works nicely if there are only few actions but many different objects, like in a typical GUI. But in cases of few objects and many actions, for example, doing a lot of different queries over few data sets it leads to a lot of boilerplate. Switch over actions will lead to less code.
- rk06 5y agoPolymorphism can replace multiple if/else/switch/case with a single one. It has benefits, if you do not apply it dogmatically everywhere
- hyperman1 5y agoI see polymorphism vs if/... more as a tradeoff. For example: Suppose the program has to make a choice. When the choice exists only in 1 place, use an if/switch. When the choice exists in multiple places, polymorphism centralizes it and quickly becomes the simple option. For example, a change in law required us to make a choice based on a date: If (thing.someBusinessDate < 2010-06-01) { doTheOldWay();} else{doTheNewWay();} after a while, a few edge cases sprang up, and we got variants of this code hiding in multiple corners. Then the law changed again with a new cutoff date. So we adapted to polymorphism: interface TheWay{ edgecase1(); edgecase2();...} class TheOldWay implements TheWay{...} class TheNewWay implements TheWay{...} class TheNewNewWay implements TheWay{...} TheWay wayDetector(thing){/* all the ifs here */} Unfortunately, adding just one more 'if' was always the cheapest choice. The refactoring was nevertheless clearly a better choice: it enabled us to point out more forgotten edge cases, avoiding costly mistakes.
- viceroyalbean 5y agoI agree with you, especially in instances where that decision logic is unrelated to the data that is actually being used. In your example you'd have to pass "thing" everywhere just for that if-statement, but it might not be used for anything else. That said, I think "simple" if/else or switches are much more common than the complex cases that fit polymorphism, so it seems a bit off to have it as a general rule to prefer polymorphism.
- moonchrome 5y ago>If you follow them you will end up with code that's really nice to read and easier to maintain, and, most importantly, that you can confidently change. I remember going over unclebob github a while back and this wasn't the impression I got. The parts I read seemed like obtuse code that added pointless abstraction for the sake of following some convention.
- Hermitian909 5y agoThis is my experience as well. It takes a long time to grok what the program does and how it does it because of all the layers of indirection, even when the program is fairly simple. While this style might make small changes easy if you understand the code, the inertia to larger changes because of the layers of indirection is immense.
- Rygu 5y agoWow that's on-point, agree that in many past projects "inertia to larger changes" is exactly how I would describe so-called Clean Code. It's that small changes like adding getters and setters give an easy-to-follow PR, a false sense of accomplishment for both the author and the reviewer, and ton of dopamine with that. While important changes, that actually deliver business value or repay technical debt, become increasingly difficult to do without replacing massive amounts of code and redoing the file structure. Inertia is the biggest demotivator.
- jseban 5y agoYeah, you can't eliminate essential complexity by narrowing your view, it will just obscure the big picture.
- dt3ft 5y ago> Don't comment out code, just remove I hate commented out code, but this doesn't work in every situation. While working on a clinical information system, we had frequent "please bring it back" requests that came a few months after things were removed (also by request). Relying on source control is nice, but unless you document every feature/removal or use very extensive commit messages, finding the code that was removed (especially if it was spread over several classes) is easier if you just comment it out and write why it was commented out. The important part here is to add why something was commented out.
- karottenreibe 5y agoIn my experience you absolutely SHOULD document every request and removal in e.g. an issue tracker. It's very valuable information. And commented out code gets stale so quickly. Much better to check out an old version where it still worked than uncommenting a bunch of lines that now have compile errors and have become incomprehensible. Happens so easily, e.g. after some variable renames.
- dtech 5y agoSeems garbage advice to me. Now you've polluted your production code, likely forever. I don't see a situation where using an issue tracker + good commits - e.g. 1 commit for the 1 PR linked to the ticket that removes the feature - won't result in a superior situation from a maintenance and traceability perspective.
- serial_dev 5y agoYes, I agree with you. Another thing is how do you know which of the removed features will be asked to be put back? You can't really know. You can guess, but that will leave you with many classes/functions commented out only in the hope that one day one of them will be requested to be put back. This leads to nobody daring to remove old code for months/years. Then, one day, a new coder on the team just ask WTF is this here, realize it's outdated and not running anymore, and will remove the whole thing. Then, the "senior" guy on the team comes shouting at him/her: "We might need it one day, what do you know about our team, bla bla bla, we had a feature 4 years ago that was removed and we had to add back, so now we have the rule of commenting things out". Yuck, what a nightmare. Just create good commit messages, don't group irrelevant changes together, and you'll be able to find the commit if you really have to. And even if you can't, you know you can still just develop that thing again, right?
- hbbio 5y agoMost items are great but I was unsure about "one assert per test" until I read the comment below: << I would change "one assert per test" to "one concept per test". Uncle Bob makes a fairly big deal about how the former is a misconception. >>
- yakubin 5y ago> If you follow them you will end up with code that's really nice to read and easier to maintain, and, most importantly, that you can confidently change. I found that a lot of those guidelines lead to the exact opposite. Examples: - Prefer polymorphism to if/else or switch/case (oh, the joy of tracing a simple task through 50 files) - Use dependency injection (same as above) - Hide internal structure (that "private" is the default in C++, Java and Rust just adds to boilerplate; the default case is that you want everything public (unless you like writing trivial getters and setters just for the fun of it); legitimate uses of "private" exist, but are rare) - One assert per test (I guess I need to throw my property-based tests out the window)
- rootlocus 5y ago> One assert per test Was meant to make a failed test instantly communicate what's wrong with the unit under test. As frameworks evolve and our practices around them change, it's absolutely fine to come up with new rules. All of uncle Bobs rules come with pages of explanations of what problems they solve. If you don't have those problems, you may not need those solutions. The book is more about the spirit of the law than the letter.
- pydry 5y agoThis makes no sense whatsoever. I've read thousands of tests with multiple asserts that were perfectly plain about what was being tested and in many cases the logical sequence made it easier to understand. Breaking them into separate tests with one assert would simply have meant a shit ton more code to read. Uncle Bob's "rules" often come with mile long caveats that he seems to be blissfully unaware of. I think I get why he said it - overloaded tests are a thing - but he fucked up trying to turn his observation into a rule.
- deleted 5y ago[deleted]
- deleted 5y ago[deleted]
- bobcostas55 5y agoIf you follow them you will end up with an unreadable spaghetti made out of thousands of two-line functions.
- bryanrasmussen 5y ago>If you follow them you will end up with code that's really nice to read and easier to maintain, and, most importantly, that you can confidently change. In Java (or Java like languages [C#]), which is the language they were really written for. There are many languages for which the precepts of Clean Code are not that great a fit. Also - It's probably time to stop recommending Clean Code - https://qntm.org/clean https://qntm.org/clean
- jwdunne 5y agoWas just about to comment with this link. A useful counterpoint whenever Clean Code comes up. I do think this article makes a lot of valid criticisms and it’s well worth a read. I remember being influenced by it early on but then re-read it as I gained more experience. This article sums up what I felt on rereading.
- andrekandre 5y agomaybe its time for someone to write "clean code: the good parts"
- bsza 5y ago> Don't use flag arguments. Split method into several independent methods that can be called from the client without the flag. vs > Needless Repetition.
- lmm 5y agoLook more carefully, there's no contradiction. Splitting a method into several independent methods doesn't mean you repeat yourself.
- bsza 5y agoThe flag check could be deep within a loop, how would you split it then without repeating the whole method?
- asimpletune 5y agoUsing map?
- lmm 5y agoMake the loop operation something that's passed into the method. Or maybe replace the flag with polymorphism if there's some logical value that it really belongs on.
- bsza 5y agoThe second one is impractical because the number of classes you'd need would increase exponentially with each added flag. The first one clashes with > Prevent over-configurability, because you're accepting an infinite range of possible callback methods instead of asking a simple yes/no question. I don't get why this rule even exists, actually. If you take three boolean arguments, that's not okay, but if you wrap them all into a configuration object and pass that instead, that's suddenly... okay?
- lmm 5y ago> the number of classes you'd need would increase exponentially with each added flag. Only if you actually call it exponentially many different ways, which seems unlikely. > I don't get why this rule even exists, actually. If you take three boolean arguments, that's not okay, but if you wrap them all into a configuration object and pass that instead, that's suddenly... okay? Three or four booleans are not something semantically meaningful that you can reason about. Five or six classes representing the cases that actually occur in your program, that's something you can understand. Often passing a callback function is too. Boolean flags fall into a hole in the middle somehow, probably because of the exponentially many possible combinations; they look like a finite set of possibilities, but in practice you almost certainly haven't tested all the cases and a lot of them probably don't make sense.
- throwawayffffas 5y ago> that you can confidently change. There is no secret to this, it's merely tests, no matter how clean my code is I would not feel confident unless I have tested my changes, whether manually or automatically and manually quickly becomes intractable. There is the option of formal verification, but I have found the tooling to be disagreeable with me.
- onion2k 5y agoIf your code is complex you can't ever be sure that you're testing the right things, or that there isn't a codepath that your tests miss. Tests are very useful, but they're not a replacement for simple code.
- otikik 5y agoOn the internet I have very rarely encountered "Clean Code" as code following explicitly the rules listed by Bob Martin in his books. People use it like the OP says - "code that feels good to me in the moment, without being able to explain why". I believe that's because the term Clean can be intuitively defined in opposition of quick and dirty : "I have spent a lot time on it. So it isn't quick and dirty. It's clean."
- asimpletune 5y agoI'm all for the following the spirit of things, but I also want to take a minute to disagree with the "without being able to explain why" part. I have worked with people who were super booked up on Go4 and all the design stuff, and yet they can be fairly insufferable to work with when they parrot design advice from all this stuff, precisely because they can't explain why in every given situation. Then, on the receiving end of things, most of the criticism people who are expressing in this forum wrt to uncle bob, do so about examples where they coming from the perspective of something they understand. I think there is A LOT of value in all of this experience, but it has 0 utility if you don't understand it. In other words, you are much better off not considering how you feel about code at all, and always sticking to doing things because you understand why. (And I'd say 'understand' means in a way that you can explain to someone else who may be skeptical). Context is key... so it's like if you're about to do a live coding demo in front of the board, then yeah, it's ok to comment stuff out - I think that's obvious. Ideally, your outcomes will be much, much better if they're coming from those 'obvious' points of view in any given situation. Any just to be fair, doing something because you're curious, or it's fun, is just a good a reason as any, it just depends on the context. This is why, there are other people who I've worked with, who know all that stuff - I'm talking super senior people - and yet rarely ever cite it or even mention it. Usually, they will ask questions because they're patiently trying to get you (or whomever) to see for yourself the same, all-encompassing 'obvious' reason, that overrides all the other important factors in a given context, that they already see. If you ask them why a piece of code that doesn't 'feel right' to you is the way it is, they'll usually have a story about some situation, where they just had to do it that way, and maybe it's bad now but if you were there, and knew what they knew, you would have probably done the same. So, my point is, yes uncle bob stuff is fairly sound, but no I don't agree that we should follow it if we don't understand it and we're better off almost always sticking to what we do understand. O but also cool analogy about the quick and dirty. I just realized though that if you invert it completely it's like 'slow and clean'.
- GordonS 5y ago> If you follow them you will end up with code that's really nice to read and easier to maintain, and, most importantly, that you can confidently change ...for trivial cases. For more complex, real-world scenarios, these kind of rules need to be applied sparingly, and where needed. Otherwise you get things like too much abstraction and several layers of inheritance, making it difficult to figure out what anything actually does - the opposite of nice to read Uncle Bob et al have some good guidance, but cargo-culting them led to long-lasting, negative effects on some communities, such as Java and C# developers.
- jeremy8883 5y agoBob himself has mentioned that many of the rules are contradictory to one another. I can't remember what examples he gave. I think it was in his video series.