10 ms·
Finally! I'm glad to hear I'm not the only one. I've gone against 'Clean Code' zealots that end up writing painfully warped abstractions in the effort to adher
by jugg1es 5y ago
Finally! I'm glad to hear I'm not the only one. I've gone against 'Clean Code' zealots that end up writing painfully warped abstractions in the effort to adhere to what is in this book. It's OK to duplicate code in places where the abstractions are far enough apart that the alternative is worse. I've had developers use the 'partial' feature in C# to meet Martin's length restrictions to the point where I have to look through 10-15 files to see the full class.
The examples in this post are excellent examples of the flaws in Martin's absolutism.
- ziml77 5y agoSeriously? That's an abuse of partial and just a way of following the rules without actually following them. That code must have been fun to navigate...
- pdpi 5y agoMany years ago I worked on a project that had a hard “no hard coded values” rule, as requested by the customer. The team routinely wrote the equivalent to const char_a = “a”; And I couldn’t get my manager to understand why this was a problem.
- jandrese 5y agoClearly it is still a hardcoded value! It fails the requirement. Instead there should be a factory that loads a file that reads in the "a" to the variable, nested down under 6 layers of abstraction spread across a dozen files.
- taeric 5y agoSaw this just the other day. I was at a loss to know what to say. :(
- ebiester 5y agoThis gets to intent. What, in the code base, does char_a mean? Is it append_command? Is it used in a..z for all lowercase? Maybe an all_lowercase is needed instead. I know that it's an "A". I don't know why that matters to the codebase. Now, there are cases where it is obvious to even beginners, and I'm fine with it as magic characters, but I've seen cases where people were confused to why 1000 was in a `for(int i = 0; i<1000; i++)`. Is 1000 arbitrary in that case, or is it based on a defunct business requirement in 2008? Will it break if we change it to 5000 because our computers are faster now?
- jugg1es 5y agoCan I please forward your contact info to my developer? Maybe you can do a better job convincing him haha ;)
- SassyGrapefruit 5y agoYou were never alone Juggles. We've been here with you the whole time. I have witnessed more people bend over backwards and do the most insane things in the name of avoiding "Uncle Bob's" baleful stare. It turns out that following "Uncle Sassy's" rules will get you a lot further. 1. Understand your problem fully 2. Understand your constraints fully 3. Understand not just where you are but where you are headed 4. Write code that takes the above 3 into account and make sensible decisions. When something feels wrong ... don't do it. Quality issues are far more often planning, product management, strategic issues than something as easily remedied as the code itself.
- hackinthebochs 5y ago>Write code that takes the above 3 into account and make sensible decisions. When something feels wrong ... don't do it. The problem is that people often need specifics to guide them when they're less experienced. Something that "feels wrong" is usually due to vast experience being incorporated into your subconscious aesthetic judgement. But you can't rely on your subconscious until you've had enough trials to hone your senses. Hard rules can and often are overapplied, but its usually better than the opposite case of someone without good judgement attempting to make unguided judgement calls.
- tarsinge 5y agoYou are right, but also I think the problem discussed in the article is that some of these hard rules are questionable. DRY for example: as a hard rule it leads to overly complex and hard to maintain code because of bad and/or useless abstractions everywhere (as illustrated in TFA). It needs either good experience to sense if they "feel good" like you say, or otherwise proven repetitions to reveal a relevant abstraction.
- blowski 5y ago"How do you develop good software? First, be a good software developer. Then develop some software." The problem with all these lists is that they require a sense of judgement that can only be learnt from experience, never from checklists. That's why Uncle Bob's advice is simultaneously so correct, and yet so dangerous with the wrong fingers on the keyboard.
- ryanmarsh 5y agoI started OOP in '96 and I was never able to wrap my head around the code these "Clean Code" zealots produced. Case in point: Bob Martin's "Video Store" example. My best guess is that clean code, to them, was as little code on the screen as possible, not necessarily "intention revealing code either", instead everything is abstracted until it looks like it does nothing.
- forgetfulness 5y agoSOLID code is a very misleading name for a technique that seems to shred the code into confetti. I personally don't feel all that productive spending like half my time just navigating the code rather than actually reading it, but maybe it's just me.
- zimpenfish 5y ago> I personally don't feel all that productive spending like half my time just navigating the code rather than actually reading it Had this problem at a previous job - main dev before I got there was extremely into the whole "small methods" cult and you'd regularly need to open 5-10 files just to track down what something did. Made life - and the code - a nightmare.
- astrange 5y agoI have had the experience of trying to understand how a feature in a C++ project worked (both Audacity and Aegisub I think) only to find that I actually could not find where anything was implemented, because everything was just a glue that called another piece of glue. Also sat in their IRC channel for months and the lead developer was constantly discussing how he'd refactor it to be cleaner but never seemed to add code that did something.
- mumblemumble 5y agoMy last company was very into Clean Code, to the point where all new hires were expected to do a book club on it. My personal take away was that there were a few good ideas, all horribly mangled. The most painful one I remember was his treatment of the Law of Demeter, which, as I recall, was so shallow that he didn't even really even thoroughly explain what the law was trying to accomplish. (Long story short, bounded contexts don't mean much if you're allowed to ignore the boundaries.) So most everyone who read the book came to earnestly believe that the Law of Demeter is about period-to-semicolon ratios, and proceeded to convert something like val frobnitz = Frobnitz.builder() .withPixieDust() .withMayonnaise() .withTarget(worstTweetEver) .build(); into var frobnitzBuilder = Frobnitz.builder(); frobnitzBuilder = frobnitzBuilder.withPixieDust(); frobnitzBuilder = frobnitzBuilder.withMayonnaise(); frobnitzBuilder = frobnitzBuilder.withTarget(worstTweetEver); val frobnitz = frobnitzBuilder.build(); and somehow convince themselves that doing this was producing tangible business value, and congratulate themselves for substantially improving the long-term maintainability of the code. Meanwhile, violations of the actual Law of Demeter ran rampant. They just had more semicolons.
- in9 5y agoI love how this is clearly a contextual recommendation. I'm not a software developer, but a data scientist. In pandas, to write your manipulations in this chained methods fashing is highly encouraged IMO. It's even called "pandorable" code
- mumblemumble 5y agoTBH, the only context where I've seen people express a strong preference for the non-chained option is under the charismatic influence of Uncle Bob's baleful stare. Otherwise, it seems opinions typically vary between a strong preference for chaining, and rather aggressive feelings of ¯\_(ツ)_/¯
- mywittyname 5y agoThe latter example (without the redundant assignments) is preferred by people who do a lot of line-by-line debugging. While most IDEs allow you to set a breakpoint in the middle of an expression, that's still more complicated and error prone than setting one for a line. I've been on a team that outlawed method chaining specifically because it was more difficult to debug. Even though I'm more of a method-chainer myself, I have taken to writing unchained code when I am working on a larger team. var frobnitzBuilder = Frobnitz.builder(); frobnitzBuilder.withPixieDust(); frobnitzBuilder.withMayonnaise(); frobnitzBuilder.withTarget(worstTweetEver); val frobnitz = frobnitzBuilder.build(); ...is undeniably easier to step-through debug than the chained version.
- arthur_sav 5y agoI've gone through java code where i need to open 15 different files, with one lined pieces of code, just to find out it's a "hello world" class. I like abstraction as much as the next guy but this is closer to obfuscation than abstraction.
- Cthulhu_ 5y agoSpaghetti code is bad, but lasagna code is just as bad IMO.
- deleted 5y ago[deleted]
- gilbetron 5y agoAt a previous company, there was a Clean Code OOP zealot. I heard him discussing with another colleague about the need to split up a function because it was too long (it was 10 lines). I said, from the sidelines, "yes, because nothing enhances readability like splitting a 10 line function into 10, 1-line functions". He didn't realize I was being sarcastic and nodded in agreement that it would be much better that way.
- tomnipotent 5y ago> where I have to look through 10-15 files to see the full class The Magento 2 codebase is a good example of this. It's both well written and horrible at the same time. Everything is so spread out into constituent technical components, that the code loses the "narrative" of what's going on.
- ericmcer 5y agoWhat do you say to convince someone? It’s tricky to review a large carefully abstracted PR that introduces a bunch of new logic and config with something like: “just copy paste lol”
- throwaway8581 5y agoSometimes you really just do need a 500 line function.
- rzanella 5y agoNah mate, you never do. Nor 500 1-liners.
- karmakaze 5y agoYes, it's the first working implementation before good boundaries are not yet known. After a while it becomes familiar and natural conceptual boundaries arise that leads to 'factoring' and shouldn't require 'refactoring' because you prematurely guessed the wrong boundaries. I'm all for the 100-200 line working version--can't say I've had a 500. I did once have a single SQL query that was about 2 full pages pushing the limits of DB2 (needed multiple PTFs just to execute it)--the size was largely from heuristic scope reductions. In the end, it did something in about 3 minutes that had no previous solution.
- throw1234651234 5y agoIn C# and .NET specifically, we find ourselves having a plethora of services when they are "human-readable" and short. A service has 3 "helper" services it calls, which may, in turn have helper services, or worse, depend on a shared repo project. The only solution I have found is to move these helpers into their own project, and mark the helpers as internal. This achieves 2 things: 1. The "sub-services" are not confused as stand-alone and only the "main/parent" service can be called. 2. The "module" can now be deployed independently if micro-services ever become a necessity. I would like feedback on this approach. I do honestly thing files over 100 lines long are unreadable trash, and we have achieved a lot be re-using modular services. We are 1.5 years into a project and our code re-use is sky-rocketing, which allows us to keep errors low. Of course, a lot of dependencies also make testing difficult, but allow easier mocks if there are no globals.
- fastasucan 5y ago>I would like feedback on this approach. I do honestly thing files over 100 lines long are unreadable trash Dunno if this is the feedback you are after, but I would try to not be such an absolutist. There is no reason that a great 100 line long file becomes unreadable trash if you add one line.
- throw1234651234 5y agoI mean feedback on a way to abstract away helper services. As far as file length, I realize that this is subjective, and the 100 lines number is pulled out of thing air, but extremely long files are generally difficult to read and context gets lost.
- username90 5y agoPutting shared context in another file makes it harder to read though. Files should be big enough to represent some reasonable context, for more complicated things that necessary creates a big shared context you want bigger files and simpler things smaller files. A thing that can be perfectly encapsulated in a 1000 line file with a small clean interface is much much better than splitting that up into 20 files 100 lines each calling each other.
- qxmat 5y ago... and here I was thinking I was alone!
- WorldMaker 5y ago> It's OK to duplicate code in places where the abstractions are far enough apart that the alternative is worse. I don't recall where I picked up from, but the best advice I've heard on this is a "Rule of 3". You don't have a "pattern" to abstract until you reach (at least) three duplicates. ("Two is a coincidence, three is pattern. Coincidences happen all the time.") I've found it can be a useful rule of thumb to prevent "premature abstraction" (an understandable relative of "premature optimization"). It is surprising sometimes the things you find out about the abstraction only happen when you reach that third duplicate (variables or control flow decisions, for instance, that seem constants in two places for instance; or a higher level idea of why the code is duplicated that isn't clear from two very far apart points but is clearer when you can "triangulate" what their center is).
- mumblemumble 5y agoI'm coming to think that the rule of three is important within a fairly constrained context, but that other principle is worthwhile when you're working across contexts. For example, when I did work at a microservices shop, I was deeply dissatisfied with the way the shared utility library influenced our code. A lot of what was in there was fairly throw-away and would not have been difficult to copy/paste, even to four or more different locations. And the shared nature of the library meant that any change to it was quite expensive. Technically, maybe, but, more importantly, socially. Any change to some corner of the library needed to be negotiated with every other team that was using that part of the library. The risk of the discussion spiraling away into an interminable series of bikesheddy meetings was always hiding in the shadows. So, if it was possible to leave the library function unchanged and get what you needed with a hack, teams tended to choose the hack. The effects of this phenomenon accumulated, over time, to create quite a mess.
- twic 5y agoAn old senior colleague of mine used to insist that if i added a script to the project, i had to document it on the wiki. So i just didn't add my scripts to the project.
- cerved 5y agoPartial classes is an ugly hack to mix human and machine generated source code. IMHO it should be avoided
- FalconSensei 5y agoI think the problem is not the book itself, but people thinking that all the rules apply to all the code, al the time. A length restriction is interesting because it makes you think if maybe you should spit your function into more than one, as you might be doing too much in one place. Now, if splitting will make things worse, then just don't.
- JustSomeNobody 5y agoThere seems to be a lot of overlap between the Clean Coders and the Neo Coders [0]. I wish we could get rid of both. [0] People who strive for "The One" architecture that will allow any change no matter what. Seriously, abstraction out the wazoo! Honestly. If you're getting data from a bar code scanner and you think, "we should handle the case where we get data from a hot air balloon!" because ... what if?, you should retire.
- buescher 5y agoI like to say "the machine that does everything, does nothing".
- littlecranky67 5y agoThe problem is that `partial` in C# should never even have been considered as a "solution" to write small, maintainable classes. AFAIK partial was introduced for code-behind files, not to structure human written code. Anyways, you are not alone with that experience - a common mistake I see, no matter what language or framework, is that people fall for the fallacy "separation into files" is the same as "separation of concerns".
- nobodyandproud 5y ago> It's OK to duplicate code in places where the abstractions are far enough apart that the alternative is worse. Something I’ve mentioned to my direct reports during code reviews: Sometimes, code is duplicated because it just so happens to do something similar. However, these are independent widgets and changes to one should not affect the other; in other words, not suitable for abstraction. This type of reasoning requires understanding the problem domain (i.e., use-case and the business functionality ).
- ozim 5y agoPeople to people dealt this fate ... What is mostly surprising I find most of developers are trying to obey the "rules". Code containing even minuscule duplication must be DRYied, everyone agrees that code must be clean and professional. Yet it is never enough, bugs are showing up and stuff that was written by others is always bad. I start thinking that 'Uncle Bob' and 'Clean code' zealots are actually harmful, because it prevents people from taking two steps back and thinking about what they are doing. Making microservices/components/classes/functions that end up never reused and making DRY holy grail. Personally I am YAGNI > DRY and a lot of times you are not going to need small functions or magic abstractions.
- goto11 5y ago> I've had developers use the 'partial' feature in C# to meet Martin's length restrictions That is not the fault of this book or any book. The problem is people treating the guidelines as rituals instead of understanding their purpose.