
Goodbye, Clean Code - danabramov
https://overreacted.io/goodbye-clean-code/
======
burlesona
I’ve usually heard this phenomenon called “incidental duplication,” and it’s
something I find myself teaching junior engineers about quite often.

There are a lot of situations where 3-5 lines of many methods follow basically
the same pattern, and it can be aggravating to look at. “Don’t repeat
yourself!” Right?

So you try to extract that boilerplate into a method, and it’s fine until the
very next change. Then you need to start passing options and configuration
into your helper method... and before long your helper method is extremely
difficult to reason about, because it’s actually handling a dozen cases that
are superficially similar but full of important differences in the details.

I encourage my devs to follow a rule of thumb: don’t extract repetitive code
right away, try and build the feature you’re working on with the duplication
in place first. Let the code go through a few evolutions and waves of change.
Then one of two things are likely to happen:

(1) you find that the code doesn’t look so repetitive anymore,

or, (2) you hit a bug where you needed to make the same change to the
boilerplate in six places and you missed one.

In scenario 1, you can sigh and say “yeah it turned out to be incidental
duplication, it’s not bothering me anymore.” In scenario 2, it’s probably time
for a careful refactoring to pull out the bits that have proven to be
identical (and, importantly, _must be_ identical across all of the instances
of the code).

~~~
scarface74
That can be boiled down to the “Rule of 3”.

My CTO often asks me to implement a feature to do X and make it “generic
enough to handle future use cases”. My answer is always the same - either give
me at least three use cases now or I am going to make it work with this one
use case. If we have another client that needs the feature in the future then
we will revisit it.

Of course, there are some features that we know in advance based on the
industry how we can genericize it.

~~~
Stratoscope
The Rule of 3 is a great rule, except when it isn't.

I had a colleague some time ago who wrote a couple of data importers for FAA
airspace boundaries. There were two data feeds we cared about, "class
airspace" and "special use airspace". These airspace feeds have nearly
identical formats, with altitudes, detailed boundary definitions, and such.
There are a few minor differences between the two, for example different
instructions for when a special use airspace may be active. But they are about
95% the same.

The developer wrote completely separate data definitions and code for the two.
The data definitions mostly _looked_ the same and used the same names for
corresponding fields. And the code was also nearly the same between the two
(in fact exactly the same for most of it).

It was clear that one importer was written first, and then the code and data
structures were copied and pasted and updated in minor ways to create the
second.

Because the data structures were unique for each (even if they looked very
similar in the source code), this impacted all the downstream code that used
this data. If you saw a field called FAA_AIRSPACE_MIN_ALTITUDE, you had be
sure to not confuse the class airspace vs. special use airspace, because each
of these had a field of the same name, the compiler wouldn't catch you if you
used the wrong one, and you may have the wrong offset into the data structure.

I asked the developer and they told me, "I have this philosophy that says when
you have only two of something, it's better to just copy and paste the code,
but of course when you get to the third one you want to start to think about
refactoring and combining them."

Yep, the Rule of 3.

In this case there were only ever going to be two. And the two were nearly the
same with only minor differences.

But because of blind obedience to the Rule of 3, there were many thousands of
lines of code duplicated, both in the importer and in its downstream clients.

I still like the Rule of 3 as a general principle, and I have espoused it
myself. But I think it is best applied in cases where the similarities are
less clear, where it _seems_ that there may be something that could be
refactored and combined, but it's not yet clear what the similarities are.

I think it is a very bad rule in a case like this, where it should be obvious
from the beginning that there are many more similarities than differences.

~~~
gameswithgo
Every single rule or advice in programming is good until it isn't. OOP is good
until it isn't, function programming is good until it isn't, premature
optimization is the root of all evil until it is the root of all good.

For some reasons humans have this deep need to try and boil things down to
bulleted lists which in the domain of programming are just incredibly not
useful.

~~~
bjornjajajaja
This is because programming is an art. It has fundamental components (objects,
functions, templates, iterators, etc) like in visual design (point, line,
shape, value, etc).

I think engineers should read the old and the new C++ books and master that
language to know the evolution of all these paradigms and how to use them.
There’s so much wisdom in the “Effective C++” series and Gang of Four and “C++
Templates: The complete guide“ to name a few.

Problem is in this “start up culture” to bang things out and get them working
the art is left behind. Just like many other arts.

~~~
hermitdev
I was part of an uncredited team interviewed by Meyers for "Effective Modern
C++". Always thought ir was somewhat ironic. At the firm (declined
acknowledgments because they shun media attention) we werent even using a
C++11 compiler on either Linux or Windows at the time. Yet, at least two of
the patterns, I recognize as being at least a partial contributor to.

Myself, I lost track of C++ standards changes with C++17, and Ive not been
using C++ for the last several years.

I still love the power and speed, but right now I'm dping more ETL work, and
Python is a better and more productive language for that.

------
bjourne
> 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. 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.

I totally disagree one million percent! If you are on my team and you want to
rewrite code I wrote (cause it sux) then do it! Don't ask, just DO IT! DO IT
NOW! Have a blast, tear it apart, rewrite all my shitty abstractions and see
if you can do it better. If the result is better code, then awesome! I learn
something and the software gets better. If the result is worse code, then
awesome! I'll tell you why, you'll learn something and your improved
understanding will allow you to write better code.

I also think that a salaried engineer who thinks that a piece of code he or
she (but almost always he) wrote is "his" or "hers" is totally wrong. It's the
company's code. Having a false sense of ownership towards that code will just
cause grief for that engineer and friction for his or her team.

~~~
BinaryIdiot
> I also think that a salaried engineer who thinks that a piece of code he or
> she (but almost always he) wrote is "his" or "hers" is totally wrong. It's
> the company's code.

While this is _technically correct_, this isn't how humans work. Humans attach
their worth to things they do, even when they shouldn't. It's a difficult
thing to avoid to most people so if you write some code, commit it and then
later see a teammate completely rewriting it, it can come off as either:

1\. You spent a lot of time and hard work on code that you kinda identify with
(I mean, it's code you wrote. It's your "art") and then someone comes in and
re-writes it without even asking you about it can make it feel that they think
you're an idiot.

or maybe even worse

2\. They didn't know the reason you wrote it that way (maybe it was in support
of future changes?) and now they just screwed it all up.

Maybe you haven't run into either of those situations. If you haven't, great!
I spent the first part of my career hitting the first one because of how hard
it can be to disassociate with the work you produce and the second one I see
happening occasionally. It's a breakdown in communication within a team.

~~~
jdnenej
This is how _some humans_ work. No one on my current team has ever expressed
disatisfaction when their code gets changed.

~~~
abcpassword
Same here. And my own code is rewritten frequently as business needs change or
better factorings are discovered when we hit the rule of 3, as mentioned in
this thread.

It's nothing personal; we all get paid and go home. There's a lot of bruised
egos in this thread that would be better spent in therapy, I think.

------
altitudinous
I'm 52 many would consider my code a mess. Been a professional coder ->
solution architect all my life, I work for me now with my own apps. With my
own code I clean things up when I can, but sometimes it isn't worth it. I used
to write clean code, spend time doing it but no more.

\- Rewriting requires retest, introduces new bugs.

\- If it ain't broke, don't fix it.

\- Users don't care about clean code. They only care about the end product.

\- Developers (less experienced?) obsess about this stuff. It is so expensive.
Layers of coding management to deal with it all.

\- If your code requires constant maintenance you are doing it wrong.

\- Obeying the above rules means you can leave old software behind and
generate new software if you want.

I am about to celebrate 10 years of my first software (an app) I wrote for
myself. It is still in prod. It is fine, been earning $$ for years. I don't
touch it unless I need to support new devices or support API updates. On the
flip side, there isn't much code I wrote in my previous professional career
that is still live - the reason is vendors only make a profit because they
need to generate cashflow constantly, and they do it by breaking the above
rules unnecessarily.

Do you work for a vendor? Think about the real reason you are breaking the
above rules - the reason is because the extra time is billable. You are not
breaking the above rules because it is the best way to write software. You are
breaking the above rules because it generates the most profit for the company
you work for.

~~~
lmm
> \- If your code requires constant maintenance you are doing it wrong.

Disagree. Your code should reflect the business you're in, and changes to the
business are what creates opportunity and let you turn your skills into
profit.

Clean code is absolutely a means to an end, and if you've got a codebase
that's just sitting there fulfilling some static business purpose then yes, it
makes sense to leave it ugly. (Similarly with code that you know is being end-
of-lifed soon - if there is not going to be a need to make changes to the code
in the future then it's fine to load it up with technical debt until it
sinks). But that's the exception rather than the rule.

~~~
humanrebar
Even for static business purposes, the target environment changes over time.
The OS gets upgraded, security bugs are discovered, dependencies become end-
of-life'd, language runtimes are replaced, etc.

------
temac
> 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.

I'm against the idea that people should be attached to "their" code (that is:
the code they wrote). Now I _also_ understand that humans that humans, but the
priority should be to make them evolve toward more detachment from their work
and acceptance that what they design "can" (actually will) be imperfect,
rather than avoiding upsetting them even when it would not be justified to be
upset. Plus one essential purpose of source control is to be able to make
changes, and revert them if they were "wrong"; or for any other purpose. Maybe
propose a patch instead of committing directly, but that's really a cultural
matter about how projects are organized.

I don't want to ask of permission for an improvement. If there is a need to
have formal authorization of maintainers responsible for parts of the code,
they setup just that. Otherwise, I'm certainly going to improve "your" code,
_in some rare cases_ without even telling you (that's not a goal in itself to
do it behind your back, obviously, and I also value collaboration -- but that
should not be a problem). The question that remains is: is it really an
improvement. If you are not sure, then maybe don't commit. If you are, do it
(following your local rules), and if it ends up being a mistake, yes, it will
be reverted, and so what?

You should not take issue of your work being reverted (for good reasons), like
other people should not take issue of "their" code being modified. Better ask
for forgiveness than permission.

> 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.

Then re-expand the code later.

~~~
Jasper_
If someone spends a week or two writing a patch and you come in and rewrite it
in an evening, that, in and of itself, is telling me something: You think your
teammate is a worse coder than you, given you were able to solve it with
"cleaner" code. You assumed that your solution was better, without talking to
the person who authored it to see if they did things that way for a reason.

This could have been solved with a "why did you do things this way", and maybe
you would have learned a bit about the thought process behind it, or maybe you
would have gotten "yeah this could probably be better, go ahead and clean it
up".

In a lot of cases, I definitely get the latter, but I always ask first,
because if they had a reason to do things a certain way, they probably don't
want someone stomping on a feature they're actively working on.

~~~
awb
> If someone spends a week or two writing a patch and you come in and rewrite
> it in an evening, that, in and of itself, is telling me something: You think
> your teammate is a worse coder than you, given you were able to solve it
> with "cleaner" code. You assumed that your solution was better, without
> talking to the person who authored it to see if they did things that way for
> a reason.

The time they spent developing the solution is a sunk cost. The only thing
that should matter is whether the afternoon rewriting it is the most
productive use of your time.

Also, rewriting something is far easier than writing it from scratch. Just
because you can rewrite it quickly doesn't mean the original developer didn't
save you a bunch of time by letting you see a workable first pass.

