[HN Gopher] Goodbye, Clean Code (2020)
___________________________________________________________________
Goodbye, Clean Code (2020)
Author : DeathArrow
Score : 246 points
Date : 2021-11-16 12:44 UTC (10 hours ago)
(HTM) web link (overreacted.io)
(TXT) w3m dump (overreacted.io)
| regnull wrote:
| Having duplication does not mean your code is not clean.
| Sometimes, having duplication is actually cleaner, and easier to
| understand. Removing duplication may introduce complexities, and
| force developers to untangle the additional logic that was
| introduced to remove dupes. Good duplication means code just
| happened to be the same in a few places (but it can be
| potentially different). It's fine and easy to read. On the other
| hand, you may have code that MUST be the same in a few places,
| and updating it in one place but not others would break stuff.
| Those dupes must be removed.
| ThaJay wrote:
| >just happened to be the same >MUST be the same
|
| This difference is something important to stress.
| whakim wrote:
| IME abstractions generally begin with the latter case. Perhaps
| there's one or two corner cases that the abstraction also
| covers, but it seems justifiable at the time because the core
| of the code really ought to work the same across all cases.
| Then you slowly start adding corner cases, or you change your
| new feature slightly so that the abstraction has to change.
| Little by little you end up with this insanely complicated (and
| unclean) abstraction which is arguably significantly worse than
| simply duplicating the code would have been in the first place.
| I make this remark because I think that "the code needs to be
| the same in a few places" may not be a sufficiently strong
| reason for writing an abstraction right away. Even in these
| cases, sometimes it's ok to leave duplication in your codebase
| for a little while while you let the feature you're working on
| shake itself out, then come back and see if the abstraction is
| worth writing. That's the only way to break the cycle.
| makeitdouble wrote:
| An issue in the duplicated approach is keeping track of the
| different blocks once they diverge enough.
|
| In the best case scenario they all slighty change over time
| without forcing complexity on each other. But it becomes a
| problem as changes that should be breaking only break part of
| the code, and the rest can go unfixed as nobody remembers all
| the linked bits.
|
| It would be critical for instance if the duplicated bits were
| involved in invoicing procedures, and one in five remained
| unchanged while the other got updated.
| whakim wrote:
| I'm not necessarily claiming that all abstractions are bad.
| I'm say that (as a general rule) engineers should be more
| willing to duplicate code.
| dragonwriter wrote:
| > and force developers to untangle the additional logic that
| was introduced to remove dupes.
|
| Additional logic is only needed when code is _almost_
| duplicated, and that is where the question arises of whether
| what is happening is best viewed as a different "version" of
| some common process, or a different basic process that has
| similarities.
|
| Actual duplication doesn't require additional logic to remove
| duplication; and even near dupes often don't require distinct
| (i.e., branching) logic if the language offers the right
| abstraction facilities.
| agumonkey wrote:
| Would you go with a simple comment/tag to denote both piece of
| code are identical but not abstracted away on purpose ?
| Cthulhu_ wrote:
| Some IDE's and external tools have duplication detection; I
| mean it probably won't be enough for people to go 'if I fix
| it here, I should fix it there' if it applies to both
| locations, but still.
| djbusby wrote:
| I do this
|
| // @note this code is duped in ../../some/other/file.code
| agumonkey wrote:
| I need to write an emacs helper to duplicate and tag in one
| go.
| datavirtue wrote:
| It's not duplication unless the intent is exactly the same
| between the disparate code fragments.
| zugi wrote:
| _When we don't feel confident in our code, it is tempting to
| attach our sense of self-worth and professional pride to
| something that can be measured._
|
| I don't think assuming people who disagree with you lack
| confidence and are "compensating" is an effective way to reach an
| audience.
|
| _Rewriting your teammate's code_
|
| There should be no such thing as "your teammates code", there is
| only your team's code. If the changes were improvements they
| should be welcomed by the team, if they worsen the code they
| should be unwelcomed - independent of who first authored the
| lines.
|
| There is absolutely judgement involved in refactoring, code
| replication, and choosing the right abstraction. But this article
| doesn't offer much wisdom that helps with those decisions.
| OneTimePetes wrote:
| To be honest, i found these values touted, to be the signal of
| the worst kind of subjective code, leading to endless "battles"
| between subjective "better" code styles and bitter endless "but
| mine is more logic"-debates where the subjectiveness of ones
| opinion is not even perceived.
|
| I had programers on lousy toolchains argue for the 9000
| codeline one-file copy paste monolith, because it was
| "objectively" easier to debug.
|
| There is always a teammates code, who will be different.
| treebot wrote:
| You should 100% talk to the original author before rewriting
| though. Not doing so communicates that you think you know
| better than them, so much better than them that you don't even
| need to talk to them to understand why they wrote it the way
| they did, and the tradeoffs. This is very arrogant and would
| most likely bother the original developer, which is bad for
| team dynamics.
| muzani wrote:
| I disagree. Most of the time I'm intrigued by the changes
| someone makes to my code. It's like a musician getting their
| song covered. There's a creative interpretation, often
| something I didn't see.
|
| This one was a bad example. But past 5 or so years of
| experience, when someone has experience with other languages
| and coding styles, it's more interesting and I often learn a
| lot from them.
| zugi wrote:
| Maybe we're envisioning different scenarios here. If someone
| just committed code yesterday and is still working on it,
| absolutely discuss with them first. I'm imagining a scenario
| where someone comes across duplicated code months or years
| later. What if the original author is gone? Code needs to be
| clear enough to understand after the original author is gone.
|
| Change for change's sake may be bad for team dynamics. But if
| the change is one that everyone agrees is for the better, no
| one should be offended by the improvement.
| jamil7 wrote:
| I mostly agree with you but I've found that in certain
| (smaller) teams with a high level of trust and experience it
| can be fine to do this without a heads up.
| burnished wrote:
| You might be projecting. That read like the authors personal
| experience, one that you might reasonably suspect others have
| shared.
| lgleason wrote:
| There are always judgement calls to be made. Some of this is
| throwing out the baby with the bathwater. You don't want to be so
| rigid that you can't violate the rules for good reason. Also,
| refactoring someones code is not necessarily a bad thing per se,
| though it is better to come up with a style that everyone agrees
| with.
| pelasaco wrote:
| I'm shocked that from all discussion nobody noted the root of all
| evil:
|
| "It was already late at night (I got carried away). I checked in
| my refactoring to master and went to bed, proud of how I
| untangled my colleague's messy code."
|
| No PR, no code review, no CI. Just a cowboy pushing to the
| master..
| dataviz1000 wrote:
| This guy was working at Facebook at the time too.
| ARandomerDude wrote:
| Seems fine then. Move fast and break things.
| rapsacnz wrote:
| Huh, don't you mean Move fast and break society
| drclau wrote:
| And... "it was already _late at night_" (emphasis mine). I
| learned a long time ago that not all hours in a day are created
| equal.
| gchamonlive wrote:
| And using clean code wrong. Clean code is not a panacea. If
| people use it as such and without the correct agile rites, they
| should do it at their own peril, but blaming it on clean code
| is at the very least naive (and somewhat dishonest)
| sombremesa wrote:
| Personally, I disagree that DRY is the same as "clean". Clean
| code is merely code that has been thoughtfully structured
| (regardless of repetition) and is relatively transparent as
| to its purpose.
| tcbasche wrote:
| I've seen a few developers see multiple "duplicated" API
| formats for instance (for different API's) in the same
| codebase and think those are "duplicated" and then entangle
| two completely separate API's together. This is worse IMO
| bluGill wrote:
| There is accidental duplication to beware of. That is
| when different things looks the same. Then there is real
| duplication.. Only the later is bad and should be
| eliminated. Telling the differences is hard
| cma wrote:
| I'd rather look at a simple arithmaticl expression and
| know immediately what is going on, rather than have to
| make up a hard to remember name for every small
| expression that happens to get repeated one or more times
| in the name of DRY. The same applies to small bits of
| non-arithmatical code.
| herval wrote:
| ...and getting told by a manager to revert code. The entire
| environment in the example doesn't sound very healthy
| postalrat wrote:
| It was reviewed the next day by his boss.
|
| How is pushing to master not a form of CI?
| donatj wrote:
| I was going to downvote you, but then you made me think -
| everyone pushing to a shared branch _is_ the _most
| continuous_ form of continuous integration I suppose.
|
| I worked this way for years before we moved into full
| branched code review. I would never go back however.
|
| That said, you should still have at least some review and
| automated testing before your code gets in everyone else's
| way.
| deeviant wrote:
| Likely, it was their coworker who saw the changes, then
| reported the incident to the manager, rather than some sort
| of manager picking through every commit type process.
| the_arun wrote:
| > It was reviewed the next day by his boss.
|
| Assuming Master is the "protected branch" and not kitchen
| sink, this sounds like - "we test only in production".
| Wouldn't or shouldn't reviews happen before merging to
| master?
| tcbasche wrote:
| > No PR, no code review, no CI. Just a cowboy pushing to the
| master..
|
| Let's be clear that the dangers here are "no code review" and
| "no CI".
|
| The others are fine in most circumstances, unless you work on
| some huge monolithic codebase.
|
| Smaller cross-functional teams should be able to push to master
| to solve problems then and there without everyone's unanimous
| approval, and everyone should feel enough safety to be able to
| make changes to the codebase they share with their team.
|
| The _real_ "root of all evil" is why is someone working against
| the team from within the team?
| LandR wrote:
| This is just how some places work, unfortunately.
|
| We all push to main, no code reviews, no CI etc.
|
| Hell, some people here patch things by downloading the live
| dll, decompile, update code, recompile and stick it back to
| live...
| benjiweber wrote:
| You realise that everyone pushing changes to main several
| times a day is the definition of CI? and it's pretty much
| impossible to do with code review as a gate for pushing to
| main. Interesting how CI has become semantically diffused to
| mean literally the opposite of CI: tooling and processes that
| delay integration and make it easier to work in isolation.
|
| https://wiki.c2.com/?ContinuousIntegration " The most
| granular unit of integration should be one step of one
| refactor. "
| 5e92cb50239222b wrote:
| CI is more than that, at least in my understanding of the
| term. You have to have some sort of automated testing
| before doing the merge. We were doing the same shit (all
| pushing to the same branch) and it broke constantly before
| we introduced some tests and merges only after a successful
| test run on a separate CI server. Just pushing everything
| into the main branch without any checks is no CI, even if
| it technically can be called so.
| CaveTech wrote:
| That's `automated CI` which is now or less interchanged
| with just `CI`, however at face value there is no
| requirement that all CI be automated, it is just usually
| better to do so.
| Jtsummers wrote:
| For anything non-trivial (read: more than around 10-20k
| SLOC of C or equivalent; straightforward with minimal
| branching logic; non-critical) you cannot, in any
| practical sense, have CI without automation. Unless, of
| course, you want a garbage system. If you do the minimum
| things necessary to ensure the system is correct (in both
| verification and validation senses) and of decent
| quality, and you have no automation, you will _not_ have
| CI because you will introduce delays between code change
| and deployment ranging from days (for smaller projects)
| to weeks or months (for significant projects). That delay
| means you do not have CI. And it also encourages batching
| many changes together so that you only have to have one
| test run (or a small number of test runs) which is also
| the opposite of CI.
|
| Now, you could bypass all those tests and reviews and
| just deploy it anyways. And then you'll have a shitty
| codebase, but you'll have CI so that's good, right?
| CaveTech wrote:
| I'm not advocating for this from an engineering
| perspective, just stating that it does meet the basis of
| `CI`, regardless of whether you believe it meets _your_
| threshold of "good" CI.
| prerok wrote:
| It's weird that the author agrees with their sentiment. In my
| opinion, duplicating code is never the right answer.
|
| Using a different form of deduplicating would be the solution
| here. Just write a few functions that do the math and then call
| them, rather than create new abstractions that can later become
| monstrosities. I think they learned the wrong lesson here.
|
| Just my two cents :)
| cma wrote:
| > duplicating code is never the right answer.
|
| Say you have (a*b+c)*2.0f+d several places in your code with
| potentially different a b c d variables. Are you really going
| to make up a new name for it as a function? It's much easier to
| just read the expression to know what it does than to memorize
| a new name for every possible small occasionally duplicated
| expression.
| pezzana wrote:
| > My boss invited me for a one-on-one chat where they politely
| asked me to revert my change. I was aghast. The old code was a
| mess, and mine was clean!
|
| Not a single word about what the boss objected to. Reading
| between the lines, I suspect the author still may not know.
| That's the problem, not Clean Code.
| skeeter2020 wrote:
| The real miss here is that you had a chance to share, teach and
| learn with your coworker, and accomplish your original intent but
| instead you unintentionally focused on an individual producing
| code at a higher level than their peers. I'm continually suprised
| at how absolutely brilliant developers fixate on being 10x
| individuals, a (perhaps impossible or at least) very challenging
| goal rather than make everyone on their team 10% better, a
| relatively easy task.
| etothepii wrote:
| The author has took the wrong lesson from this experience. The
| issue was surely almost exclusively that done first year coder
| decided to change someone else's code without talking to them
| first. I don't think it would have mattered if the committed code
| was literally the worst code ever written (providing it wasn't
| going to delete the internet). The author refers to this but by
| calling the article "good bye clean code" they have completely
| missed the point.
| charles_f wrote:
| > My code traded the ability to change requirements for reduced
| duplication, and it was not a good trade. For example, we later
| needed many special cases and behaviors for different handles on
| different shapes. My abstraction would have to become several
| times more convoluted to afford that, whereas with the original
| "messy" version such changes stayed easy as cake
|
| Yeah code duplication is fine when the commonality of code is
| incidental. The thing that bothers me is starting from the
| assumption that the requirements for each shapes are going to
| diverge. It looks like the wrong kind of premature optimization
| to me. In this case the refactoring seemed sensible, and unless
| they already _knew_ that this would happen soon, I think it was
| safe to assume that it wouldn't and that factorizing made sense.
| _Then_ duplicate the code in the future if it's necessary -
| seeing that the abstractions were not working properly.
|
| There are tons of codebases in the wild with large portions of
| duplicated logic, from experience they seem the rule rather than
| the exception for any sizeable project. "DRY" as itself is not a
| rule, but I think it makes sense to be doubtful of duplication as
| a default stance.
| [deleted]
| nowherebeen wrote:
| The problem with clean code and avoiding duplication is that you
| will eventually create a God class where everything inherits
| from. By then you will realize a lot of bugs stem from having a
| one size fits all God class.
| jihadjihad wrote:
| There was a discussion a couple of years ago on this article with
| 500+ comments, for those interested:
| https://news.ycombinator.com/item?id=22022466
| swader999 wrote:
| No thanks, we're doing it all over again here. This time will
| be better!
| [deleted]
| datavirtue wrote:
| The mantra of every disasterous rewrite.
| bryanrasmussen wrote:
| in the old version the highest rated comment was 19 lines
| split over 7 paragraphs and contained internal lists! The
| highest rated comment at this time on our version is only 11
| lines over 4 paragraphs! A win for our side already!!
| emaginniss wrote:
| I think it will be better if we redo it in another language:
|
| Acest articol este interesant
| [deleted]
| marginalia_nu wrote:
| As a bit of a tangent.
|
| I've been a bit of an apologist.
|
| I thought maybe people just took clean code a bit too far, that
| people were being too dogmatic about what was basically sound
| advice. Then I found Uncle Bob's old website the other night and
| I can no longer deny he's had a finger in this.
|
| > _Anything not testable is useless._
|
| > Here's the thing. If you can't test it, you don't know that it
| works. If you don't know that it works, then it's useless. If you
| have a requirement and cannot prove that you have met it, then it
| is not a requirement.
|
| - http://www.butunclebob.com/ArticleS.UncleBob.AgilePeopleStil...
|
| If these are the vibes you're putting out, your followers are
| going to be zealots. They're going to be frothing at the mouth
| whenever someone proposes alternative paradigms.
| Dudeman112 wrote:
| And yet, at least for the quoted part, he is right.
|
| It requires some _massive_ balls to deploy code to production
| while not even seeing if it works for any case.
|
| If the answer to "how do you know it works?" is "trust my
| judgement bro" then I will not, in fact, trust their judgement.
|
| I do agree he has a finger in the zealotry and he should
| probably have modulated how he expressed himself.
| marginalia_nu wrote:
| You know, they sent people to the moon using software written
| before the unit test paradigm.
| rootusrootus wrote:
| Way, way back in the day, one of the best applications I've
| ever written had no tests. That application is probably in
| use today, in fact, even though its life is measured in
| decades. My takeaway is that tests are important, but good
| code is still more important.
| marginalia_nu wrote:
| What I really wanted to say is that there is more than
| one way of knowing whether code works. Unit testing is
| one paradigm.
|
| Just because Uncle Bob, seller of unit testing software
| and TDD-books will hear of no other doesn't mean that
| this is the One True Way.
| Dudeman112 wrote:
| There's more to testing than unit testing.
|
| I haven't actually read his site, I'm going by what was
| quoted.
|
| Might be that he meant "unit tested" when he said "tested",
| I guess.
| [deleted]
| UglyToad wrote:
| This seems close to illustrating a thought I've been toying with
| turning into a blog post but need some more examples of before I
| do:
|
| "Abstract over data, not behaviour"
|
| This seems to fall into the trap of abstracting over behaviour.
|
| The best other example I've thought of for this is the 'generic
| repository pattern' common in C# and I'd guess Java. Just because
| CRUD on types is all similar behaviour you can't really build a
| generic/abstracted way of doing it, because at some point you
| need different behaviour in some update/insert depending on how
| the domain concept should act and then you're in a world of pain
| with a crappy abstraction.
|
| One's code actually 'doing something' isn't a sign of unclean
| code, it's why one writes it. Stop trying to build abstractions
| that simplify the doing of what your code does.
| TehShrike wrote:
| Absolutely - I was sad that the post didn't point out the
| better (correct?) way to clean up that code:
|
| leave the objects alone, but write pure helper functions that
| implement the most common expressions inside the mathy parts,
| and have all the similar-looking objects use those helper
| functions.
| thegeomaster wrote:
| Ah, thank you. I never managed to put it quite clearly as that.
|
| But thinking back and considering the intuition I built around
| when an abstraction makes sense and when it's just going to be
| unhelpful cruft, boils down to this.
|
| Abstracting over behavior gets messy very quickly. Special
| cases will probably arise on the next requirement change.
| Unless they're already there and you missed the subtle
| interaction. And if you didn't, and actually handled that
| properly, your abstraction will have a lot of hooks and bells
| and whistles to support the different behaviors, and it'll just
| be hell to maintain.
|
| Data can change underneath you, sure, but I think we as humans
| have a much better intuition about concrete things rather than
| algorithms, so it's easier to abstract data in a useful way.
| Which is why I think that works better.
| ThaJay wrote:
| The abstraction is the standard case and should invite custom
| behaviour to be added dynamically. This extra behaviour
| should be added by the caller / user of the abstraction.
|
| Especially in a CRUD situation, every custom procedure will
| probably have a few basic steps that stay the same. All you
| usually need is a pre and a post hook.
| thegeomaster wrote:
| See, but that's exactly what I'm saying does not work.
|
| It's tempting to think that for e.g. a CRUD situation, pre-
| and post-hooks are all you need.
|
| And it may be that way at the beginning, although hardly
| so, except for the simplest applications. But _very soon_ ,
| you start to run into things like (not an exhaustive list,
| but all are things I actually encountered while trying to
| do precisely what you're saying, many years ago):
|
| * Transactions, when multiple things have to happen
| atomically. Your pre-hook must start a transaction, and
| your post-hook must commit it. But what if there's an error
| somewhere? Python and JS don't have RAII, so you need some
| kind of catch block to abort the transaction. Where does
| that happen?
|
| * Tricky validation, e.g. needing to do queries against the
| data store to check the request is well-formed. So if
| you're using an event-based language, your pre-hook also
| needs to be asynchronous.
|
| * Data transformations (what the user sends will hardly be
| what needs to end up in your data store), so your pre-hook
| needs to be able to return a new object.
|
| * Tracing across service calls, so you need to pass some
| kind of request ID as well to your hooks.
|
| * An "update" request needs to return something (e.g. an
| ID) to avoid another round trip. But sometimes it needs to
| return more stuff, so your post hook must be able to return
| data. Oh, but your ORM or DB usually returns the created
| object, so you'd like to use that instead of re-fetching in
| your post-hook, so now your post-hook must accept that as
| well.
|
| These just keep coming up. The first three in particular
| are usually guaranteed to happen before the first release
| of the product, since requirements always change. Soon you
| have an unmaintainable monstrosity. A little copy-paste is
| tame in comparison.
| UglyToad wrote:
| Exactly right, transactions being the killer/most common
| one.
|
| And the proposed solutions of hooks, AOP etc all start
| down the road of scattering actual logic across files and
| methods making the code harder to reason about which
| seems to be the worst part of overeager abstraction.
|
| The library Automapper from C# is another place I run
| into this a lot. At its core it simply maps from domain
| to dto properties but you soon end up needing awful
| unwieldy configs and magic. I'd rather have a few hundred
| lines of obvious mapping than ever work with automapper
| again.
| ptx wrote:
| Some of those could perhaps be solved by an "around"
| hook, in addition to pre- and post-hooks, like advice[1]
| in Emacs. Although in OOP maybe that would be represented
| by a decorator class (or a subclass) that overrides the
| method and calls the original method?
|
| [1] https://www.gnu.org/software/emacs/manual/html_node/e
| lisp/Ad...
| datavirtue wrote:
| In this case the abstraction should be designed so as to
| invite/welcome custom functionality. The domain/entity repo
| should extend a generic crud repo--as they usually do in my
| experience.
| tarkin2 wrote:
| A good engineer uses Object Orientated Design/Design
| Patterns/Inversion of Control/Functional Types/Algebraic
| Types/Clean Code/New Buzzword Coming Off The Tech Press Soon. All
| of these ideas have good value but the tech industry seems to
| lack moderation.
|
| The buzzwords are like catnip to most developers. They go
| absolutely crazy. Their fevered madness, detailed on medium.com.
| Their codebase, transformed. Conference tickets, bought. Once
| they've weaned themselves off one then another buzzword will come
| along and books will be bought and blog posts will be written and
| code will be refactored and job specs will be altered.
|
| I live by "when you have a hammer, everything looks like a nail"
| now and try to stay wary of this year's transformative codebase
| panacea.
| charlieyu1 wrote:
| As an amateur programmer I do feel the same about my LaTeX
| projects. I would try to DRY my code into reuseable chunks, but
| eventually every use case is slightly different, and LaTeX is a
| terrible language to work with if you want refactoring
| harpiaharpyja wrote:
| Duplicated code != Messy code.
|
| The only time duplicated code is bad is when there is an actual
| _logical requirement_ for the multiple instances of duplicated
| code to be the same. Like some actually underlying concept
| linking the duplicated code sections that is worth abstracting.
|
| That is not always the case. Just as often IME the situation is
| actually, "these two things happen to have the exact same
| behaviour right now." It's important to recognize that there may
| be no guarantee that will be the case *in the future*.
|
| If the _reason_ for the duplicated code is just coincidence, let
| the code be duplicated. You 'll give the duplicated code sections
| freedom to drift apart naturally as requirements change and save
| yourself the trouble of having to decouple things later.
| georgeecollins wrote:
| >> If the reason for the duplicated code is just coincidence,
|
| Totally agree! However, if code is duplicated because there is
| some is some assumption of functionality that is represented in
| multiple places then it gets more iffy. I took this article to
| imply that in each function implementation there were something
| like similar calls to a library or particular orders of
| redrawing based on the way the code base worked. That's where
| it can get dangerous.
|
| It's not as dangerous if there are four very similar functions
| sitting right next to each other. But if there is a block of
| code that implements a way of working and another block of code
| has the same implementation in a place that isn't as easy to
| find that seems dangerous to me. If you want to make one change
| to a code base ideally you make the change in one place.
| kevincox wrote:
| I don't think this is necessarily true. If I have one place in
| my code where I sign unsubscribe tokens for emails and another
| place where I sign cookies for auth I don't want to implement a
| HMAC twice. Even if the verification paths are different so the
| algorithm doesn't actually need to be the same it is still
| better to use the same code for both so that any bugs,
| inefficiencies or other problems can be fixed once rather than
| up to twice.
|
| I think the true decision function lies somewhere in the
| middle. For relatively simple code that doesn't need to be in
| sync then yes, duplication is likely the best option. For
| complex code or code that should stay in sync then extracting
| the common code is likely best. But there is a huge range of
| code in the middle where it is a judgement call with no
| obviously correct answer.
| kayodelycaon wrote:
| Even if you have a number of different hashing functions,
| each used one, throughout the code, it would make sense to
| place all of the hashing functions into namespace next to
| each other. That way you can reuse if needed and checking for
| security problems is easier. (It can also make the code more
| readable because the details of the hashing are abstracted
| behind a method.)
| harpiaharpyja wrote:
| Yeah, you're right and I think we agree here. I should have
| been clearer about that.
| HelloNurse wrote:
| Things that are coincidentially similar are much "larger" and
| more trivial than something that becomes easier and simpler
| if it is more abstract like a HMAC algorithm; for example
| accounting applications would care about records that need
| VAT and records that reference a counterpart, which are
| approximately 90% the same and the other 90% special cases
| with business logic organized according to personal taste.
| paiute wrote:
| "let it go"! Everyone senior, lead, etc needs to understand this
| if they want a productive team.
| DeathArrow wrote:
| Uncle Bob fooled many developers to join his sect. I am much
| happier since I've discovered that there's not only "one true
| path" and OOP, SOLID and Clean Code are not the means to
| everything in software.
| extrememacaroni wrote:
| He makes money whenever he convinces someone he's right, of
| course he's convinced many to join his sect.
| yuvalr1 wrote:
| This post is not related to Uncle Bob's book, which is not even
| mentioned. The term "clean" here is subjectively interpreted by
| the writer of the post.
| jimbob45 wrote:
| It's ambiguous really. Clean architecture refers to
| architecture that is structured in such a way (mostly by
| relying on tests) that one can "clean" out crufty portions of
| the code and not feel anxiety because one's tests should give
| one peace of mind. Whether or not the author was referring to
| the clean code as part of a larger clean architecture or just
| that the code wasn't crufty in general isn't clear. I would
| have preferred that the author used a different term (or that
| Uncle Bob had chosen something less common himself).
| zoomablemind wrote:
| Is it really about Clean Code? It seems to me that a big chunk of
| team interactions was missing!
|
| Firstly, was there any kind of review or just a chance for team
| members to talk about the original implementation before it got
| onto master? After the fact?
|
| Second, Ok, the master commits seem liberal, but why then the
| boss goes the one-on-one way to make a team-member purge the
| commit? It seem like a wasted chance at regreasing the team
| dynamic, let developers find a way to forge the code together.
|
| There should be an open channel between devs and also a way for
| them to express themselves by means of code... without fear of
| blame or dangers to the mainline. 'Branches are cheap', isn't it?
| Personal-branch it so it could later be showcased to others, to
| the boss?
|
| IMO, this is more about Open Team, than Clean Code.
| danlugo92 wrote:
| The boss was right, he removed duplication but he did NOT make
| the code simpler.
|
| Sometimes duplicate code is simpler.
| pydry wrote:
| This blog is essentially a journey of self discovery arriving at
| https://sandimetz.com/blog/2016/1/20/the-wrong-abstraction
|
| "duplication is far cheaper than the wrong abstraction."
| bcrosby95 wrote:
| > The moral of this story? Don't get trapped by the sunk cost
| fallacy.
|
| Most engineers seem to have the wrong takeaway from this post.
| Every abstraction goes from right to wrong eventually. The
| answer is not to not abstract, the answer is to ruthlessly tear
| down abstractions when they go from right to wrong, which
| people seem to have trouble doing.
|
| I think the other major problem with abstractions is that they
| have a cost and no one seems to like to discuss that. Even the
| right abstraction has a cost.
| abledon wrote:
| I've seen this site cripple teams
| https://sourcemaking.com/design_patterns
|
| "hey we need feature X" (feature X is 2 lines of code).
|
| "Ok, I've made 4 layers of abstraction, and 2 interfaces! an
| extra abstract class!"
| VBprogrammer wrote:
| The thing I hate most is when some creates the "new way of
| doing things". Then fails to convert most of the existing code
| which is "doing things the old way" even when it's clear that
| it is the same thing.
| Cthulhu_ wrote:
| The pitfall there - and I myself am guilty of this - is that
| you only apply it halfway, so instead of fixing the problem
| (if there even is one), you're left with two different
| approaches to the same problem, meaning you added a second
| problem as well as a lot of overhead because you're not
| maintaining two different solutions.
| emaginniss wrote:
| That's essentially the lava flow anti-pattern. You have
| remnants of old architect/tools/philosophy left in the old
| code while the new code is being built with the new hotness.
| Invariably everyone has to learn the old stuff to maintain
| the system as well as learn the new stuff to keep building.
| swader999 wrote:
| Which then devolves into five ways of doing things after a
| few more iterations. Kitchen sink architecture.
| danuker wrote:
| The "new way of doing things" is risky and might not turn out
| so good after all.
|
| If you treat the new way as an experiment, and only gradually
| convert the rest of the code, you're in for a smoother ride.
| Spivak wrote:
| Please no, this is how you end up with 20 different
| experiments and zero consistency in everything. Gradual
| migrations are fine because it makes project managers happy
| but it requires a _commitment_ to actually follow through
| with it. If there 's a chance that you won't actually have
| dev time to finish the migration then just don't do it. It
| can't be that important then.
|
| It's totally fine to try out an experiment in a branch and
| deploy to QA but don't deploy it to prod until you're sure.
| CTmystery wrote:
| > Please no, this is how you end up with 20 different
| experiments and zero consistency in everything.
|
| This comes from a lack of discipline in the org, not from
| the parent's (IMO) correct guidance that an org should
| undertake architectural changes with pilots in isolated
| parts of the code base.
| [deleted]
| tpfour wrote:
| The typical SE path I'm familiar with looks like:
|
| 1. Write _anything_ that works. Dependencies, code quality
| don't matter. Code is idiosyncratic but can be understood by a
| reviewer.
|
| 2. Apply abstractions _everywhere_. Re-implement data
| structures and algorithms (maybe unknowingly). Code is now very
| hard to understand.
|
| 3. Figure out one is completely unable to update or even
| maintain code written 6 months ago. Rewritten code suddenly
| becomes clearer, one begins to think about programming as an
| craft and not just hacking things on a keyboard until you get
| the desired result. Dependencies, judicious comments, code
| quality and a sane terseness become important. The journey
| starts here.
|
| Personally, step 1 was very short as I learned to program on
| the job. I was responsible for code, and so I needed to get it
| together quick. A language like Python makes 2 very easy, and 3
| came very quickly too since, as I mentioned, I was responsible
| for the code (one man team).
|
| I have met people who work at large institutions who are stuck
| at 1. Others with degrees in CS who are stuck at 2. Some just
| "get it" and go straight to 3. But typically, the real journey
| starts when you need to go on with work but your prior self is
| preventing you from being efficient. You need to get rid of
| that prior self's work to move on.
| yuvalr1 wrote:
| Totally agree!
|
| I think this is a journey a lot of software engineers go
| through. I can say I was walking down this path in the past.
| V-2 wrote:
| _" Firstly, I didn't talk to the person who wrote it. I rewrote
| the code and checked it in without their input. Even if it was an
| improvement (which I don't believe anymore), this is a terrible
| way to go about it."_
|
| While undeniably true, this feels completely orthogonal to the
| question of clean code. You could ruffle someone's feathers in
| the exact same way by taking code into the opposite direction.
|
| _" Goodbye, Clean Code [...] Don't be a clean code zealot"_
|
| I agree with choosing pragmatism over purism. But the clickbaity
| title exercises a pattern I've seen hundreds of times, to which I
| have frankly become a bit allergic over the years: "X considered
| harmful! ...I mean, if you're overdoing X". Not being a healthy
| food zealot doesn't equal "goodbye, healthy food".
| AnimalMuppet wrote:
| Well... you're right. But humans have a tendency to become
| zealots. They (we) take a good thing, and make it The One True
| Thing(TM). It happens with many people with many different
| things. It seems to just be human nature.
|
| So it may be good to remind ourselves "Hey, this is just a
| thing. Use it when it makes sense, _and don 't overdo it_."
| [deleted]
| PragmaticPulp wrote:
| > While undeniably true, this feels completely orthogonal to
| the question of clean code.
|
| I think this mentality and the Clean Code Movement (with
| capital Cs) actually have a lot in common.
|
| Much of the Clean Code movement is occupied with the idea that
| there is one, single, _right_ way of doing things. Anyone who
| does things differently is doing things the _wrong_ way.
|
| There is a subset of programmers who are attracted to these
| movements because it can feel like a free license to feel
| superior to your peers. Once you understand all of the rules
| and intricacies of the Clean Code Movement, it can feel like a
| free pass to push the Clean Code rules on to others and the
| codebase. No need for code review or consultation, because
| you've already decided that the Clean Code Way is the _right_
| way and therefore there isn 't anything to discuss.
| drewfis wrote:
| > No need for code review or consultation, because you've
| already decided that the Clean Code Way is the right way and
| therefore there isn't anything to discuss.
|
| I liken the CC movement to religion - it has good intentions,
| but when taken too far you end up with a holier-than-thou
| attitude.
| mdoms wrote:
| > Much of the Clean Code movement is occupied with the idea
| that there is one, single, right way of doing things
|
| Well if this is the case then they certainly didn't get it
| from the Clean Code book itself. The books very explicitly
| makes the point that there's no one right way and that the
| guidelines in the book won't apply to every situation.
| Jtsummers wrote:
| That's in the first chapter. I'll quote it here because it
| seems many of the objectors to the book haven't read it or
| haven't read this part:
|
| > Consider this book a description of the _Object Mentor
| School of Clean Code_. The techniques and teachings within
| are the way that _we_ practice _our_ art. We are willing to
| claim that if you follow these teachings, you will enjoy
| the benefits that we have enjoyed, and you will learn to
| write code that is clean and professional. But don 't make
| the mistake of thinking that we are somehow "right" in any
| absolute sense. There are other schools and other masters
| that have just as much claim to professionalism as we. It
| would behoove you to learn from them as well. [emphasis in
| original]
| theptip wrote:
| > this feels completely orthogonal to the question of clean
| code.
|
| For me, one of the interesting points of the article is not
| just that the replacement code was inferior, it's the
| recognition that the emotional compulsion to "clean up the
| dirty code" was a problem in itself.
|
| Sure, as you say, if you refactored it in the other direction
| in this would also be problematic -- but there aren't many
| people who feel a visceral emotional compulsion to refactor
| code to make it less "clean". (Maybe brainfuck aficionados?
| Anyway, a rare breed.)
|
| So I think there's something about the concept of "neat/clean
| code" that asymmetrically/directionally produces this error,
| and I took the "goodbye" to be giving up on this compulsive
| attachment to the concept of "all code must be cleaned", rather
| than saying that the concept of clean code shouldn't be used
| anywhere. The money quote being:
|
| > Am I saying that you should write "dirty" code? No. I suggest
| to think deeply about what you mean when you say "clean" or
| "dirty". Do you get a feeling of revolt? Righteousness? Beauty?
| Elegance? How sure are you that you can name the concrete
| engineering outcomes corresponding to those qualities? How
| exactly do they affect the way the code is written and
| modified?
|
| It's really more about identifying the feelings that "dirty"
| code produces and realizing that they might lead you astray.
|
| > "X considered harmful! ...I mean, if you're overdoing X"
|
| I'm 100% on board with the general objection to this kind of
| thing, it's a pet peeve that I share. I just didn't get
| triggered by this one :)
| gspencley wrote:
| The thing that bothers me about conversations in opposition to
| "best practices" or "clean" code" is that it misses the
| pragmatic aspect of those concepts.
|
| WHY do we write clean code?
|
| Because the value of software is its ability to CHANGE.
| Otherwise we would stick with fixed circuits. Best practices
| and clean code don't exist in a vacuum and they are not
| floating abstractions that developers should throw around as if
| everyone is on the same page with respects to what they mean.
|
| They are a set of quality standards that allow for the rapid
| development, adaptability and ease of maintenance as a codebase
| evolves over time.
|
| Clean code does not exist to slow or hinder development, or to
| handicap junior developers. It exists to maintain a team's
| velocity as the scope and scale of the project increases. Clean
| code is one of the crucial tools in the toolbox for avoiding
| death march projects. It it is not the only tool: an efficient
| management process, feedback loop, QA cycle, deployment process
| etc. are all critical as well. It is a team effort and code
| quality is the part that rests directly on the shoulders of the
| developers.
|
| The one and only part of the blog/article that I agree with is
| the part about refactoring without making it a teaching moment
| for the dev who wrote it. That part implies they weren't doing
| code reviews, another crucial element in the quality control of
| the product being developed.
|
| It blows my mind that these conversations are so often
| misunderstood. Us old timey senior devs have a professional
| obligation to make sure that the younger generation understands
| these concepts. Why do we see so many trendy medium articles
| and blog posts that seem to want to throw decades of hard
| learned lessons out the window? I blame the older generation
| for failing the younger ones. This stuff is not self-evident
| and has to be taught.
| discreteevent wrote:
| I'm from the older generation. I don't agree with a lot of
| what's in clean code. Particularly the focus on micro unit
| tests. Nor do I like the moralising tone of the books author
| in general. Also flexibility comes at a cost. A codebase
| where everything is easy to change becomes complex in
| aggregate, such that nothing is easy to change.
| gspencley wrote:
| Don't confuse Robert Martin's book "Clean Code" with the
| broader concept of clean code.
|
| "A codebase where everything is easy to change becomes
| complex in aggregate"
|
| What you're describing there is a code smell that is
| commonly referred to as "Speculative generality" and was
| written about in a book by Martin Fowler called
| "Refactoring, improving the design of existing code."
| Which, incidentally is a great companion book to Robert
| Martin's Clean Code.
|
| Speaking of trendy medium articles, here is one that I
| wrote on the concept of clean code that might help better
| understand what I mean when I use the term:
|
| https://medium.com/@gspencley/what-does-clean-code-mean-
| anyw...
| superjan wrote:
| In the article the author said that his "cleaner" version had
| less duplicate code, but was harder to adapt to new usecases.
| His point is that clean looking code is sometimes harder to
| change, because while cleaning people introduce abstractions
| or assumptions that hinder new usecases rather than
| facilitating them. The right way to keep code maintainable is
| not always self evident, and not necessarily always clean
| (which I interpret as SOLID, deduplicated).
| gspencley wrote:
| I didn't miss that, I just chose not to address it for
| brevity.
|
| The problem that highlights is both a process and an
| experience problem. It indicates that he did not have a
| clear picture of the development road-map for the project,
| which suggests there are silos of information. There were a
| lot of hints about this actually. The lack of code reviews
| (that's where the original dev's opportunity for quality
| improvement should have been caught and addressed), the
| fact that a manager would chastise him for trying to
| improve code quality and the fact that he made poor design
| choices when trying to improve said quality really does
| point to a dysfunctional process.
|
| Pointing the finger at code quality here is failing to
| understand the difference between the trees and the forest.
| sombremesa wrote:
| HN has a lot of people who seem to benefit from these types of
| "common sense" articles that bury the lede, judging by the fact
| they appear on the front page so frequently.
|
| (Putting common sense in scare quotes to acknowledge that this
| is new information to some, no matter how obvious to others)
| leoc wrote:
| I'd suspect it's more a matter of people enjoying the sense
| of validation they get from an article which tells them that
| the low standards they're accustomed to from work are good,
| actually.
| vletal wrote:
| Is it really the case? Could it maybe be that these
| clickbeity titles are upvoted and comment on more, regardless
| of the questionable added value to the community "burried"
| deep inside?
| MaxBarraclough wrote:
| I'd put it differently: HackerNews has an immune system to
| defend against low-quality submissions, including articles
| with clickbait titles, but it's imperfect. (As _dang_ puts
| it, _we 're trying for something different than internet
| default here_. [0])
|
| [0] https://news.ycombinator.com/item?id=29153668
| thinkharderdev wrote:
| Yeah, "don't overdo X" is always true by definition. That is
| what "overdo" means. Likewise "don't be an X zealot" is always
| true by definition (based on how we use the term zealot in
| modern English).
|
| That said, despite the clickbait title, I think these sorts of
| posts make a valid point because people often do in fact take
| heuristics like DRY to mean "never repeat any code under any
| circumstances and any code repetition is per se bad."
| deeviant wrote:
| They author didn't focus on the rewriting without checking with
| their coworker part of the lesson, and I'm not sure why you
| are. It felt like they added it because it was part of the
| truth and mentioning is honest, but the bulk of the article was
| on the actual code changes he made, why they thought it was a
| good software engineering as a their younger self and why they
| no longer think it was good software engineering now.
|
| I really don't see your point about the article being
| linkbait-y at all.
| _zachs wrote:
| Agreed! The article seems more geared towards a lesson teaching
| teamwork and not being a code cowboy than about clean code.
| literallyaduck wrote:
| It is a journey:
|
| Learning that you can deduplicate code.
|
| Learning that deduplication causes coupling.
|
| Learning how to inject behaviors.
|
| Learning how to discuss problems before job hobbying someone
| else's code.
|
| Learning that all code is really not what the end user cares
| about. They only care that it works and corporately you want to
| do it as cheaply as possible, privately you want to remain
| employable and increase your marketable skills. Everything else
| is a distraction.
| muzani wrote:
| IMO ideal code is code that looks like the problem it's trying to
| solve. Really good code will be faster to write than the user
| stories; it resembles pseudocode.
|
| A lot of bad refactoring is sort of like putting Chair as a
| subclass of Dog because both have four legs. Or someone thinking
| that long paragraphs have high cognitive load, so all paragraphs
| are split into 2-3 lines.
|
| In this scenario, OP simply changed the code without
| understanding what problem it was meant to solve. It's not about
| trading readability for flexibility, or respect or anything. It's
| that the code was probably further from solving the problem.
| hackbinary wrote:
| >> We could remove all duplication by grouping the code like this
| instead:
|
| 1. It seems to me that you refactored the code in the wrong
| dimension. I would have abstracted out a template shape object.
|
| 2. Are you not using a pull request/peer review process to accept
| changes into the prod branch?
|
| 3. I don't think you quite arrived at the right conclusion. While
| you conceed that your original conclusion was incorrect, I do
| think your original criticism has merit, and while your solution
| may provide a different set of problems, that doesn't mean that a
| different solution based upon the collaboration between yourself
| and your colleague might have been yet even better.
| jdmichal wrote:
| This is a good example of what I call Similar vs Same. "Similar"
| code is code that happens to look very close, but is actually
| addressing separate use-cases. "Same" code is code that looks
| close, and is addressing the same use-case. Same code is safe to
| unify, similar code is _not_.
|
| Of course, that's looks like a weak definition, because it ends
| up falling to, "how do you define your use-cases?" Well, that's
| kind of the point. The biggest thing to point out is that use-
| cases are not static, but change as the business changes. The
| second point exemplifies this:
|
| _For example, we later needed many special cases and behaviors
| for different handles on different shapes._
|
| So now they're definitely separate use-cases, that just happen to
| look similar because they're achieving similar goals. But the
| methodology may and can be completely different depending on the
| circumstance.
|
| So even if this was originally same code, and the unification was
| successful, it would have ended up split apart again anyway. And
| that's OK to. Sometimes things that really are the same use-case
| end up splitting as the business changes or learns. We as
| engineers need to recognize when this happens and split out the
| code also, instead of creating a tangled mess of one code serving
| two different use-cases.
|
| EDIT: Just read some of the comments, and my thoughts expressed
| here are probably a subset or incomplete version of "prefer
| duplication over the wrong abstraction":
|
| https://sandimetz.com/blog/2016/1/20/the-wrong-abstraction
|
| https://news.ycombinator.com/item?id=29240647
| nacho_weekend wrote:
| Blindly checking out code late at night, without PR review,
| without discussion about context, overwriting approved and (I
| assume) tested changes, for code you did not write and were not
| included on review for, BLINDLY merging to master (!?) - this
| could be tantamount to a fire able offense depending on where you
| work. I would have a hard, long talk with any of my mid or junior
| engineers if this was attempted (although master should ideally
| be protected, preventing this scenario unless OP had
| administrative senior permissions on the code base).
|
| All around a mess. I agree with the sentiment (write clean code
| unless you shouldn't) but there is some serious process issues
| here that should be caught and corrected and would have made this
| entire issue something during the PR review phase, where OP
| presumably asks for an improvement and is answered with the
| context for the decision. Not to mention the red flag of "I know
| better than my peers" attitude of many programmers I have known
| in the past, leading to distrust and backstabbing. It can get
| ugly.
| [deleted]
| 29athrowaway wrote:
| Here the author tries something then gives up. That does not mean
| clean code is not something to aspire or pursue.
|
| What is described as building trust can be also interpreted as
| groupthink and colleagues forming pacts of mutual non-criticism
| (which breeds mediocrity). Which is also the way to build
| dogmatic cults of personality, and becoming superficial
| developers that care more about talking about their weekend than
| their craft.
|
| You do not trust code. Memory and attention are fragile. Everyone
| makes mistakes. Everyone can have a bad day, be distracted,
| tired, etc. Building trust sucks. Trust nothing and be able to
| verify everything.
|
| Your job is to edit code: add, delete, modify code. Each time you
| check in code it becomes company property. It is not "your" code.
| Modifying other people's code is completely OK, and expected.
|
| In the end, this article is just wrong. "Give up, tech debt is
| your friend. Your job is to be best friends forever with your
| colleagues". All wrong conclusions.
| draw_down wrote:
| Still, if the math itself was repetitive it can be moved into a
| library/module that the shape classes call into.
| vsareto wrote:
| Some duplicated code is okay, but duplicated _math_ code seems
| more risky than duplicated CRUD code or other business logic.
| Math code /functions seem easier to unit test to me, deal with
| much simpler data types, and are a great use case for unit tests
| unlike CRUD code. CRUD and business logic often has dependencies
| that must be mocked too and math functions can likely be pure.
|
| Math code also seems way more amenable to being consumed as
| passed-in functions that are consumed as a black box, then those
| things call the math functions and get the result, and really
| don't care how the math was done. But it's then good to signal
| (debug logging) what math function was passed in and why. Here,
| you can have OvalSpecialCaseMathFunction() along side the regular
| ones too.
|
| I'm probably falling into the same pit as the author here, but it
| doesn't seem like duplicating every "kind" of code has the same
| cost/benefit.
| tinco wrote:
| Same here, baffled at that everyone's just taking the boss's
| judgement as gospel. Provided that he tested his refactor well,
| as his colleague I would be stoked someone went and cleaned up
| my code.
|
| It's not just about reducing lines of code and increasing
| abstraction, which I agree are bad metrics. It's about
| legibility. He took a block of 140 lines of dense math
| descriptions of objects, and reduced it to nicely structured
| semantic code. It's a 100% improvement, if that code has a bug,
| anyone could read it and find it.
|
| The author said they would later need support for different
| behaviour that would have made his code more convoluted. Well
| big deal, it's not like there was a way to do it without
| violating open/closed anyway. This code is so easy to
| understand, anyone can read it and go "ah this won't work with
| the new requirements, let's delete it and start over".
|
| Not saying this code 100% needed to be refactored, but if
| you're an engineer and you see it and you think you have some
| spare energy to pretty up a piece of the codebase, and you
| diligently test and verify your implementation, why not do it?
|
| I wish my team would do this more often.
| cjfd wrote:
| I think in general if there is duplication and an abstraction can
| be made it should be done. The (in)famous article about
| 'duplication is cheaper than the wrong abstraction' that is also
| mentioned below is seriously one of the articles about
| programming that I hate the most of all articles about
| programming. It goes very awry when it says 'Programmer B feels
| honor-bound to retain the existing abstraction'. As programmers
| we should expect the continuously refactor our abstractions and
| be very eager to do that. Of course, such a thing is only
| realistic with automated tests. Reluctance to change things is
| how things stagnate and go bad. Even if you create an abstraction
| that is wrong and that is undone later you have learned
| something. Just adding to the code base while not even attempting
| to improve anything does not teach one much and is the road to
| stagnation in poor code quality, aka the famous unmaintainable
| mess.
| [deleted]
| ladyattis wrote:
| This is why I tend to avoid refactoring code as often I find just
| having a common pattern in code that's been adopted either
| officially or unofficially by the dev team being more important
| than trying to 'clean it up.' Sure, I say delete commented out
| code since we have git and other VCS today that makes this habit
| unnecessary but if a piece of code doesn't use LINQ style queries
| to parse lists and uses for loops instead, I just leave it be.
| RbrtM wrote:
| Clean code vs dirty code is not a problem here. Rewriting someone
| else's code (no matter how dirty) without telling or consulting
| was the bigger mistake. This can poison professional
| relationships for life.
| baq wrote:
| that relationship arguably deserves to be poisoned, it's a
| relationship between toddlers instead of engineers.
| ravenstine wrote:
| This post is such a breath of fresh air! I'm glad I'm not the
| only one who has come to realize that a lot of "clean" code is
| actually just developers trying to be clever or scratch an itch
| of obsessive-compulsion.
|
| Repetition isn't necessarily a bad thing. If the repetition isn't
| difficult to alter and it more clearly describes what is
| happening on a step by step basis, it's difficult for me to call
| that "dirty".
|
| Code is for _humans_ , not computers, first and foremost. Certain
| things necessitate performance early on, but for a lot of
| projects it isn't reasonable for all code to make _absolute
| logical sense_ from the get-go. What 's more important is that
| people can _read_ the code and _understand_ it without undue
| _deciphering_.
|
| Sometimes I come back to code that I've written long ago. When I
| wrote that code, I almost _always_ thought I was writing "clean
| code". It often turns out that the code I was immediately able to
| understand and make changes to was the code that had repetition,
| wasn't mindlessly spread out into a bunch of "tiny functions",
| and had comments spread out to describe my reasoning.
|
| In contrast, "clean" code is often inherently hard to change
| because it relies on _centralizing_ functionality. When you
| centralize something and you change it, it may work for one
| circumstance but mysteriously cause something else to break. With
| repetition, your code base might be larger but code may be more
| decoupled and thus easier to make a small change to fix one thing
| without breaking another similar thing.
| hunterb123 wrote:
| When you say centralizing I believe you mean coupling. When
| writing clean code you should reduce & abstract but not couple.
|
| Flexibility should be kept in mind. In the blog post example I
| would have at least abstracted the math out and there's no
| reason of the limitation of one handle function, you could swap
| them out.
|
| Yeah I may a be a bit OCD, no I'm not trying to be clever. I'm
| trying to reduce complexity so I can keep the entire system in
| my head to optimize things easier. Don't atomize everything
| into tiny functions though, there's a huge middle ground.
|
| No book gets everything right, but I recommend The Pragmatic
| Programmer for this sort of thing.
|
| People seem to use wrong instances of people "cleaning code" to
| justify dirty code and tech debt.
| acrophiliac wrote:
| I disagree with the author, who says "My code traded the ability
| to change requirements for reduced duplication, and it was not a
| good trade." I think this is a great trade, because the number of
| times I've seen errors resulting from duplicate code is FAR
| greater than the number of times I've had to kludge an
| abstraction because of changed requirements.
| makecheck wrote:
| I used to be much more of a curmudgeon about wanting to clean up
| _existing_ code on sight but a few bits of reality seeped in over
| time, such as:
|
| 0. Existing code is probably working fine, and making changes
| (for any reason) risks adding a bug. Worse, reviewing code with a
| bunch of reformatting is tedious and the reviewer may assume
| you've _only_ done reformatting and not notice the accidentally-
| changed behavior either.
|
| 1. You have to merge, a lot. And "fixes" that do little except
| reformat or redo are a _pain_ to deal with when all you _really_
| care about is getting functionality in. This multiplies across
| branches and team members, and might require multiple manual
| merges.
|
| 2. There is a decent chance the code you're not pleased with will
| be ripped out entirely as part of some bigger change, at which
| point all effort to fix unclean parts is moot.
|
| 3. Far more people than you are familiar with how it _used to be_
| , warts and all. The "cleaned" version now looks alien to
| everyone else and might slow them down.
|
| That said, there is a time to clean things up; it just has to be
| at a well-defined point in the project. It involves a combination
| of things from the article (e.g. talk to the team) and the above
| (e.g. do it when many features are merged in and there are few
| branches to deal with).
| shane_b wrote:
| For my team, we use a rule of two naive implementations then
| the third is a refactor.
|
| Cleaning code is often wasted effort when you don't have the
| full picture or understand future variations. Like you
| mentioned with ripping out your refactor later.
|
| This improves the speed of implementation one and two because
| we don't have to think as much. Then third iteration is faster
| as well because it's obvious how to abstract.
| SamuelAdams wrote:
| 4. It increases personal liability and risk if something goes
| wrong due to a refactor.
|
| I've worked at "blame game" companies before, if your name was
| on the commit it was your fault, all other process steps be
| damned.
|
| So it reduced my inventive to change things unless actually
| required to ship a feature or fix.
| brainwipe wrote:
| I think articles like this serve to drive absolutism through
| clickbait title. "Goodbye clean code" becomes "It's an attempt to
| make some sense out of the immense complexity of systems we're
| dealing with". And on the other side you have people writing
| "Comments are evil, burn die" and in the text you have "Comments
| shouldn't repeat the code, but are necessary sometimes, use your
| brain".
|
| In the real world of real people coding and not in click-bait
| land, people use clean code as a tool that's sometimes
| appropriate.
| WalterBright wrote:
| 1. A novice follows the rules because he is told to.
|
| 2. A master follows the rules because he understands the point of
| the rules.
|
| 3. A guru breaks the rules because he knows they don't apply.
| dktoao wrote:
| I realize this a click-bait title, and the article plus the
| discussion is more nuanced (this is why I love HN!). But I think
| less experienced developers might take this as a license to just
| write pages and pages of spaghetti, then point to this article
| and say "Duplication is good!". Duplication is not good, it is
| the enemy of maintainability. However, sometimes under very
| complex software requirements (which hopefully aren't needlessly
| complex), there is no good abstraction and attempting to create
| one will just make your code confusing and inefficient. However,
| there are many things that you can easily abstract safely and
| practice doing this makes a developer better. If you are a junior
| developer I highly recommend taking this article with a huge
| grain of salt or outright ignoring it. Both an ill-fitting
| abstraction and duplication are going to lead to messy code,
| messy code is a fact of life, but only one of these practices is
| going to lead to good habits and help you develop as a software
| engineer (hint: it isn't duplicating your way out of problems).
| rbongers wrote:
| There must be another more common term for this, but an older
| programmer once told me to watch out for "false parallelism" -
| code that is currently duplicated, but is likely to diverge in
| the future, and so trying to make the code part of the same
| abstraction would not be meaningful.
|
| There's also the fact that with complex abstraction, you run into
| the problem that it can be more difficult to maintain as there's
| more layers of indirection.
|
| That cost of abstraction has been mentioned elsewhere, but I want
| to throw out there that both of these points fall under the
| umbrella of a powerful concept for writing maintainable code,
| that you should not only think about the best way to structure
| your code _now_ , but you should consider how that code will
| change and how that structure will facilitate or hinder that.
|
| What I think I would have favored in this case would be to use
| _some_ small, well-named utility functions that can be used to
| implement that math (not necessarily to replace all of it). I
| cannot see the code so I don 't know for certain if utility
| functions would actually help, but I have taken this approach for
| geometry oriented code in the past and it worked well. In
| general, don't be afraid of writing small utility functions.
|
| I don't agree with the concept of letting "clean code" go. All
| he's done is replace "clean code" patterns with other ones that
| work better for the situation. What's best is becoming familiar
| with new patterns like the ones I've described above, not just
| "clean code" patterns, weighing their benefits and downsides for
| the current case, and not necessarily worry about whether you're
| using patterns that are arbitrarily part of a paradigm that
| hasn't worked in other cases (however, remaining aware that you
| don't know every pattern).
| mdtrooper wrote:
| The famous book about Clean Code was a big Oxymoron, because the
| examples in it pages was in Java.
| manuelabeledo wrote:
| Java is a fairly clean language. It may be verbose, but that is
| it.
| pjmlp wrote:
| It depends where one gets their beans from.
| rootlocus wrote:
| I work with clean java, and it's a charm!
| game_the0ry wrote:
| As a one time java dev, I see what you are saying, but modern
| java is ok. Nowadays, its more on the dev team than the
| language.
| ChrisMarshallNY wrote:
| That was a great article!
|
| I really appreciate it, when people write from hard-won personal
| experience.
|
| For myself, I write in Swift, and it's quite possible to write
| totally inscrutable code in Swift. I am guilty of making code
| harder to read, and less grokkable, in my "cleanup" sweeps.
|
| I am currently doing a bit of navel-gazing, on this very topic. I
| have gotten into the habit of writing very "swifty" code, and am
| thinking that I should probably back off a bit from that.
| breckenedge wrote:
| This article really buries the most important lesson about
| refactoring:
|
| > I didn't talk to the person who wrote it. I rewrote the code
| and checked it in without their input. Even if it was an
| improvement (which I don't believe anymore), this is a terrible
| way to go about it. A healthy engineering team is constantly
| building trust. Rewriting your teammate's code without a
| discussion is a huge blow to your ability to effectively
| collaborate on a codebase together.
|
| Also (2020).
| mannykannot wrote:
| While that is undeniably an important issue, personally, I
| would pick this as the more important point in this particular
| context:
|
| _" My code traded the ability to change requirements for
| reduced duplication, and it was not a good trade. For example,
| we later needed many special cases and behaviors for different
| handles on different shapes."_
|
| The reason for my choice of this point is that provides a
| direct, technical counter to one of the simplistic technical
| rules on which the Clean Code movement is founded, and so might
| make a impression on someone who has, heretofore, regarded the
| case made by Clean Code proponents as irrefutable.
|
| That's also why I say "in this particular context", as in the
| broader scope of software development practices generally, the
| interpersonal dynamics of the organization are almost always
| more important than coding style.
| francisofascii wrote:
| And... > It was already late at night.
|
| Probably not the best time to refactor someone else's code that
| currently works.
| shikoba wrote:
| Why one should ask to the original developer if the code can be
| modified?
|
| If I see a code I want to change, I do it. I don't ask for
| permission. And colleagues can do the same with code I wrote.
| We trust each other.
| emerongi wrote:
| The other developer usually has reasons why they designed it
| that way. If you just rewrite other people's code, it makes
| it seem like you think there is One True Way to writing code
| and that you're smarter than them.
|
| I'd ask first if they agree with a certain improvement. It
| shows I value them as an engineer and they might bring up
| critical information that I'm missing, making my
| "improvement" actually worse. It doesn't hurt to talk to
| people.
| datavirtue wrote:
| I just assume that they got one pass at it. Agile!!
| ThaJay wrote:
| I agree Agile steers a team towards hastily bodged
| together code.
| lowercased wrote:
| Years ago (2005?) I worked at a call center. Most agents were
| on one coast - data center was in another coast. Agents in
| our call center complained about speed. When they'd pull up
| an account, it was taking seconds... sometimes 15-20
| seconds... to pull up the account.
|
| This was all a bespoke CRM system. I poked in the code (it
| was something I had access to) and noticed that ... the
| entirety of the whole screen was duped - the output of the
| client's history was embedded in an HTML comment tag. So...
| if the client had, say, 3 years of info/comments, it was
| rendered, then rendered again in HTML comments. It was a
| single line duplicating the entirety of the info. It was
| obviously a debug remnant. I removed it. I tried to talk to
| the original developer beforehand - he was on the phone, and
| kept waving me off. I emailed him. No answer. I committed it
| and got it to a testing server. The test guy immediately
| loved the speed improvement, and we got it out to the floor.
| There were about ... 50-70 agents live at any one time, and
| everyone's hold/wait times were cut by 20-30% overnight.
| Clients and agents were happier. I found out later that, over
| time, we cut bandwidth bills by a measurable amount.
|
| I was raked over the coals and nearly fired for that.
| "unprofessional", "insulting", etc. It was primarily because
| I'd made someone else look bad. _Other people on the team_
| also made changes now and then to each others ' code without
| asking permission, when it was needed/obvious
| (emergencies/etc). It was a simple oversight in removing some
| debug code, and it made a huge difference (it had been tested
| and had other eyes on it as well - this wasn't "change prod
| by hand without telling anyone").
|
| Still bugs me to this day (as you can tell).
| emaginniss wrote:
| A lot of people attach their ego to the work that they
| produce. I did when I was younger and would try and defend
| poor choices because I had made them. They were my
| children. Once I put that aside and separated my ego from
| my code, I got much more comfortable pointing to my own
| mistakes and learning from them. I am a far better engineer
| and teammate because of that.
| shikoba wrote:
| You mailed the original developer, he didn't answer. And
| you were fired for doing your job right. What you must
| conclude is that they don't care about quality but about
| ego. They have a philosophy that don't favor quality. You
| should be glad they fired you.
| lowercased wrote:
| I wasn't fired, just... the threat was raised. That
| original dev left soon after (in no way related to that -
| he'd been looking around for a while from what I heard).
| It was a strange situation because in the group of... 8
| or 9 of us, I was the last one in, and got along OK with
| everyone, except him.
| dc443 wrote:
| The point your parent post was making was that regardless
| of whether or not you actually got fired doesn't actually
| matter. The signal you should be paying attention to
| (which you are surely aware of, since you say this
| episode still bothers you) is that the environment was a
| toxic one so if they did fire you they would have been
| doing you a favor. At least from a silver linings point
| of view.
| sigzero wrote:
| If they were still there, why wouldn't you engage them about
| a modification you think was better than the original?
| Everyone learns in that situation.
| lowercased wrote:
| they may not be 'on that project' any more, and if the
| burden of support is on you at that point, and you're the
| one dealing with the code, and they're not, them saying
| "meh, it was good enough" doesn't really help. I've hit
| that before, and mgrs have said "devX said it's good
| enough, they wrote it before, don't try to change stuff".
| Well.. they wrote it 2 years ago when there were 300
| customer records, not the 14200 today, and they're not
| fielding support issues from people complaining about
| delays now.
|
| There's plenty of reasons to not engage with someone.
| There's also plenty of reasons why it may make sense, and I
| think it's highly context dependent. For me, it's about
| 50/50. And... in the cases where I reach out, it's about
| 50/50 as to whether they have any
| time/inclination/memory/ability to help anyway.
| uberman wrote:
| The student said to the teacher:
|
| _" This fence is in the way and obstructing the flow, it
| should be removed"_
|
| The teacher said to the student:
|
| _" If you can tell my why someone made the fence in the
| first place, I will allow you to remove it."_
|
| Note that this has nothing to do with "code ownership". If
| you worked for me and randomly changed code that you did not
| like, I would fire you.
| shikoba wrote:
| > I would fire you
|
| Ha ha ha. That's not how it works with me.
| uberman wrote:
| Then I guess I would fire you :-)
| emerongi wrote:
| > If you worked for me and randomly changed code that you
| did not like, I would fire you.
|
| From one extreme to the other? People make mistakes,
| educate them instead of brutally punishing them. Talk it
| through - isn't this exactly the mistake that was made by
| the developer (they didn't talk to their peers)? If they do
| not respond to feedback, that can eventually lead to a
| firing.
| uberman wrote:
| Of course I would talk to a junior dev right out of
| college and tell them these kind of "drive by"
| refactorings were counter productive and dangerously
| susceptible to the introduction of lack of context
| bugs... ONCE. The hiring process is too expensive and
| time consuming to punt on a junior dev or intern for such
| a transgression.
|
| However, if the behavior persisted then yes, I would fire
| them.
|
| If anyone sees code that they hate they are free to
| submit an issue and I will be happy to review it and
| prioritize it with the other work to be done.
|
| If they hate code that smells so much that they want to
| volunteer their time to refactor it, then they can
| participate in code reviews for me instead.
| breckenedge wrote:
| Because it wasn't necessarily an improvement on the original
| code. A discussion should have happened either during the
| original commit or before the refactoring commit.
| nibix wrote:
| Trust would also include to consider that there might be a
| reason for why the code is the way it is, even if it is not
| directly obvious.
|
| A brief chat can then help to clarify things.
| cmsefton wrote:
| Agreed, it's always worth remembering Chesterton's Fence.
| https://fs.blog/chestertons-fence/
| yxhuvud wrote:
| The reasonable thing to do is often to add the original
| developer as a code change reviewer. They should know about
| what the code is doing and about any pitfalls involved. If
| they don't like the changes, then it is easy to take the
| discussion then.
| RbrtM wrote:
| If I ever see you again rewrite someone else's code without
| asking, I will fire you on the spot.
| shikoba wrote:
| So deluded. :D
| baq wrote:
| i'd fire you for micromanagement, mismanagement and
| encouraging bad practices.
| [deleted]
| emerongi wrote:
| Ironic. You learned nothing from the article.
| pm215 wrote:
| Two reasons, one technical, one social. Firstly, the original
| developer probably spent more hours thinking over and working
| on that bit of code than you have -- so maybe they had a good
| reason for doing it the way they did. You won't know unless
| you ask them. Secondly, I think it's a fairly natural and
| common human behaviour that when you spend a lot of time
| working on or making something that you feel some degree of
| satisfaction, pride and investment in the result. Even in a
| cooperative joint endeavour that is striving to avoid an idea
| of individual "ownership" of parts of the project, I think it
| can be helpful sometimes to acknowledge that feeling of
| investment by consulting the original author before walking
| in and radically changing something they did.
| photochemsyn wrote:
| I've heard this many times... 'take ownership of your code'
| ... but the problem is the vast majority of devs work for
| corporations who own the code, and unless you're a
| shareholder that code certainly isn't yours, it's corporate
| property, that's the deal in most employment scenarios.
|
| Realistically, people doing such work care very little
| about writing 'great code' because they know they have no
| real 'ownership'. There hopes for higher pay and
| recognition rely on climbing the corporate ladder by
| whatever means available. Team member, team leader,
| division manager, VP of whatever, etc. Blame bad results on
| someone else, that's the normal tactic for these types.
| Don't hold up production over code quality concerns,
| because delays in pushing product to market upset the
| shareholder board, which they see as lost profits.
|
| The whole notion of a 'skilled technical individual who
| takes pride in their work because they own it' sounds like
| some awful corporate in-house propaganda campaign to be
| honest. And this accounts for much of the current mass
| exodus from the corporate workforce, I imagine.
| yongjik wrote:
| There's a big difference between fixing a problem that's been
| in production for two months, and rewriting code your
| coworker pushed two hours ago. For one thing, your coworker
| might be still working on the next iteration of the code, and
| adding an unnecessary diff could easily cause hours of wasted
| time.
| chris_wot wrote:
| The "duplicated" example seems to me to be similar to an
| inherited class. An abstract class defines the interface
| Rectangle, has resizeTopLeft, etc. and you implement the
| interface.
|
| It's not duplication at this point.
|
| _edit:_ I'm an idiot. Perhaps not making it an abstract class.
| But it's one of those examples where inheritance might actually
| help.
| sesm wrote:
| See also: "Reuse Versus Compression" chapter from the book
| "Patterns of Software" by Richard Gabriel.
| angarg12 wrote:
| All of that would have been avoided with a code review.
| Subsentient wrote:
| Ahh, well. All I can say. Is.
|
| REEEEEEEEEEEEEEEEEEEEEEE!!!!!!!!
|
| /s
| [deleted]
| LennyWhiteJr wrote:
| "first you learn the value of abstraction, then you learn the
| cost of abstraction, then you're ready to engineer"
|
| - Kent Beck
| gpderetta wrote:
| Premature abstraction is the root of all evil.
| dgb23 wrote:
| I liked this article about "semantic compression"[0] by Casey
| Muratori very much. He also posted some video lectures fairly
| recently about what he calls "non pessimisation" (a term that
| I personally didn't read anywhere else so far), which is
| mostly concerned with performance, but the idea is to simply
| avoid and isolate abstraction and indirection and programming
| in a style that adheres to what some people describe as
| mechanical sympathy.
|
| It basically comes down to:
|
| 1. Write the code that makes the computer do what it needs to
| do to achieve a specific goal. No more and no less.
|
| 2. Refactor ("compress") the code by deriving re-used
| (implicit) structures and functions.
|
| He puts a lot of emphasis on those steps being done in order
| and on deferring the second step
|
| Note that this is from someone who writes
| games/engines/tooling so there is a large, real pressure to
| write simple and reasonably performant, optimizable code. He
| would also prefer that most of programming would be done in
| this way.
|
| [0] https://caseymuratori.com/blog_0015
| giu wrote:
| There are only two hard things in Computer Science: cache
| invalidation and to know when not to use abstraction.
| grishka wrote:
| But what about naming things
| bryanrasmussen wrote:
| naming things is a form of abstraction.
| s_dev wrote:
| Thats an off by one error which naturally is included in
| all CS laws.
| dkersten wrote:
| Maybe the wrong thing was invalidated from the cache of
| hard things in computer science.
| commandlinefan wrote:
| There are only two hard problems in computer science:
| cache invalidation, naming things and off-by-one errors.
| tgv wrote:
| Follows by symmetry. Proof is left as an exercise for the
| reader.
| the_only_law wrote:
| I cost myself a position recently arguing about this.
|
| I had a first round interview that went well, and was given a
| take home and the problem was very simple, dead simple. In
| turn I tried to keep the code as simple as possible, with
| only necessary abstractions.
|
| The second interview was with an entirely different person
| who seemed displeased I didn't bloat the solution with all
| sorts of enterprise patterns, dependencies and conventions. I
| was kinda backed into a wall and while attempting to explain
| my reasons I ended up in an argument with the interviewer and
| it was clear we did not see eye to eye. He would later
| challenge my ability to perform in a patronizing tone and I
| was sure the opportunity was dead
|
| It wasn't the only thing that went wrong with that interview,
| but I feel that disagreement was the biggest killer.
| ncmncm wrote:
| Looks to me like you dodged a bullet.
| pelasaco wrote:
| Kent Beck said so many other great things.. he was really ahead
| of his time: http://blog.cleancoder.com/uncle-
| bob/2013/12/10/Thankyou-Ken...
|
| Yes, Uncle Bob :)
| btbuildem wrote:
| You're lucky! Once upon a time I had a team lead that had the
| same priorities as OP. Code cleanliness was more important than
| the architecture of the solution. It was infuriating.
| manuelabeledo wrote:
| I have seen this so many times, it is not even funny anymore.
|
| Refactoring is fine. Abstraction is fine. But programmers fall
| to, at some point in time, some sort of tunnel vision that drives
| us to use either tool to target the wrong thing.
|
| At my current job, we have this tool to create query
| abstractions, a Spring Data of sorts if you will. It seems nice,
| and it naturally feels better than the alternative, which is
| allow the upper layers to issue queries themselves.
|
| But it is _wrong_. All flexibility is gone. Our team is just not
| big enough to expand the functionalities of such library, so the
| queries it can handle are pretty basic. Also, there are at least
| five layers to go through in order to debug it, and the only
| thing it really does is converting objects into queries.
|
| Of course, most API consumers have to bypass the library. It is
| just not flexible or powerful enough.
|
| Moral of the story is, to accomplish a big refactor, having a
| clear, global vision of the long term benefits and shortcomings,
| is essential. Refactoring just for the sake of making something
| look good, is not worth the time.
| Cthulhu_ wrote:
| "A little duplication is better than the wrong abstraction"
| (https://sandimetz.com/blog/2016/1/20/the-wrong-abstraction)
|
| My current codebase has a lot of duplication for fairly trival
| things, because the previous codebase tried generic solutions,
| only to end up needing a lot of exceptions, which ended up making
| the code look messy and hard to maintain.
| one_off_comment wrote:
| The real story here is not about the code.
|
| When it's your own personal project and you can be your own
| little tyrant, do whatever you want. Be as "clean" as you need to
| be. But this was at work, and Dan was being a bad coworker.
|
| He snuck in a change in the middle of the night over a coworker's
| code. This code wasn't his responsibility. He didn't leave his
| thoughts on a PR where others could discuss it. He overwrote
| someone else's code without asking. There was no opportunity for
| discussion or collaboration. Dan's actions said he's right and
| his coworker is wrong. They said he doesn't trust his coworkers.
| They said he knows best. They said if you want something done
| right around here you have to do it yourself. Dan was not being a
| team player. This story is less about code and more about team
| dynamics.
|
| I hope Dan knows this and he was just trying to sneak the message
| past people who don't take social cues as easily as others. I
| hope his boss didn't just tell him to revert his change. I hope
| his boss told him why.
|
| EDIT: I just reread the article and I think Dan missed the point.
| This wasn't about the code, as I said. But Dan really does seem
| to think it's about the code.
| nacho_weekend wrote:
| Agreed, 100%. I would be beyond livid if a coworker did this to
| me if I were the original committer. OP does not realize the
| insane level of mistrust he just created with this action.
___________________________________________________________________
(page generated 2021-11-16 23:02 UTC)