~~~
Jasper_
The same gut reaction that causes you to go "ugh, crap code, I'm going to
rewrite" is probably similar for the other developer, but with your rewritten
version.

People tend to have an aversion to code they didn't write in general, and
actually reading and understanding existing code tends to be a rare skill.

> Also, rewriting something is far easier than writing it from scratch.

Given the amount of times I see companies rewrite their product with half the
features missing and more bugs than before, I'm not sure I agree with that.

------
beaker52
I dislike the panning of "clean code" in the article because I really think
that duplication can be clean as shown. Clean code has a quality of
comprehension, not simply a lack of duplication.

The example given is pretty typical actually - where you can frame the problem
in a way that your solution ends up without duplication yet the solution
requires mental gymnastics to get your head around. We've all been there in
the pursuit of the perfect code.

"Write code that immediately makes sense to someone with half your smarts" is
a far better guideline than "don't repeat yourself".

~~~
erikpukinskis
I disagree strongly. Raw code, that anyone who knows the language can read...
is readable!

The OP’s code, which added an invisible abstraction, is “clean” I guess. It’s
less repetitive anyway. But it’s not readable. You need to go read the other
file to be able to read this one.

It’s like a clean kitchen with everything behind cabinet doors. Yes, there’s
less to look at. But you can tell what’s happening without opening all the
doors.

------
aazaa
> ... 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.

The article's starting example and revision clearly illustrate how the
original code was changed. Duplication was removed - just like so many
advocate for.

Unfortunately, the article doesn't state the changed requirement that broke
the new design, leaving that to the reader's imagination. And I can imagine
cases in which the revised code would be superior to the original.

It sounds like the problem arose from the need for specialized handles on
different shapes. If so, I don't see how using various createXHandle functions
(where "X" is an identifier associated with a specialized handle) would
necessarily cause convolution. It could become quite convoluted if not done
with care. For example, keep the createHandle method, but pass customization
parameters to it. But that's really the naive way to go.

What we'd be faced with is nothing more than the kind of specialization
requirement we usually see with Object Oriented programming. We can use
composition over inheritance to great effect there. We can extend the author's
refactored design and end up with something clean and maintainable.

------
whoisjuan
If there's something that I have learned about refactoring code that is
repetitive into "cleaner" shorter code, is that the refactored version looks
better but it's way harder to understand. When other people try to look at the
"cleaner" version they have to spend more time trying to understand it and
mentally untangle the abstraction.

I like syntactically shortcode as long as it's clear. I also understand that
sometimes shorter code has some small performance advantages that can add up
in languages like JavaScript where the size of the files can become a loading
speed bottleneck.

But to be honest it really bothers me when someone tries to make perfectly
fine and readable code into something different just to satisfy some weird
intellectual urge to make things more abstract.

Sometimes long code is not only easier to read and understand, but it also
helps to create a better technical outline of decisions that otherwise will
get lost in the reasoning of whoever is writing that code.

~~~
ben509
> But to be honest it really bothers me when someone tries to make perfectly
> fine and readable code into something different just to satisfy some weird
> intellectual urge to make things more abstract.

They obviously don't think it's "perfectly fine" or that their changes are
some "weird intellectual urge" or they wouldn't do it.

It's a subject for debate, and to make your case to that person, you need to
do better than vague claims that what you're doing is "fine" and their idea is
"weird."

~~~
whoisjuan
This is by no means a generalization. You’re right. Most people are coming
from a good place when they make suggestions that change your code. This
especially true for code reviews where I believe everyone has the same goal.

I’m referring to some isolated cases especially those that involve receiving a
codebase that was created by someone who is not working on it anymore. I think
this is where that strange urge to refactor everything kicks in for some
people.

------
on_and_off
Code that is easier to read == better code.

That's something I see most people struggling with, including myself, in
particular when learning a new language.

e.g. in kotlin it can be tempting to use a ton of complex operators and rely
on the intricacies of let vs run vs apply .. even if the resulting code takes
some thinking to understand.

At the beginning of my career, the code I was the proudest of was a gigantic
piece of code doing some very complex stuff. It took a lot of effort to
understand. Nowadays I am extremely happy when I take a complex problem and
solve it only by writing very simple code that even the most junior dev we
would hire can easily understand.

The downside I can see is that in some orgs more respect is given if you write
black box code making you unfireable but that's a good sign you don't want to
work there/need to make the culture evolve.

~~~
iforgotpassword
That often depends on how familiar you are with a code base, and your personal
knowledge about for example linear algebra or whatever specific thing is going
on somewhere.

Obviously there is no objective "right" in this matter, but always writing
code that the greatest idiot can understand obviously isn't good either.

Another problem with the duplication approach is that I'd you need to make
some change later on, you need to apply it to 12 places and are pretty much
guaranteed to mess up one of them, since this is the most boring task ever and
your brain switches to lie power auto pilot after the third instance.

~~~
sixstringtheory
Either you need to change 12 places and test those 12 (or whatever) use cases,
or make one change and still test those use cases. Only with the DRY function,
as the article says, it’ll wind up accumulating options and configuration to
accommodate all those use cases, becoming a big hairy abstraction that’s
harder to understand for any of those use cases, whereas with the duplicated
functions they would all evolve separately and in isolation and remain
independently simpler.

------
blhack
Ive been writing a react application with my wife for the last couple of
months (something we started at the YC hackathon in November actually). It’s
been a few years since I’ve written any react, so it’s been nice having her
there to help me.

Maaaaaan...it’s like I’m learning how to program again. There is SO MUCH focus
on shorthand and abstractions and stuff, all seemingly in the name of being
“concise” that it produces code which to me is extremely difficult to read. It
feels like I’m doing everything “wrong” when I read the documentation, and
this would have been a lot harder without her there to pair program with.

Especially when I’m switching between golang on the backend (where I’m
comfortable) and react on the front, it highlights how much they seem like
totally opposite philosophies.

I’ll say this, which is probably mostly due to me growing up on python: code
should read like a description to the computer of what you are trying to do,
and even if somebody barely speaks the language you’re writing in, they should
still be able to read what you’re doing. This doesn’t just help others, it
helps you need less cognitive overhead when reading your own code back.

Saying the same thing with less characters rapidly approaches the point of
inverse return, and it seems like most of the recommendations in the JS world
right now just want to keep pushing us further and further into that
inversion.

~~~
madeofpalk
What are some examples of React's focus on shorthand and abstraction? React is
fairly small and doesn't really encourage much at all. The one 'battle' that I
often find myself in is "should this be a seperate component?" but that's more
of a people problem and something that every language and framework will have.

~~~
blhack
Instead of writing

    
    
         <ComponentName someVariable={true}>
    

It’s just

    
    
         <ComponentName someVariable>
    

Of course there might be some completely valid reason for this, but it’s
baffling to me why you would want this type of shorthand, instead of just
explicitly writing what you mean.

Of course in golang this could be something like:

    
    
        var someVariable  //is false
    
        someFunction(someVariable)
    

But I would (personally) not write code like this if I could avoid it. It
would be:

    
    
        someVariable := false
    
        someFunction(someVariable)
    

It’s a little bit longer, but imo takes slightly less mental overhead to read.

~~~
madeofpalk
It's a shorthand, and its similar to what HTML does.

If you don't like the optional shorthand, don't use it? I don't understand how
this is something exclusive to React, or something it specifically encourages.

~~~
blhack
Yeah you might be right, and it definitely possible this is common and I’m
just weird for not liking it.

~~~
christophilus
It’s common, but you’re not weird. I’ve done a lot of React and front end
programming, and I hate this pattern, too.

------
BeetleB
The first thing I did when I read this was come to the HN comments and search
for people complaining about DRY:

[https://news.ycombinator.com/item?id=22022599](https://news.ycombinator.com/item?id=22022599)

[https://news.ycombinator.com/item?id=22022842](https://news.ycombinator.com/item?id=22022842)

[https://news.ycombinator.com/item?id=22022896](https://news.ycombinator.com/item?id=22022896)

[https://news.ycombinator.com/item?id=22022836](https://news.ycombinator.com/item?id=22022836)

The author of this piece was not engaging in a DRY activity even if he thought
he was. He (perhaps unwittingly) admits to it himself:

> My code traded the ability to change requirements for reduced duplication,
> and it was not a good trade.

The acronym DRY was coined in _The Pragmatic Programmer_ , and the author of
that book makes it clear that DRY is about not repeating _requirements_ in
code. You definitely don't want to deduplicate similar code if it involves
_multiple_ requirements because then you entangle yourself, which is what the
author of this piece did.

~~~
leoc
Wasn't it the case that the requirements were the same at the time the code
was written? TextBlock.resizeTopLeft(...) and Rectangle.resizeTopLeft(...) did
the same thing for the same reason, not by coincidence. The issue was the
possiblity (and eventually the reality) of _future_ divergence.

~~~
thefifthsetpin
The issue wasn't divergence. The issue was that there was no requirement that
TextBlocks should behave like rectangles.

It's very possible that DRY could apply even where divergence is expected.
Suppose we had a requirement that "A user should not be able to resize a
TextBlock if they are not allowed to read the text." At the outset, the read &
resize permission logic might be identical & DRY is easy to apply. Nobody
would be surprised if eventually resizing permissions became more restrictive.
Even at that point, DRY applies. If you repeated the shared logic, and later
you add new restrictions to the read permissions, it'd be easy to introduce a
bug where you forget to restrict resizing permissions accordingly.

~~~
mattkrause
> The issue was that there was no requirement that TextBlocks should behave
> like rectangles.

Sure, though that’s not quite what the new code did.

Instead, it added a new abstraction (createBox), and then used that to
incidentally assign the same behavior to TextBlocks and Rectangles. Your
change could easily be accomplished by changing the last line to check the
permissions and call createBox or something else, depending. I’d guess that
createBox would also be useful for many other shapes, but if you wanted to do
something totally different, like a star with handles at each point, you could
just....not call createBox, and write a totally independent “let star()”
function that does its own thing; it doesn’t seem like a base class
constructor is forcing you to call createBox or anything.

So....this code doesn’t actually seem that bad to me. What am I missing?

------
smacktoward
My rule is: if I find myself sorting things into two buckets, one of which
I’ve given a name that obviously means “good” and the other “bad,” that’s a
sign I’m deciding emotionally rather than rationally. It’s time to take a step
back and think about the distinctions between the things in the buckets
harder.

Sorting code into “clean” and “dirty” buckets is a good example of this. Both
bucket names are completely subjective, with “clean” obviously meaning good
and “dirty” bad. As the article indicates, dig in a little deeper and it’s not
hard to find objective ways the “dirty” code would actually be preferable.

------
Insanity
I agree that not every "smart approach" is worth it if you sacrifice
legibility.

But I don't think you necessarily need to ask permission to refactor code. On
the projects I had the past couple of years everyone understood that code was
open to be changed by anyone.

In practice we'd often ask "why did you do X" instead of just rewriting it.

I trusted the people I worked with on those projects though and if they
thought they had to rewrite part of my code for whatever reason, that was fine
by me. And vice versa, I changed their code and that was fine.

~~~
scarface74
Well in his case, did refactoring the code to clean it up add business value?

There has to be a very good reason for me to refactor existing/working code
for instance for performance.

I won’t refactor code to reduce existing duplication but I will refactor code
if I see there is some functionality that I need elsewhere so I won’t just
copy and paste.

~~~
Nullabillity
> Well in his case, did refactoring the code to clean it up add business
> value?

Yes, it made future modifications easier and discouraged adding special-case
behaviour.

And after reverting the change they did apparently fall into that trap.

~~~
scarface74
Well, in that case, why do it now instead of waiting until “the future”?

If it’s needed in the future, the cost is not more than it is now. If it isn’t
needed in the future, then he’s wasted his time. If something similar is
needed, but it needs to be refactored in a different way, they are going to
have to refactor it

There is no special case “trap”. Business requirements necessitated changes.

~~~
Nullabillity
> Well, in that case, why do it now instead of waiting until “the future”? If
> it’s needed in the future, the cost is not more than it is now.

They're familiar with the code and its gotchas when it was written. In "the
future" most of that context will be gone (people forget stuff, and change
companies/teams/roles).

Better to get rid of the gotchas before you get burned.

> If it isn’t needed in the future, then he’s wasted his time.

The new version was still more readable to newcomers, and resizing stuff is
pretty core functionality for a drawing program.

> If something similar is needed, but it needs to be refactored in a different
> way, they are going to have to refactor it

A second pass of refactoring tends to be easier than a big ball of mud. And in
the worst case, they can always inline it and start over from the same state.

> There is no special case “trap”. Business requirements necessitated changes.

When requirements change you can either start adding special cases or rethink
how it fits into the bigger picture. That matters in every layer, but it's
especially important in the UI layer.

Unless you want to end up with an unusable Apple-style hodgepodge. In that
case, go wild, I guess.

------
preommr
Clickbait title.

This is a story about:

1) Changing someone else's code without discussing it with them. Seriously,
wtf?! Even if the code is better maybe there are reasons the code is the way
it is and the it's not worth making the changes for the overall progress of
the business.

2) Misjudging the requirements. If those modules needed changes, then they
should've been left alone. This story could've easily gone the other way with
the author competently abstracting away messy details for an overall positive
impact. This happens quite frequently, but obviously doesn't make for a good
article because it's expected.

~~~
bagacrap
And a story about not having a code review process prior to check-in. How is a
team of developers ever going to work together if they all write code
completely on their own with zero input from or approval by others? That's why
style guides exist.

------
hyko
You are not saying goodbye to clean code: you are embracing it!

“Clean code” is a semantically overloaded term, so discussion about it is
inevitably mired in confusion. Every brain that first hears the phrase “clean
code” will construct some plausible placeholder definition, and the most
readily available one by analogy is “less source code”.

You can easily try and apply principles such as SOLID and then go on to
produce an unmaintainable web of nonsense, because they are meant to be facets
of a unified organising principle and not a bag of tricks for local
optimisation towards the state of “clean code”. I’ve experienced that
firsthand.

“Clean code” is great marketing, but I almost wish Robert Martin had called
his theories “Martin Code” so there was no doubt what is under discussion.

------
nathell
I can't help thinking of this excerpt from re-frame's docs:

> Now, you think and design abstractly for a living, and that repetition will
> feel uncomfortable. It will call to you like a Siren: "refaaaaactoooor
> meeeee". "Maaaake it DRYYYY". So here's my tip: tie yourself to the mast and
> sail on. That repetition is good. It is serving a purpose. Just sail on.

[https://github.com/Day8/re-
frame/blob/master/docs/Subscripti...](https://github.com/Day8/re-
frame/blob/master/docs/SubscriptionsCleanup.md#a-final-faq)

~~~
Noumenon72
Thanks for the link. Best as I can tell from the ClojureScript, this is in
reference to a refactoring that would cross layer boundaries. Like taking the
methods

    
    
        findItems(db)
        findWidgets(db)
    

and refactoring to

    
    
        findAnything(db, items_tablename)
        findAnything(db, widgets_tablename)
    

So the repetition is good because it's a data access layer and you shouldn't
know the storage details, not because repetition is always good.

------
nojvek
1) Always do pull requests. No change should make it into a master without
someone else’s eyes. Unless it’s a “get out of trouble” surgical revert that
really needs to go out in an emergency.

This would have given the author to raise concerns about very duplicated code.
We do that all the time with comments like “this could be a bit DRY-er, how
about moving this bit to a common place with this other bit in the codebase
that also does that”

2) when the author refactored code, he should have deffo asked for OG to
review his code. May be his duplication didn’t account for edge cases.

Reviews are great. Make reviews a part of the system. Small, incremental
reviewed code, with decent coverage, being shipped multiple times a day by CI,
reacting to customer needs is how the best engineering orgs are operating.

He is right about some parts of over accounting for the future. I’ve had that
happen to me multiple times.

Same with doing large refactor s of other people’s code. It’s not about code
anymore, you gotta ensure they feel good about it too. Esp junior engineers,
it has to feel that it’s still their work.

------
fyp
Why must the messy code win in this case?

Building a transform tool should be a pretty well understood problem. Just
take a look at what photoshop's transform tool does and you will see all the
possible extensions you might need to support in the future (rotate, skew,
distort, perspective, warp, transform origins, a bajillion modifier hotkeys,
etc).

Of course don't go ham and support all of them up front. But with those future
use cases in mind, it's pretty hard to write yourself into a corner. I would
say Dan's refactoring looks fine other than needing more customization for
positioning and handle behavior. But it seems easy enough to just add them
later.

------
29athrowaway
> Firstly, I didn’t talk to the person who wrote it. I rewrote the code and
> checked it in without their input.

If you want to modify a method that has 10 unique contributors. Do you really
need to talk to different 10 people to maintain to make a change? That does
not sound very effective.

And, most importantly: when you code as a job, all your deliverables are
company's property. They are not yours. The company can do whatever they want
with them, they don't need your opinion or approval. If they want to replace
every piece of code that you ever submitted with an ASCII clown, then print
the source code make a pinata from it, they can do that too if they want.

Secondly: version control. If you want the old version of some code, you can
retrieve from version control right? You can leave a comment such as: "for a
verbose version of this method, see revision <sha1>".

~~~
flurdy
> > Firstly, I didn’t talk to the person who wrote it. I rewrote the code and
> checked it in without their input.

> Do you really need to talk to different 10 people to maintain to make a
> change?

No, you don't talk to all of the 10 contributors. But if you want to still
work as a team you should talk to a few. Depending on the size of the change.

Some or even all of the previous contributors may no longer be at the company
or are inaccessible. But a super quick discussion with 1-3 other team members
should float if this is a good idea or not.

Obviously, if you practise pair programming, most smaller rewrites just need a
consensus within the pair, and a more extensive change may be a good idea to
get the approval of another pair, especially if some other team members were
the original contributors.

There is no need to be precious about any existing code. Nor any need to be a
bull-in-china-shop either.

~~~
29athrowaway
For quick changes, you can simply submit your changes in a pull request and
assign them as reviewers.

For non-quick changes (that are worth the cost), you can have a discussion
about it and evaluate pros and cons.

------
quickthrower2
In my experience: don’t worry about duplication. It’s sometimes OK and
sometimes a Symptom. Don’t fix the Symptom, fix the main cause:

Classes or functions that do more than one thing. The definition of one thing
is one conceptual thing.

For example I’ve seen DTO objects with both wire concerns and Ui concerns.
Splitting that out made a lot of code simpler and incidentally deduped some
stuff.

So attack poor abstractions, not duplication. The S.O.L.I.D principle is a
good start (even for non OO code)

If you are unsure, I’d always err on keeping the code as is, you can always
refactor it tomorrow when you understand more. Instead: make sure the unit
test coverage is adequate.

------
franciscop
This should rightfully be on the front-page of HN. I've gone through something
almost the same as Dan Abramov here, and I feel it's difficult to impossible
to explain this and you have to experience it yourself.

The tricky bit is where to draw the line, which I've learned only after
pouring thousands of lines of code and I understand is a bit different for
everybody. The rule of three[1] was a very useful rule of thumb in the
beginning, but there are many secondary variables that I unconsciously use to
decide when to remove repetition. e.g.:

\- Single file tends to be split _less often_ , since refactoring it is
easier. Example: the single file routing in React being repetitive is okay,
but if there's multiple files with routers and custom logic I'd consider a
helper a lot stronger.

\- Conceptually straightforward APIs tend to be split _more often_ , since
it's easy to package and reason about, as well as design. Examples: cookies,
warnings, kv stores, etc.

\- Early stage projects tend to be split _less often_ , since few things are
not yet clear and being able to put everything in your head is a lot more
important.

It's also one of the lessons that you should learn going to senior. To the
extreme, someone insisting in removing this kind of duplication for the sake
of it is often a sign of a junior dev (as in, because it's _wrong_ and not
trying to understand the codebase/tradeoffs first).

[1] [https://blog.codinghorror.com/rule-of-
three/](https://blog.codinghorror.com/rule-of-three/)

~~~
bsaul
The more time passes, the more i consider coding to have a strong aesthetic
component. What makes it complex is that there are two kind of "aesthetics":

\- the code itself

\- the abstractions that the code represents.

You can have very "clean looking" code (short functions, short files, no
repetition, etc. ) that is in fact modeling a problem in the most convoluted
way. And the other way around : a bit of repetition, but the concepts behind
are completely obvious.

And most of the time, nothing falls completely into one category, and the way
you'll decide where to draw the line is almost a matter of taste.

------
boffinism
I think this highlights an important nuance to the Don't Repeat Yourself
maxim. It's not just about whether two bits of code are similar now. It's
about whether, when they change in future, they are likely to change in the
same ways. If not, then DRYing up the code now is only making more work for
yourself down the line.

~~~
erik_seaberg
I've always found the
[https://en.wikipedia.org/wiki/Open%E2%80%93closed_principle](https://en.wikipedia.org/wiki/Open%E2%80%93closed_principle),
where you derive from classes instead of editing them in-place, sort of
appealing for this reason.

~~~
terandle
What is this implying? That instead of adding a property/method to a class you
should instead create a child class that inherits from the original with the
new properties? Sounds like a great way to have a very complicated class
hierarchy IMO

~~~
erik_seaberg
Exactly that. Just because _you_ want new behavior doesn't mean all the
_other_ consumers of that class also want it. The old behavior should still
exist, and have a name. I'm not sure whether the idea was ever fully developed
into patterns for replacing deep hierarchies with simplified classes that
factor out some inheritance from otherwise-unused base classes.

------
MattyRad
There are at least 6 giant and contentious subjects covered in this one, short
article:

\- Code duplication (when it's appropriate)

\- Code cosmetics/legibility

\- Code scalability/flexibility

\- Code review (protocols, how to perform them)

\- Accepted widespread coding wisdom (abstractions, and shirking them, in this
case)

\- What defines a "junior" engineer (there are wildly different perspectives
in this thread)

\- Common courtesy (coworker interaction, etc)

Almost all out these are highly circumstantial to the type of project,
codebase, language, culture, etc. I would like to see a productive
conversation regarding any of these topics, but discussing so much at once in
such circumstantial ways just leaves us talking past each other (the threads
here are pretty clear evidence of that). I don't think this article has enough
substance to cover any of these topics in any depth.

It's like if someone asked "What vehicle is the best vehicle?" Some people
chime in with how much they like their brand of vehicle. Others say how trucks
are better then cars. Others say that electric is the only choice. Others say
cars made after 2010 are the best. The question is too broad so the responses
aren't effective.

------
Ace17
_Code is not clean : code is cleanable. What is clean today might not be clean
tomorrow_ (Robert C. Martin, author of the book "Clean Code")

Trying to evaluate code cleanliness in isolation is a waste of time ; as it
strongly depends on the way your requirements evolve (but it's not
subjective). You only need abstraction points at the frontiers between parts
of your requirements that change at different times.

 _Duplication is better than a wrong abstraction_ (Sandi Metz, author of
Practical Object Oriented Design in Ruby).

Trying to religiously factorize every structural similarity you can find, for
the sake of the almighty DRY principle, without taking into account how
business requirements evolve, is counterproductive.

------
avip
Ignored throughout comments insofar is the _fact_ [®™|©] that LOC is a
material which has actual weight and cost.

Code is being read, reviewed, commented, linted, compiled, test-covered, read
again, contemplated, diffed, documented in weird diagrams, mentioned in
READMEs, revisioned. More LOC = more places for dust and rot to accumulate and
more places for bugs to hide and stick. LOC is the environment under which
bugs prosper.

So having less code is a legit, noble, and to some extent practical state to
strive for.

A smaller house is faster to clean.

------
Havoc
I feel the code aspect could have gone either way.

It's the human part that went sideways. Egos are fragile & this is essentially
"your code is bad and I can do it better".

I've been on the receiving side of this exact thing - also duplication.
Fortunately the dup was bad enough that I could see I was wrong.

------
luord
> Let clean code guide you. Then let it go.

Nah, no offense to the react fans but I'd rather listen to Fowler than to this
guy.

Sure, there are very specific exceptions where repeated code is an asset
instead of a liability (and many of them are, appropriately enough, when
dealing with graphics), but they are that: _very specific_. For 99% of
situations, we ought to follow the principles (obligatory IMO).

~~~
seattle_spring
Do you have specific experience that suggests that hyper-generalizing code
before you even know about multiple use cases is beneficial over carefully
extracting shared logic only when needed?

If I to pick one single practice junior engineers employ that ultimately bites
everyone in the ass, it's a blind adherence to generic code and DRY at all
costs.

~~~
luord
> Do you have specific experience that suggests that hyper-generalizing code
> before you even know about multiple use cases is beneficial over carefully
> extracting shared logic only when needed?

I've gone over my comment three times now and I'm yet to see where I even
_implied_ something like this. Could you tell me how you got this from my
comment? I do want to see how I could send the wrong message so I can word
myself better in the future.

Just in case, one of the _principles_ I follow is "premature optimization is
the root of all evil".

~~~
seattle_spring
The article was literally about that, and your first line says "nah I'm good."

------
acvny
Wait. What? Goodbye clean code? Clean code obsession is a phase?

Your only mistake was that you didn't ask the committer to adjust their code,
you didn't discuss, but you went and changed their code and "pushed it to
master" :) This is your mistake. Nothing to do with clean code. And the title
is a very misleading, untrue click bait.

------
gfodor
Another similar anti-pattern is acting in a way that implies the computer
itself cares about such things. Other than efficiency concerns, the audience
for code is other programmers, and those programmers over time. The computer
doesn't care about your variable names, how DRY your code is, or even the
elegance of your data structures except if the ones you have chosen cause it
to run more or less slowly.

Another way to put it: if data is passed from one function to another in a way
that would make any programmer wince, but the odds of it ever being re-visited
by another programmer are near zero, and it affects nothing else, does it
matter? No, unless you think the computer itself winces as well.

------
jrs95
To me the #1 thing that fixes the social issues described in this post
(changing code someone checking in code a few hours after they pushed it) is a
code review process. The most valuable thing about always doing pull requests
and code review is sharing information and having discussion about changes,
not necessarily the immediate improvement of the code. A team will gain more
knowledge about what everyone is doing by reviewing each others pull requests
than they do from a daily standup by a pretty large margin in my opinion.

In this particular example, reviewing a PR would have meant that a discussion
about whether or not that abstraction was a good change _before_ the code was
ever merged.

------
wizzzzzy
One place I've found this to be especially true is when writing CSS. I went
through a phase years ago of trying to abstract any repeated styles into
'clever' oocss patterns but in the end it's often lead to a confusing mess, in
part because of the nature of CSS. In the end I've found duplicated and
verbose code to be so much easier to work with and maintain.

------
Iv
> I suggest to think deeply about what you mean when you say “clean” or
> “dirty”.

I'd go one step further: I suggest to think deeply. Period. I used to think I
was a slacker for taking one or two hours to go for a stroll during a work
day, thinking hard about my code, my problems, the architecture, the
abstractions... and daydreaming as well.

I had to shut down my inner Jiminy boss who was telling me that an hour with
no code written was an hour lost. And I am still occasionally bad at it.

In the last project I did, there was a strong deadline, so I went straight
into coding, went into two dead-end before backtracking and changing the
approach totally. Probably could have been saved by some hard thinking at the
beginning.

~~~
l0b0
Balancing thinking and doing is one of the Hard Problems™ of software
development. I suspect successful methodologies work because they formalize
the act of thinking about the code before implementing it. And that when
methodologies fail the reason could be that they can easily be implemented
such that people think more about the technicalities (assigning work,
deadlines, estimation accuracy etc.) than the development (modularity, overlap
with existing work, conflicts with concurrent work, security, third party
dependencies etc.).

------
DoubleGlazing
Someone once taught me a phrase I often use "Lowest common denominator
coding." What it means is that you should code to the most common abilities
held by the group of coders you are working with.

In practice what that means is when you are writing a piece of code you should
do so in a way that the new hire fresh out of university with less that a
years real world experience should be able grasp what you are doing fairly
quickly.

Abstractions, using the latest and greatest additions to the language you are
using and obsessing over reducing the code line count can help the code run
faster, but it also increases the learning curve for people looking at the
code for the first time.

------
lleolin
When I look at his example I still sense a compelling sense to create some
kind of abstraction. It's just that the example abstraction he chose to make
is somewhat esoteric and arguably over-solves the problem. The author isn't
suggesting a different, perhaps simpler abstraction, but forgoes abstractions
entirely.

~~~
jariel
I tend to agree.

To me, this is a good fit for some kind of 'fairly simple abstraction' \- even
the abstraction used is fine.

So long as everything worked, I think this might be a better solution.

I'm fine with 'don't just refactor because' but this looks like a fairly clean
cut opportunity.

Social considerations aside.

------
acjohnson55
I find myself wishing the question of whether to factor code out wasn't
binary. Like if you could transclude parameterized code. You'd get the
benefits of not having to keep a stack trace in your head, while still having
a canonical version of an abstraction. Like a macro, but always expanded in-
place.

------
kirstenbirgit
It's tiring to work with developers who have a very narrow focus on
"correctness" and e.g. code style. It can result in a lot of extra work, extra
refactoring, extra cleanup, extra rewrites, etc.

It's how you end up with a toolchain, build environment and release process
that's worthy of Netflix, but you're just 5 developers, and it takes up all
your time to maintain.

Always focus on how it's benefiting the business. Is my time better spent on
something my customers need or want, than fixing something that's already
working?

------
daxfohl
This reminds me of Wayne Gretzky's phrase "Skate to where the puck is going".

A really good coder will find clever ways to implement the requirements in the
most DRY fashion possible. And it will be perfect. Until the next requirement
comes along. This is skating to where the puck _is_.

At some point you pick up the knack to know where to just leave things dumb
and obvious as possible, because something inside you is shouting that there's
going to be some new requirement soon that breaks the abstraction you're
considering.

------
kevin1024
Duplication is far cheaper than the wrong abstraction.

[https://www.sandimetz.com/blog/2016/1/20/the-wrong-
abstracti...](https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstraction)

~~~
klingonopera
Yeah, until your count goes above two.

------
yason
These are all judgement calls. Sometimes it's good to refactor to avoid
duplication, sometimes it isn't. Only experience gives some hints at which way
to go each time.

If unsure, I tend to at least break down these cases down by writing low-level
building blocks and express the duplicated code in terms of those blocks.
Those blocks should be low-level enough that they can't have a say about how
they should be used in all these cases.

You could say it's basically abstracting out the stuff that's so small there's
no risk of overabstracting but that's not the point. Rather, it's like
building a language for expressing the kind of problems that are solved by
duplicate code all around.

I'm sure in the original example there would've been some things that are
common for resizing all shapes. You would still have repetition under
individual cases but you would replace raw math (I assume) with certain basic
operations that you know are common.

Of course, this isn't a generic solution either. Just another step between raw
duplication and finely abstracted model.

------
ryanwinchester
I don't use Ruby, and don't do OOP either, but my favourite talk is still
Sandi Metz's "All The Little Things" and I think that's where she says "prefer
duplication over the wrong abstraction." That's really changed me. I've since
been seeing DRY and other misapplied dogma in a new light and have grown much
over the years since.

~~~
xwowsersx
Great quote. Link to the talk? When you say you don't use OOP, are you saying
you use functional languages or you use imperative languages but don't do any
of the OOP design crap?

~~~
ryanwinchester
Been using mostly Elixir full time (w/ some JS here and there) for the last 3
years.

Talk link: [https://youtu.be/8bZh5LMaSmE](https://youtu.be/8bZh5LMaSmE)

~~~
xwowsersx
Thank you.

------
starky
It is interesting to see the parallels with my work with solid modeling in
CAD. I'm constantly harping on the people I work with to think about what they
are working on and structure it in a way that can be edited later (as well as
stability). This often means a larger/less efficient feature tree, but it is
actually maintainable. Just this week my insistence that we build the model to
be flexible meant that we were able to fix something in 5 minutes instead of
days.

Updating colleagues work without talking to them is something I often struggle
with though. On one hand I absolutely hate it when others change my work
without at least getting the history as to why I did it that way. Thus I try
my very best to give others the courtesy of discussing it with them first. On
the other hand, if something isn't being paid the proper attention, or they
aren't making progress, sometimes it is a useful way to light a fire under
certain people if asking politely hasn't worked.

------
rlonn
Have to push this fantastic article again - if you read it and follow what it
says you won't make these mistakes:
[https://programmingisterrible.com/post/139222674273/write-
co...](https://programmingisterrible.com/post/139222674273/write-code-that-is-
easy-to-delete-not-easy-to)

------
mattacular
"First you learn the value of abstraction, then you learn the cost of
abstraction, then you're ready to engineer"

------
bfung
Did the colleagues code go through code review before it was accepted?

And obviously, there wasn’t a review for the refactored code before check-in.

Use a code review system, would e avoided a lot of this in the first place.

------
valand
> Rewriting your teammate’s code without a discussion is a huge blow to your
> ability to effectively collaborate on a codebase together.

Debatable. Detachment to one's code is as important as being passionate in
delivering requirements and enabling people with your code. A better approach:
Anyone should objectively weigh the effect of a change whether it is
communicated or not. If it turns out better, the change should be allowed for
the sake of delivery, improvement, and knowledge sharing.

> My code traded the ability to change requirements for reduced duplication,
> and it was not a good trade.

An excellent point from danabramov. Premature abstraction is an ugly creature
waiting to ambush you couple months in the future. By that time you'll forget
why you did it in the first place and have written a lot on top of that
abstraction.

I've been through a phase where juniors and intermediates (a lot of them) are
obsessed with abstracting duplicated codes (not their fault, they were at the
wrong place at the wrong time while receiving the principle that abstraction
is always good).

I meditated for a while on it and found a principle to avoid this issue:

Any abstraction MUST be designed to be as close as possible to be language-
primitive-like. Language primitives are reliable, predictable, and non-
breaking. If they do, they don't affect business logic written on top of it.
If parameters are added, defaults are provided. They don't just abstract, they
enable developers to express business logic more elegantly.

The challenge is to pick the part of the code abstractable to be primitive-
like first and make it a top priority.

This is why language features like Rust async, Go channel-based comm, ES6, C++
smart pointer was such hype in their time and is used up until now. It also
applies to enabling tools such as React, tokio, wasm-bindgen, express,
TypeScript, jquery (even this which is not a thing anymore).

------
itchy
The problem is not abstractions or clean code, it is the lack of experience,
lack of time to think and contradicting views of fellow developers in the
team. I code for my own projects now, what a joy that is compared to working
in a team for company x. Goodbye teams and good luck with your perfect
codebases!

------
dboreham
I never had this religion imposed on me in my career, except recently.
Originally I designed hardware. There, if you change something it might cost
$1m in retooling a factory or throwing away a defective wafer batch. Then, I
worked for years in teams producing production code, where job #1 was to not
break something. Pretty soon you learned that even an innocuous tidying up
change can end up breaking functionality seriously. You also learn that if
code is not pretty that probably means paying users are relying on it.

I think the culture of turd polishing code so it looks pretty comes from
academia and people who are control freaks. That, plus only deployment where
if you screw something up you can just push a fix in seconds. Except if your
bad but pretty code leaked sensitive data or lost data..

------
abvdasker
"Duplication is far cheaper than the wrong abstraction." — Sandi Metz
([https://youtu.be/8bZh5LMaSmE?t=891](https://youtu.be/8bZh5LMaSmE?t=891))

This talk by her is one of my favorites ever. Her ideas definitely made me a
better programmer.

------
Camillo
The interesting part here would have been the actual code for each case, and
the changes that it required later, i.e. exactly what the article does _not_
contain. Without it, we have to take the guy at his word, and it's hard to say
we've learned anything.

For example, in any vector graphics program I've seen, an oval is defined by
the surrounding box. The box defining an oval works exactly the same way as
the box defining a rectangle, it makes sense for it to be that way, and it is
also what users expect. It's hard to believe that they had a good reason to
implement a different resizing behavior for ovals.

------
Traster
I think this is a great post, it covers a topic succinctly and agree with its
conclusion.

On the topic of actually refactoring code I think we should consider the code
as a variable - that is to say, sometimes these variables just happen to equal
each other in which case they are two separate things. Sometimes two variables
aren't just equal, but they're the same. That's when to factor out the code.
Otherwise the second these two variables are no longer equal you end up in
trouble. The secret is divining when something is _for all intents and
purposes_ the same as something else in this specific situation.

------
Tade0
Weird that there's no mention of writing tests.

After all, they help in explaining the intent behind the given piece of code.

With well written tests even the most "clever" implementations become at least
_reasonably understandable_.

Anyway, here's what I do to deal with this problem:

1\. Write code (and tests).

2\. Switch to some other task.

3\. Forget about that previous piece.

4\. Get back to it and judge if it's still readable.

If something is readable then it's likely to be modifiable as well.

One classic example of things that I learned to never de-duplicate is the
routing configuration. In this instance anything that isn't an explicit list
of URLs is usually too "clever" to be readable.

~~~
latchkey
Agreed. Tests often have the benefit of defining better interfaces, so the
implementation details often matter less and code duplication often works
itself out on its own.

------
huntleydavis
From years of schooling and artificially constructed engineering problems
we've gotten used to, for some addicted to, the idea of end solutions fitting
neatly into a clean edged box. We've grown up on complex physics problem with
a clean integer solution, homes with clean lines and polished finishes, and
engineering that's constantly pushing for smaller and faster solutions. So,
naturally we want our code bases to reflect our experiences : concise, clever,
code that feels like a clean integer solution. And while maybe it should be a
common aspiration to write code that's 'clean, the reality of the natural
world is that solutions to some of the most complex problems are not 'clean'.
Furthermore, rarely is the true end goal of the code you're writing to be the
'final state' of logic. The code that we write is almost always for ever
evolving and expanding use-cases; we are creating scaffolding rarely a
'finished' product. Furthermore, most of us are writing code in ever evolving
languages so even 'finished' state code will ultimately be paved over with
something more concise and efficient as the language evolves. The point being
: there's a happy medium ( I haven't mastered it ) of reconciling with the
realities of complex evolving solutions and our internal desire for clean
elegance in our codebases.

------
JBSay
I find this problem to be overblown. I've met dozens of dirty coders who
happily copy-paste code without a single thought for maintainability and their
colleagues. I've yet to meet a single clean code zealot.

It's impossible to tell from this example (because no actual code is included)
whether refactoring the code was a bad idea or whether it was this specific
implementation which was wrong. Clearly the math should sit in a
geometry/linear algebra libraries and nowhere near the presentation layers.

------
awinter-py
IMO line count is an important metric

if two companies are broadly similar but one gets the job done with 10x less
code, they're probably in a better survival position

not all logic _should_ be reusable or super generic, but there's a size level
at which avoiding abstractions becomes toxic to further growth

there's a good article floating around somewhere about things you learn at 1k,
10k, 100k-line codebases and how your philosophy changes

with a million caveats, of course, size and maintainability are going to be
correlated

------
keithnz
Clean Code sounds like a virtue, but it wasn't ever supposed to be a primary
goal, it's more about disciplined with the code you write. Architecture and
Design are more fundamental.

Removing duplication is most often about Design. Very easy to introduce
unintended coupling if the only motivation is to reduce duplication. Coupling,
Cohesion, Composition, and Abstraction are generally more important concerns
than clean code. Sometimes simply by trying to remove duplication you find a
better design, but sometimes you don't, sometimes you tangle your design with
couplings that don't really make sense. However, after your higher concerns
are taken care of, you should keep the code clean.

As a sidenote, what the author did by going to extremes and then backing out
of it is actually a really good exercise in learning some of this stuff. We
are fond of our "rules" for producing good software, they often encapsulate
key insights. But there are no real rules, there's just the vast interacting
world of these insights and how they play off against each other and how we
socially interact with others with often similar but slightly different
insights to create software as groups. It can be done many ways and should be
a constantly evolving thing.

------
GordonS
As usual with code principles/guidelines, it's about knowing when to apply
them, and _how much_ to apply them. In the particular scenario discussed in
the article, I think the better option would have been something in between.

I think most devs go through learning phases with code guidelines, code
patterns and the like, when they first hear about them, actively trying to
crowbar them into every line of code, even if it results in more complex code
- because they treat these things as _laws_ , rather than guidelines.

IME, good devs eventually grow out of this, and from _experience_ are much
better able to apply guidelines and patterns when it's "suitable" to do so.

This problem is sometimes exacerbated by management, who like to focus on
metrics. For example, a project I recently worked on used SAST (SonarCloud),
with all sorts of arbitrary restrictions enabled: less than 3% duplicated
code, minimum 90% test coverage etc. Predictably, it helped make the code more
complex than it needed to be, and led to mock-heavy unit tests that didn't
seem to actually _test_ anything - they existed only to satisfy SonarCloud
(and ergo, management).

------
Dirak
I would even take your learnings from your experience a step further and say
that one should be proactive in talking to teammates and exploring ideas on
how to improve the codebase if you think there is room for improvement. For
instance, it may well be that inheritance is the wrong pattern for that given
codebase, but there are indications that the current architecture isn't
scalable.

With more investment, more research could have been put into looking at better
suited architectures such as the Entity Component System architecture, where
Entities such as Rects and Ellipsis are composed of units of functionality
(position, resizability, etc), which is a proven architecture for this sort of
application. Then, implementation would be a matter of getting team buy-in /
weighing the refactor pros and cons with your team.

Tangential, but Entity Component Systems are a great way to structure systems
with many types of entities that share subsets of functionalities.
[https://kyren.github.io/2018/09/14/rustconf-
talk.html](https://kyren.github.io/2018/09/14/rustconf-talk.html).

------
ummonk
I really get annoyed by the lack of good tooling to deal with abstraction. I
wish there were a way to view abstracted code with function calls (to some
depth) inlined and macros applied. It would solve a lot of the difficulties
with heavily abstracted code while preserving the advantages.

 _> Secondly, nothing is free. 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._

The opposite is also often true though. If the requirements had changed in a
uniform way across the different shapes, then it would have been much easier
to apply the modifications to the abstracted code. The trick is to have an
idea of what kinds of modifications are likely in the future, and when you're
not sure yet, avoid premature abstraction but ensure things aren't so ad hoc
you'll have trouble abstracting in the future.

------
runninganyways
Clean code is not a phase. That's an obnoxious thing said by non-programming
amateurs who find themselves in positions of authority.

------
kuon
If you really want to remove duplication, try the other way first, like this:

where there is

// 10 repetitive lines of math

write a few helper functions, like

// 3 repetitive lines of math calling my helper functions

I don't known what math was in this case, but removing duplication should
preferably handled in this direction to avoid introducing more abstraction and
in this case keep a full separation of concerns.

------
kerkeslager
I think the author got the wrong message from this situation. Yes, the
refactor is bad, but I think they misidentified WHY it is bad.

> 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. 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.

Yes, but that's a social problem not a code problem. It doesn't mean your code
was wrong, it means the process by which you integrated that code was wrong.

> Secondly, nothing is free. 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.

It's telling that there isn't a code example for these "special cases". My
guess is that allowing them as configuration points in the de-duplicated calls
would not have been that complicated.

I have heard this "problem" brought up before, and in my experience, it's not
nearly the problem people claim it is. Yes, there is a balance to be struck:
you don't want to remove duplication before you understand what's really
duplicated, hence the rule of three (and I'm even fine going well beyond
three). But I've yet to work on a codebase that wasn't well on the "too much
duplication" side of that.

The REAL problem I see with the refactor is that the new data structures don't
correspond to business objects, the way the user thinks about them.

------
sebastianconcpt
The OP is using _clean_ as a language figure. He attacks the idea of _clean
code_ but not as the hygiene of the writing but as in removing what he thought
to be _unnecessary repetitiveness_ only to later come to the conclusion that
it was adequate yet somehow repetitive.

Why repetition is the opposite of cleaneness in the first place?

What he calls here clean code (and dirty code) is modelling that piece of
software with or without repetition and _not_ legibility (which would be a
more reasonable opposite concept of "clean code").

He reflects on one thing right tho, the idea that he jumped into abstractions
too soon, hence he refactored removing repetitions but injecting a model that
was inadequately modelling what was needed in the project in the first place.
And worst, he did that without discussing design in advance with any colleague
first (that was the biggest mistake IMHO) loosing time and efforts for all
involved parts.

So the real underlying issue was that code repetition blinded him of design
priorities. Nothing to do with the ability to do abstractions and write clean
code on top those abstractions. If you do clean code in the wrong abstractions
cleanness (or its opposite) will be irrelevant.

But a huge problem is that he again (with his article itself), jumps too soon
into the wrong conclusions: attacking the skill to do abstractions and the
skill to write clean code. Readers will be induced to confuse the real thing
for the language figures he is forcing with that text.

The skill of imagining good abstractions for general concepts and writting
well are things that go way beyond writing software code (all professions
needs these), hence attacking them makes no sense at all and a successful
attack on them would only promote some degree of general confusion (including
areas beyond those directly affecting the professional career).

Back to the anecdote, what would have been good?

1\. Prioritize where you want the flexibility and power of the design _first_
and care about luxury details like "code repetition" _later_.

2\. Discuss with colleagues in advance (specially those who will review your
merge requests) to agree on what's to be done and what's going to change.

3\. Actually implement the changes and open MR.

PS: in favor of the OP, he was generous in sharing his experience so others
can learn from it.

~~~
zmmmmm
> Why repetition is the opposite of cleaneness in the first place?

It's an idiom after the book by Robert Martin, whose central theme sits around
heavily refactoring code to remove repetition and in fact even non-repetitive
code. The first few chapters hammer the concept that functions should be
extremely short and aggressively refactored to compose them into subfunctions.
It essentially coined the concept of "clean" code in these terms.

~~~
sebastianconcpt
Good clarification! Thanks! I didn't read that one, although, I generally like
Rober Martin, I really don't like when language figures go too far.

If pushed one bit too far it becomes propaganda in a culture war instead of a
healthy intellectual honest discussion. What we know as flamewars is an
example of that.

------
gvjddbnvdrbv
His changes did increase the cohesion in the codebase BUT they also increased
the coupling. Ideally you minimize coupling and maximize cohesion.

The book Clean Code specifically addresses this area and his final conclusion
is compatible with the concepts of clean code.

I think devs should read Clean Code multiple times during their professional
life.

------
darkbatman
Nevertheless, I used to work with someone in a startup, who was crazy about
over cleaning the code. Every time there was something that has to be cleaned
and when requirements changes or new feature is needed we had to do all the
fancy stuff again because of the over cleaned code that wasn't flexible
enough.

------
crazy5sheep
I don't agree the issues OP later discovered has anything related to
`refactoring` itself, but more a issue of premature optimization.

my 2 cents, an interface is defined, it shall not be modified for no good
reason. Even if you do, you can still have some way to make sure it can be
compatible with the original system. and I don't believe you can't extract
some common behaviors of those repeated code, and use them within the
interface, refactoring doesn't mean you have to rewrite the whole project, it
can be done by piece by piece. reducing a line of duplicated code can save you
a lot of efforts on maintaining the project in its life-cycle. a lot of times,
I have seen a code change was made to fix some bugs were forgotten in other
place which duplicated the same original code.

------
speedplane
The examples here pale in comparison to what I see. Rather than dealing with
"ugly" code, I often come across incorrect code. For example:

    
    
      function do_add(a, b) {
        return a - b;
      }
    

This is an obvious mistake, one would think that the solution would be to
simply fix the do_add function, but that will end up breaking a tons of other
stuff that relies on the broken functionality, and instead what you get is
this:

    
    
      function do_add_real(a, b) { 
        return a + b;
      }
    

Yes, seeing this in production code is infuriating. But simply "fixing" the
issue is generally not possible.

A sign of a seasoned developer is to see this type of horrible code, take a
calm breath and focus on the issue at hand rather than scrambling to fix
something that works but is ugly.

------
shultays
I worked with someone that considered code repetition as a force of evil that
needs to be fight at all costs. Some of his refactors were valid, other not so
much and made everything more awkwars to work and more resiatant to change. It
wasnt a pleasent experience.

------
astrobe_
> dragging each handle in different directions affected the shape’s position
> and size in a different way

The cause of the duplication itself is probably wrong: why would you do it in
different ways? Treating ovals and rectangles the same way is simpler for the
users.

> My boss invited me for a one-on-one chat where they politely asked me to
> revert my change

No justification given here? Is it because the boss did not give any, or is
something omitted?

> Once we learn how to create abstractions, it is tempting to get high on that
> ability.

People should start forbidding themselves to use that "abstraction" word for
anything. That's just code factoring.

> 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

It seems here that this was taken as a hostile overwrite. Nothing to do with
"abstractions", "clean code" or factoring.

The problem here is not "building trust", the problem is that they did again
something that was already done without discussing the issue they see in it
before doing something about it.

This is a _methodological_ , not _social_ , mistake.

> How exactly do they affect the way the code is written and modified?

The usual FUD, the actual trap many people fall in. You cannot predict the
future. You should not try to predict the future, that's not your job.

I understand some may be confused by the fact the etymology of "programming"
means "write before" or "write ahead of time", but the meaning is that we
write what something is going to do; we are not actually writing what will
happen for things that are not under our direct control.

> Let clean code guide you. Then let it go.

No. What they call "clean code" is doing the right thing at a specific time.
If the requirements change, then do the right thing again. That's called code
maintenance.

------
kunglao
1\. If you broke a code, don't generalise it as an inherent problem of clean
code. Just find what exactly you did wrong. 2\. If the only reason you
refactored it was because you felt it's not clean, don't do it - that code
doesn't look terrible to me anyway. 3\. To do this kind of refactoring you
need good test coverage. So, start there. This also makes sure that you
understand the specs well enough to refactor. 4\. Clean code is good. It's a
skill to know what is good code and what is bad code, what is better code and
what is worse code and when to give in on clean code and compromise. Practice
it.

------
monksy
To me this sounds like he gave up on producing something with craftsmanship.
It sounds like he's getting bullied by someone of lessor experience.

If the code was blainently duplicated, that's going to create problems later.

~~~
danabramov
I think saying repetition makes code worse is like saying repetition makes
prose worse. Both code and prose convey ideas. Repetition is a tool. You can
overuse it, and you can underuse it. But it is a tool. Not a flaw.

------
harry8
Controversial:

Is this the same disease that leads to the love of Haskell by those particular
zealots (more not all Haskell programmers) who write yet another monad
tutorial to spread the word of the one truth without writing any useful
programs? Seeing "ah this is sequencing, so i can abstract that..." And so on.
Haskell gets you to higher and higher levels of abstraction in your
programming but doesn't seem to guarantee you'll get a useful program at the
end of it.

(Yes there are useful Haskell programs, obviously. Just less of them than
monad tutorials by at least an order of magnitude)

------
sev
As @hyperpallium said,

> Rules are a poor substitute for actual thought

There are many times "clean code" is a good goal to have. It depends on what
you mean by "clean code" and what you mean by "deduplication". As with many
things in software engineering, the best answer to whether you should strive
for "clean code" is "it depends."

If you have a utility function that you are redefining in every file,
literally copy/pasting it everywhere, then you might as well create a shared
function and import it and use it everywhere instead. This is an example of a
good goal.

------
WoodenKatana
>Obsessing over clean code is like reorganizing your clothes closet on a daily
basis. If it makes you more productive to do so, do it. Too often, however,
it's done compulsively and is counter-productive.

------
shaunw321
Lots of great examples and philosophies here.

I think, for me, I am often on two sides of the situation.

1\. I'm moving fast and writing code to get things done for the customer and
the product.

2\. I have enough time to slow down and write or refactor "testable" code.

Often 1 results in untestable code. Often 2 results in a refactor or most
likely a rewrite to create testable code.

This is an important difference. Optimizing to write code just because you
think is clean (in the sense of shorter, reusable code) is often confusing to
most. This also leads to pull requests that grow large and are hard or time
consuming for others to review.

------
ChrisMarshallNY
I'm finding this kind of issue coming up a lot with the current abhorrence
with polymorphism and inheritance.

I like using interfaces and protocols, but I also still very much use
inheritance. It's a fundamental tool that was invented for a reason, and,
sometimes, it is the best tool for the task.

I've probably weathered just about every "paradigm shift" that has happened in
software development. At one time, using variables with names longer than four
characters was considered bad programming.

Anyone remember GOTO?

Some older constructs (like the two above): good riddance. Others...not so
much. Structured Programming, which was declared The Mark of Satan, at one
time, is still very much the basis for all our work.

I love a lot of the new tools and techniques, but I still mix in a lot of the
older stuff when I write software. To some, this is "unclean," because it
doesn't tick some arbitrary "büzzwürd _du jour_."

Simple, solid code is always a great starting place.

The author talks about removing repetitiveness (DRY). I think that's
excellent, but, in my experience, I need to be very, very careful when I do
that, as the original author may have tweaked just one little line, in one of
the clones, and my refactoring may break things; sometimes, not until it's
been out to the customers for six months.

That's pretty much _de rigueur_ for any refactoring; not just DRYdock. In my
experience, having some robust unit tests and test harnesses in place is
absolutely required _(and development branches -yay new-fangled VCS!)_.

I tend to write code iteratively. I'll start with some naive, sloppy code that
works; maybe not well, then refactor it in stages, testing the heck out of it;
each time.

I used to work for a Japanese company. I had many differences with my Japanese
peers, but they were the most disciplined programmers I've ever encountered.
Whenever they would modify code, they would leave the old code in there, but
commented out, and add some comments, explaining what their new code does.

Made for some pretty verbose source files, but it was immediately apparent
what was done, and why (I think the practice began before most good VCSes were
invented). It also gave you the original code to copy and paste, if necessary.
Very old-fashioned, but it made their changes (and bugs, therein), easy to
understand. It also helped because the code was often stepped on by _many_
programmers.

I'm thinking that a lot of folks are relying on commit comments to explain
changes; which is good, but adds extra time to figuring something out.

------
mluds
The author seems to only look at a narrow case that supports his argument.
Imagine you start out with 2 shapes and 4 directions, but you eventually get
additional requirements for 20 more shapes and 4 more directions. Now you have
much more code, introducing room for errors, making it harder to change, and
making it harder for new employees to understand.

I agree that he should have told his coworker ahead of time, but I also think
his coworker should be open to understanding why his code may cause problems
and be ok with it being changed.

------
underdeserver
The original code was clean.

The thing about the handling of different shapes is that they have
idiosyncrasies. Resizing one side of a square resizes all of its sides;
resizing one side of a rectangle resizes two out of four sides; resizing one
side of an arbitrary quadrilateral only resizes the one.

These are very different abstractions, with different behaviors. Even though
all shape functions technically use the same code, they're not conceptually
doing the same thing. Linking these abstractions by way of a shared extracted
functions is inviting bugs.

------
unexaminedlife
One way I think about this. It seems the earlier version of the author's self
was thinking about the state of the code as a sequence of atomic states. When
you start thinking about your code more as a constantly evolving organism, the
BEST version of a particular section of code isn't necessarily what is BEST at
the moment. But because there are logical branches that your path can take as
the software evolves, your code at this moment is best when it can more easily
accommodate the best path to the future.

------
fendy3002
The only cost of duplication is when you're making changes to it. Let's say
that the cost of typing, checking and finding the code to change is "t".
Duplication makes it "n*t". Abstraction, in case of no special case, makes it
"1t".

So if "n" is somehow big, it'll be costly to make changes and abstraction
makes sense.

However do not abstract similar code for different behavior early (as in OP's
case), but abstract similar behavior early (ex: file system, db access)

------
alkonaut
The article didn’t mention if the team that wrote the dirty code had done a
code review. If they did, it doesn’t mention whether the author of the article
looked at the interaction between the code author(s) and the reviewer. Perhaps
this was discussed and the repetitive code accepted with good reason?

When code is merged, it should be too late for further discussion. The review
is done and flaws like this is debt that should be taken care of the next time
someone needs to touch it.

------
kilroy_jones
At first I wrote ugly convoluted code.

Then, upon seeing the light I wrote clean and organized code.

Now I write what many see as ugly convoluted code because I see further down
the line than they do.

------
paulnechifor
Sorry for the off topic grammar question, but am I the only one who finds it
confusing how people have started to use plural pronouns to refer to
individual people?

~~~
jdashg
It started in the 14th century. Wikipedia has a good article on it:
[https://en.m.wikipedia.org/wiki/Singular_they](https://en.m.wikipedia.org/wiki/Singular_they)

~~~
paulnechifor
Fair enough, but at least they had plural you back then: thou.

------
kizer
There's a balance, as is often [always?] the case in inversely related factors
like code "cleanliness" (hard to talk about since it's a little vague of a
word) and developer "velocity": pristine code can go awry and became "fine
china" code that's too verbose or restrictive to effectively change and
maintain; likewise, spaghetti code, maybe the opposite?, needs no
introduction.

------
zallarak
As PG said, take on as much technical debt as possible: it’s literally
leverage.

Now sometimes “clean” code is the worst of both worlds. It wastes time and
isn’t really clean.

------
theshadowmonkey
One similar experience i had when I was starting out was when I created a PR
and my colleague instead of suggesting changes, made the changes himself,
approved and merged my PR. I told him directly that it’d be nice if he’d have
suggested the changes instead of making them himself. He thought I was being
protective of my code. I was never able to have a good working relationship
with him anymore and eventually left.

~~~
jariel
Maybe consider that it's not 'your code'.

------
aurelianito
My experience is that good programmers prefer extra abstraction and bad
programmers prefer extra repetition. Given that we need to work with bad
programmers, sometimes it makes sense to make things easier for them. But
repetition is wrong and a source of bugs. Each time you give in and keep a
repetition that you know it should not be there you are crippling yourself to
make us programmers a commodity.

------
ultim8k
Clean code is not about eliminating repeated code. Clean code is about
readability. It's a not a rule. It's a programming philosophy.

People set rules but we should always see the reasoning behind the rules
instead of just following them blindly.

Personally every time I refactor some piece of code, I think "Does this make
code easier to read and understand? Will this save me and my colleagues time
in the future?".

------
drngdds
My takeaway from this is "do code reviews".

------
solidist
AHA

[https://kentcdodds.com/blog/aha-programming](https://kentcdodds.com/blog/aha-
programming)

------
franga2000
I would just like to take a second to compliment this website. It loaded
instantly even in that weird broken WebView that Materialistic uses, the dark
mode switcher was nice and obvious but not in my way and switching between
pages is faster than basically any other action I can take on my device
(local, native apps take longer to switch tabs than this website). Well done!

------
jtdev
I constantly see devs applying DRY in ways that conflict with the “single
responsibility principle”. It’s like they just stop at DRY because it’s a
simple concept to understand and relatively easy to defend . I’ve seen people
essentially labeled as heretics for suggesting that DRY (and any other SOLID
principle) should not be blindly applied to every piece of code.

------
cmdshiftf4
So the two cases against writing the most legible, succinct code _given the
specifications at the time of writing it_ are:

>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. 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.

There's no question about this. Nobody likes the self-proclaimed savant who
works in isolation and makes sweeping changes to the codebase or other
people's work without collaborating and gaining some consensus. If it's a
change worth making it should be a simple case to present to your (hopefully)
equally intelligent team.

There is a difference, it has to be highlighted, between refactoring someone's
code in order to extend it yourself and simply re-writing someone's
implementation because it doesn't suit your requirements. The former is part
of the job, the latter should at the very least be an opportunity to mentor
the person's whose code you want to re-write in _why_ it was suboptimal and
guide them on the changes you'd like to make, or even give them the chance to
make it themselves. This is kind of what code reviews are supposed to do.

That does not negate the need to structure and optimize code to remove
duplication whatsoever. It's not an argument against clean code standards and
it's weak that it amounts to 50% of his case here.

>Secondly, nothing is free. 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.

Time changes, requirements change. It's part and parcel of our jobs in
software development. Writing code that at one point is optimal and most
legible for the cases present should also be done to try to make it
refactorable and extendable.

It is much easier to refactor and extend code that isn't riddled with
duplication and mangled with hardcoded business logic. Abstract your code and
write your implementations well, name things in a way that people can read it
and write tests that describe what's expected from it.

Refactoring well isn't easy work. Refactoring a sprawling legacy codebase with
a lot of duplication and legibility problems is _significantly_ worse.

I'm not saying we need to be dogmatic here. If you're given the opportunity to
develop new code you should be aiming to do the best job of it given what you
know _now_ , in a way that will be comprehensible to you, or whoever needs to
touch that code _next_.

We all know that there are problems with premature optimization caused by
"best practices" evangelists who'd happily drive up time-to-market and
operating costs/complexity exponentially in the name of having the codebase
and applications / services architecture in line with whatever he or she has
read lately from "thought leaders" in our industry, but writing the code for a
given application in line with the above isn't one of them.

~~~
mntmoss
The code is not large enough to need maintenance at a fine-grained level.

There is a secondary rule to the DRY "rule of three": If I can blow it away
and rewrite it so easily, there is nothing to reuse or refactor in it. The
feature is done, and we are into code golf and speculation, neither of which
are productive uses of time. In my experience the success rate of speculative
refactors like the one author made has perhaps a 50/50 chance, so no better
than the initial strategy. It's the requirements themselves and the
application of techniques to avoid various classes of errors that give code
direction and structure - not the aesthetics at a moment in time(which is what
author took issue with).

If you spot multiple approaches on the first try, you can add a comment with a
date outlining alternatives so that the conversation may be resumed later when
the new requirements come in. But at all times you're always at the mercy of
"discipline", and there's no preemptive measure that avoids that.

~~~
Rapzid
The author also didn't sound like a particularly senior engineer at the time
for many reasons. So the original code author and the "boss" may have been
taking into consideration timelines and future work/requirements coming down
the pipe.

A very valid reason could have been as simple as "We are re-visiting this in a
couple sprints after feedback and will have a better idea of how it needs to
change. The extra day spent on this wasn't worth pushing getting it into
peoples hands, and we don't know if it would be a waste." The author would
have known this if he started a conversation about it.

~~~
cc81
I heard something like: "The second system you design will be the most over
engineered piece of shit ever"

I don't know who said it but it has been very true for me and my close friends
who work in software development. I remember first starting software
development and I started to read up on "how to do it right" in the Java/C#
world back when XML was everywhere.

I had first started to expand my skills after university by building my own
blog (who didn't at that time?) but thought I should rebuild it according to
"best practices".

Hoooooly shit that was a poorly architected and designed piece of software.
The example in the blog was of course not as poor of an example as my creation
but I feel that many end up in this trap after they have some experience that
they need to do everything "right" and they don't have the experience to
evaluate if it is worth it.

However I also think a good workplace have a healthy mix because those
youngsters will also push the old guard to learn new things and introduce new
technology. Just need a balance between using 0.1-alpha libraries and things
that were released 10 years ago.

------
jacknews
Duplication isn't necessarily 'messy'. In the example, it's very easy to see
what's going on, even if it's repetitive, since it's fairly uniform.

Adding an abstraction layer often adds complexity, and really does get
messy/complex if you subsequently have to deal with specil cases and
exceptions.

------
d_burfoot
I think this guy learned exactly the wrong lesson. The lesson he ought to have
learn is to avoid premature optimization - he tried to refactor the code too
soon after is was originally created. He should have waited a while so that
the additional requirements were fleshed out before trying to create
abstractions.

------
caseymarquis
I know it's completely tangent to the point of the article, but is anyone else
wondering why it took a week to implement scaling objects in a visual editor?
The codebase? The coder? Documentation requirements? A combination of things?

I want to understand what causes projects to lose velocity, and try to solve
those issues.

------
nchudleigh
If required to test all those duplicate lines. The author would make the right
abstractions in the pursuit of laziness/efficiency.

Overstepping and not reviewing and going through proper code review process
isn't a reason to ditch reasonable abstractions. Definitely some conflating of
topics here.

------
jb3689
To me, clean code is a separate concern from coherent interfaces. Your change
changed the interfaces and although I don't have deeper context, I personally
prefer the simpler original interface. Have good interfaces first, then use
clean code principles to handle implementations

------
hestefisk
I would also say it is waste of effort working a late night rewriting existing
code that actually worked. Why waste your time on over-polishing when s/he
could’ve left work early and get a good night’s sleep? I think that is the
more important perspective.

------
stevewilhelm
> We were working on a graphics editor canvas, and they implemented the
> ability to resize shapes like rectangles and ovals by dragging small handles
> at their edges.

It's a dirty shame their development team had to build a graphics editor
canvas from scratch.

------
bborud
What the author described in the article is where ORMs come from.

And why they don’t work. In the words of the great Jeff Goldblum:
[https://youtu.be/6GEFwiXWieM](https://youtu.be/6GEFwiXWieM)

------
whhone
We have "true" duplication and "false" duplication. True duplication is bad
but false duplication is not.

[https://imgur.com/oeN4Dzc](https://imgur.com/oeN4Dzc)

------
fold_left
It would be a shame if this article about specifically DRY created negative
sentiment towards the Clean Code Book as a whole – which is full of great
advice, not only related to duplication, and a worthwhile read.

------
abiro
There is one issue that I haven’t seen mentioned: testing.

The real problem with duplicated code in my opinion is that it either remains
untested or bloats your test suite. And a bloated test suite can slow you down
to a crawl.

------
blazespin
The point of dry is to avoid having to redo code review and security analysis
which can be quite lengthy and onerous. Fixing Bugs in many places is another
thing, but not the main thing.

------
zelly
In a nutshell this captures my argument against functional programming, design
patterns, and this whole code fetish cargo cult.

I care about what the code does, not what it looks like. If two different
things compile to the same hardware instructions, then it makes no difference
to me. Repeat yourself if you want. Make a bunch of indirections via factories
and generics if you want. Use a bunch of ugly branching if you want.

It's all so anachronistic. We have self-driving cars, VR, drones, neural nets,
machines that can write code themselves, post-quantum cryptography--and you're
lecturing me about the $(No One Cares) Pattern from the hottest bestseller of
1994?

------
dkarras
Yet React state management remains an academic exercise.

------
shapiro92
there is never a golden rule, "the rule of 3", sometimes experience because
you have seen the exact same can trump the rule of 3 and you can abstract it
on the first try.

Clean code - duplication, should focus on duplicate code that needs to change
at the same time on all X places it is duplicated.

If the code seems similar but actually has NO correlation, then its not
duplicate it just works the same.

------
Shorel
All this discussion makes an internal voice in my head scream: "They need Lisp
macros!"

But the world moved on from macros into... this.

Edit: My reasoning is as follows: the comment that says:

    
    
         // 10 repetitive lines of math
    

is exactly the line that would have been replaced by a macro in the original
code, and everything would have worked the same, as macros don't modify the
stack like a function call does. Unless you want it to.

------
einpoklum
A lot of duplication is:

* A recipe for typo-based-bugs.

* Makes the code difficult to read if there are subtle differences, since they get lost among the identical repeated parts.

* Indicates that there is (probably, not necessarily) a meaningful abstraction you could have used.

I would say it's technical debt, that needs to be paid. Perhaps not
immediately, but don't let it pile up.

So: Don't "let it go"; rather, pursue it somewhat more flexibly.

PS - There is more to clean code than less duplication...

------
revskill
In my case, i duplicate abstraction as i can. DRY is good when it's used
appropriately.

Abstraction duplication is key here.

------
tosh
hn also struggles with this.

a few days ago: an open source implementation of k (!) was mainly discussed in
terms of coding style and “readability”

[https://news.ycombinator.com/item?id=22009241](https://news.ycombinator.com/item?id=22009241)

------
jtms
I was less bothered by any duplication than I was unnecessarily using let
instead of const

------
bykhun
Great piece, exactly what I needed right now after a night of pointless
refactoring!

------
aplummer
“ I checked in my refactoring to master”

Yikes

------
werber
If someone rewrites my code better than me, I feel like I owe them a beer

------
swyx
who knows the second call may have been the right call, this is often how
libraries and frameworks start by “extracting it out of our production app”

the bigger sin was not checking with the colleague. :)

------
rawoke083600
TL;DR 1) We should not obsess with clean code, we can't agree on what clean
code is.

2) We should also not write dirty code, we cant' agree on what dirty code
exactly is (see all the counter examples)

There are few rules of thumbs that are always true with regards to code
quality. Getting sucked in a job or argument where code-cleanliness is the
no#1 metric and by distinction the "version" of clean code that your boss is
telling you is a hellish way to make a living.

We can usually identify and agree on the two extremes: The very very very good
code, and the very very very bad code... anything else and mostly all of us as
coders are in the middle, and we would be more wise to focus on making the
code work then chasing Zen-State-Of-Compilation-And-Code-Quality. It is
useless when code exists ONLY to transform data... I've never bought a product
based on code quality.

------
jcims
Isn’t the only reason we care about duplicate code that we don’t want to miss
an instance of it for enhancements or bug fixes down the line? Don’t we have
technology that helps us reduce that concern?

~~~
Rapzid
Yeah, it's not about text/code duplication as much as it's about behavior
duplication. In theory both, hopefully three or more!, code paths share the
exact same behavior and if that behavior ever changes it will need to change
EVERYWHERE you introduced a method to abstract it away.

Depending though, the indirection trade off may not be worth the abstraction
to DRY it up. Then you end up with code like what was popular in the bad old
days of Ruby. Essentially, you can get carried away.

------
ddingus
This may not come across well, because it is related to very large CAD models.
Think rockets, cars, ships, airplanes, type large.

In the assembly paradigm, each component has 6 degrees of freedom. On average,
a good human can mentally process a handful of components. Beyond that, it
gets very hard to balance all the degrees of freedom.

Solvers also do not scale well once things reach the hundreds, sometimes low
thousand component level. Linear compute performance bound. Parallelization is
hard and a focus of current development.

Very large assemblies present problems similar to the ones seen in large
software.

The dominant way of building big models, assemblies is to compartmentalize sub
systems, build to common interface points, and only constrain important
component groups.

Resolving all the degrees of freedom requires making big investments in time
and compute power that only pay off when things change.

Almost never makes sense.

The balance is making those investments where change is known or predicted
with high confidence.

Otherwise, it is easier to defer this task and work with common assumptions,
until such time as a more complete model is warranted and indicated.

It is always a hard balance to find because the allure of "handling the future
changes" now is seen as a savings when the truth is almost always a loss.

Those efforts become debt in a few ways:

Big investment in change ready model sees changes that lie outside the scope
of already implemented model dynamics. (Big refactoring needed) 2x if not more
work required.

Model actually defined incorrectly. Creates problems that would not otherwise
exist. Hard to find these due to massive compute and human time spent
simulating and debugging.

Model overly compartmentalized, due to compute limits and too many assembly
model constraints. Makes reasonable changes difficult. Refactoring is needed.

Inability to operate with full or large fractions of model, due to inability
to compute it reasonably.

Finally, where the large model is made across various CAD systems, designing
in place, just putting things relative to common assembly mating points
actually is the most efficient way!

When a dynamic assembly model is needed for analysis or other simulation
related task, or changes, making it right then, even sometimes throwing it
away when the work is done can make the best sense, with only the final result
kept for the future.

None of this is intuitive at first. Everyone will tend to over model, until
they hit one of the pain points, have to over correct, and work through it
all.

------
pier25
Principles are not rules. Also, maintainable and readable code are of
uttermost importance.

------
sirtoffski
Off topic: Docusaurus V2 makes a really clean looking blog via GitHub pages.
Cheers

~~~
sophiebits
It’s Gatsby with (I think) a custom-made theme.

------
bHack
Waste of time.

------
VLM
Code should be easy to debug and troubleshoot six months later by someone
else.

Think about realistic bugs. Somebody had a minor typo or conceptual error and
got a sign wrong in the math. That happens.

You're going to get a bug report a year later about "resizing the top left of
a text box has the wrong gap space".

In the original code, you can look at the TextBlock area and resizeTopLeft to
instantly narrow down the location of the bug, and compare it to its neighbors
resizeTopRight and in like 30 seconds, duh, you subtracted the gap from the X
in resizeTopLeft just like resizeTopRight and obviously the symmetrical
version would be adding the gap for left because you subtracted from X
coordinate for right and they're symmetric, build test, now it moves two
pixels the correct direction, commit, close bug, done in like five (labor cost
expensive) minutes, depending on build and test system, LOL. You should also
look into why the unit test system missed this, but perhaps your fully
automated unit testing does not check pixel perfect UI operations, so these
things will just happen.

Is there anything for Jenkins implementing pixel perfect UI testing, and how
would one spec all the millions of possible actions and combinations of
actions? A startup opportunity for someone?

In the new code, its going to be possibly the rendering of everything is
messed up and you have to decode and store and analyze the entire design of
the entire system in your head simultaneously and run numerous simulation
examples in your mind until you realize technically the gap spacing should be
-1 times the absolute value of X coord or whatever abstract and elaborate
formula. Or maybe you added the absolute value of a negative number which
would only affect one left/right side's gap or similar complicated bug. Its
going to be a VERY expensive bug, like an hour, maybe a day if its really
mysterious and the debugger just doesn't "get" obfuscated code.

Also in the original code you can trivially compose one discrete example at a
time and experiment with what changes when implementing the next example. Its
going to be written and tested very quickly. You don't have to worry about
symmetry related bugs to make TopLeft work at the same time as TopRight and
BottomLeft and all that.

In the new code, its very impressive to other programmers but your boss is
going to notice you have to pack the entire system into your head in a perfect
and 100% correct manner before anything works at all, which seems inefficient.

The most nifty looking abstraction possible isn't necessarily optimized to be
anything other than the most nifty abstraction. Its statistically unlikely to
hyperoptimize for X where important values of Y like debug-ability or speed of
writing are the actual real world goals.

Basically, always simplicate and add lightness, and in some situations, "clean
code" is not simple and light. Sometimes clean is obfuscated by some
perspectives.

------
smikhanov
This article is so tooth-achingly dull, that I hope n-gate will pick it!

------
codingmess
He made a tradeoff that later on turned out to be wrong. I don't think that is
a mistake, after all, it wasn't foreseeable at the time.

It could just as well have happened that some other requirement would have
made the original solution even more impractical.

I think the only mistake here might be overthinking things (can't comment on
changing other people's code, depends on company culture).

Apart from "cleanliness", what about developer happiness. If a more elegant
solution makes you happy, why not go for it, at least every once in a while?

------
0xff00ffee
Of course you're going to have scalability problems if you don't write a
behavioral specification. There are consequences to software architecture
choices, and charging ahead without thinking (aka hacking) isn't how you write
production code.

------
crimsonalucard
The author not only fails to explain why his refactored code is worse he
doesn't even show all of his code. There is simply not enough information to
know whether the refactoring is better or worse. In terms of less lines of
code and code reuse, it is better in this sense. I suspect the author is
pretty junior himself to imply the old way is better than his refactoring
without a clear explanation as to why. I guess putting your baseless opinion
on a fancy blog or medium makes you seem more legit than you really are.

I get his point, however. In the spirit of his argument there is an exact
answer as to why one way of designing a program is better than another way
despite more/less code re-use or more/less lines of code. I'm not even going
to get into legibility here as that is just an opinion piece. Also I'm going
to give a very concrete answer here. No design principles no design philosophy
or any of that.

Let's say you have feature which we call "A" that can be created as a
composition of several primitives. We call these primitives "a, b, c, d."
Let's also say feature "A" can be constructed from a different set of
primitives "b, f, g, a."

Note the overlap in primitives. The overall set of primitives are different
but both ways of constructing feature "A" can share the same primitives if
needed. The two sets in the example share primitives "a" and "b".

Now let's say we want the code to be flexible enough to construct feature "B"
or feature "C" sometime in the future.

lets say feature "B" needs primitives "a, d, b" to construct.

lets say feature "C" needs primitives "b, f, a." to construct.

Which method of constructing Feature "A" is better knowing that you will need
to construct Feature "B" in the future? what if it was Feature "C" for the
future?

Obviously depending on "how" flexible you want your design to be you can
choose one way to initially construct the program or another way. It's all an
opinion and anticipation for the way you design your program in the future.

One strategy to remove opinion from your design is to try to incorporate the
full union of primitives "a,b,c,d,f,g" Or find another completely different
set of primitives (perhaps "h, i, j") that can be used to construct features
"A", "B", and "C."

SO the concrete answer is "h, i, j" is the best set of primitives you can use
to construct your program but if "h, i, j" aren't available then your choice
of "a, d, b" or "b, f, a" or "a,b,c,d,f,g" both hinges on whether you need to
construct feature "B" or feature "C" in the future.

The problem with this article is that he never got into the nature of program
design/organization. Just vague reasoning and hand wavy examples.

------
itchy
> Goodbye, Clean Code

You mean like with the invention of the abstraction/hack called hooks?

~~~
danlugo92
Care to share the shortcomings of hooks?

------
xwowsersx
I have a Google One subscription so I pay Google for storage. Google could
suspend my account and I'd have no recourse whatsoever? I'd have no way of
getting my 10s of thousands of photos back?

~~~
xwowsersx
Whoops, this comment was intended for a different post hah

------
boyadjian
I totally agree on this post. Being too zealous about code duplication is
counter-productive. Don't be more royalist than the king.

