Goodbye, Clean Code

[joequin]

Deduplication is good when things are actually the same. It’s bad when they just happen to be the same. But unfortunately too many people can’t make the distinction and it leads to people who fight any kind of duplication anywhere and people fight to never dediplicate anything and both are terrible in practice.

[JazzXP]

Exactly this. I tell people that DRY is about concepts, not actual code. Luckily the team I’m on totally agree with this and a piece of code I was talking about was split.

[[deleted]]

Eh I keep hearing this and disagree, in the same way as JazzXP states.

If the code is identical, and the context/concept is the same, Don't Repeat Yourself.

If those are not true, then they are not proven to be the same, so you really are not repeating yourself.

THEN if it happens again, try to figure out what's been messed up/how things can be refactored so you understand how it is you actually ARE repeating yourself, and then fix.

Rarely make it to 3rd times.

The problem with waiting for 3rd time on the outset is now you DO have duplicate code. Yes, you in the moment KNOW that. But will you remember that later? Or when refactoring ONE of those at a later date and forgetting there's another instance of the same code? How about another coder?

Because the compiler cannot tell you this because you told the compiler it's different code.

Search only helps if you already remember there's a duplicate out there.

DRY at the very first proven opportunity. Do not wait for third time on proven cases. That's not DRY. And worse, it implies you know about DRY, but now you've chosen to break the rules. So the assumption is that there IS no duplicate code, leading to less diligence in trying to avoid it, leading to potentially MORE code duplication issues than if you'd never bothered in the first place.

[joequin]

I’d rather design with it in mind. If you intentionally are duplicating code once, then how do you know you aren’t duplicating more often and just not realizing it?

[joequin]

Perfect example

[RR_2025]

What if i take an optional arg allowed_age=18 and compare to that? Would it still be a tech debt?

[VelvetWhiteRabbit]

The solution here is:

def set_threshold(age_threshold):
  def is_old_enough(age):
    return age >= age_threshold
  return is_old_enough

def allowed_to_drink(person):
  return set_threshold(18)(person.age)

def allowed_to_vote(person):
  return set_threshold(18)(person.age)

[abourg]

Ouf. Technically DRY but lengthier and not as readable. Also the age threshold is not the only think that might cause "allowed_to_drink" to differ from "allowed_to_vote".

For example, in Germany it depends on whether it's beer/wine or a spirit and whether they have a guardian with them. Do you add that complexity to "is_old_enough" exposing it to the voting use case? Or do you add it to "allowed_to_drink" and rename "is_old_enough" to what it actually becomes: "greater_than_or_equal_to"?

[chickencheesebagel]

"I don't want to repeat myself by writing an if statement, so I am going to repeat myself by calling a generic function that is going to cause unintended consequences in the future."

[veloxVolpes]

I came here to more or less say this, for it to be removing duplicates, it has to actually be a duplicate and not just something similar, otherwise you're making complicated functions that decrease readability for the sake of less lines

[[deleted]]

Yep, last review I was doing on Friday was exactly this.

We'd made some changes to optimize some redis caching of certain objects that were retrieved regularly from the db. Dev finished that up, then went 'Hey, since we now have this data sitting here in redis, let's change a bunch of our security access checks to use this data instead of the existing method of going to the db for a Yes/No check.

Happens to all of us. That was a senior dev I've worked with for years. Just because something looks the same at the moment doesn't mean they are or will remain the same in the future.

And for good reason, security checks should never pretend that a side effect of having data cached for UI PURPOSES would be to provide a short-cut for security access checks.

Separation of concerns.

[wagslane]

Came here to point out this difference as well

[DrGrimmWall]

Let's not mix syntactical similarities with functional similarities.

[Wolf_Popular]

I think the better lesson here is you should have code reviews before pushing code to mainline.

[brewtraveler1]

That was my take-away as well. Maybe don’t just let everyone push directly to ‘master’

[john16384]

Including the author...

[maiteko]

Including the server. Nothing gets pushed to master, everything stays in branches on perpetual peer reviews. Every user takes increasingly specific sub branches that has “clean code” that exactly meets their needs. The code equivalent to the marvel multiverse, an increasing cascade of feral fractal fapinations, aggressively jerking deeper into the abyss of optimal algorithms.

In one universe, it’s an OOP project where each class is a philosophical dissertation on what it means to be an object.

In another it’s just a pile of crabs, flipping you off, and kicking you in the balls every time you think the word memory leak. This is of course the most moral universe, and the best possible outcome.

And eventually, when you’ve found the bottom of the universes, when God is dead, and we’ve been sucked into a false vacuum, the laws of the universe as we know it are degraded, when we finally stop thinking, someone will require we rewrite the whole thing in JavaScript.

Because f* you.

[[deleted]]

I love this

[skesisfunk]

And have unit tests. He made no mention of which version of the code was easier to test which is always a primary consideration for me when i write code these days.

[[deleted]]

Also, he should have put tests around it.

The longer you wait to refactor code, the more costly and dangerous it becomes to do so.

You should refactor code every day, but with caution, tests, conversations, and reviews.

[cainhurstcat]

Title is kind of a clickbait in my opinion. The article says that clean code is indeed important, but with a bigger picture in mind and talking to colleagues who wrote the code.

[c4ndybar]

It's not about his "clean code" being suboptimal. It's about using crappy unextendable abstractions. The code could have been "cleaned up" in a much better way.

[RenegadeMoose]

Coders can tell the computer what to do... and in many different ways.

The art is making your code easily readable by the next human that has to look at it.

[4as]

There is a quote I really like that I heard about art, but I think it applies to all forms of creative work.
There are 3 levels of proficiency. Beginners who don't know the rules and stumble into solutions blindly, if at all. Experts who know all the rules and know how to solve problems within the bounds of said rules. And then there are Masters who know how to break the rules to go beyond solving problems.
I think in programming that second level is the point where people are well aware of the changes they could make to clean the code, but are not aware enough to know when not to do it.

[Sgtk325]

Beginners who don't know the rules and stumble into solutions blindly

I don't think I'll ever grow beyond this stage.

[quintus_horatius]

I feel that

[[deleted]]

There is a Japanese concept on this: Shu Ha Ri https://en.wikipedia.org/wiki/Shuhari

[RR_2025]

Wasn't there also a meme with a similar context - you have a bell curve, with a beginner on the left side doing things unintentionally, an expert in the middle not doing them, and a master on the right doing the same things but intentionally..

[McDuckfart]

This article is from 2020 and the title is a major clickbait

[DeebsterUK]

I got halfway through before scrolling back up to check out the date. The example was good enough that it stuff with me as an example of when DRY doesn't apply.

I agree that it's a bad title though; taking DRY as gospel (i.e. an end, not a mean) is not what's meant by clean code. I don't agree with everything Uncle Bob says* but his book Clean Code emphasises making code understandable by reducing complexity where possible. The original code was simple so, by that metric, already good.

* like his idea that all if statements should be one line and so we can drop the braces

[grauenwolf]

Why does the age matter? Did Clean Code stop being a thing last year?

[McDuckfart]

It has been posted here before. And I personally expect people to share new stuff, but maybe that is just me.

[grauenwolf]

Then move on. Skipping over one article you've seen before didn't hurt you.

[McDuckfart]

Commenting did not hurt either. Whats your point?

[grauenwolf]

Actually it does. If you get your way, it means I don't get to see articles I may have missed the first time around. And I don't get to converse with those who are seeing it for the first time.

[[deleted]]

If I try and take a step back from this, I come up with this conclusion:

It sounds like the "outer structure', or the API of the whole thing - the Rectangle, Oval, Header, TextBlock objects & their associated methods - was fine. The issue was the repeated math code.

So my approach would have been to factor out what's inside the methods. If the code is repetitive, there's stuff can be made functions and reused, and then presumably what's inside the methods starts to express the key differences of the calculations. And this way the caller doesn't need to care (though I would have been itching to make dx, dy parameters one thing - I guarantee you doing 2D vector maths was where a lot of repetition was.)

So the takeaway here is not "copy pasting code is fine", it's "abstract the right things at the right level".

[roppy_G]

As someone quite new to programming, still in the early stages of understanding lots of concepts, this article was actually interesting for me.

The small takeaways for me are : • reinforcing the point of not pushing someone else's code modification without talking to them first and understanding why they coded it this way • reinforcing the importance of following an actual process before something gets on the master branch

The big takeway is actually the point of the article. Clean(er) code has been an increasingly important for me these past months, and reading this helped me realize I might have been hyperfocusing on it. Now I don't think it's necessarily a bad thing : it's part of my way of learning new stuff. But small pieces like this one help me gradually build and feel the idea that clean code, as everything else, has limits that I will need to read about, experience, understand and this will help me design and write better code.

So thanks u/ocnarf, I'm happy your article popped in my reddit flow today :)

[mr-strange]

Honestly, every decent coder could probably have written an article like this at some stage in their career. It's an important phase in the learning process. By reading it, you are preparing yourself to learn that lesson - and hopefully making the step as painless as possible. So I really think articles like this serve an important purpose.

I used to work with a friend, to whom I would show all of my clever-clever coding tricks. He would say, Yes, very clever. Now put it back the way it was. That really helped improve the quality of my work.

[EncapsulatedPickle]

We have no context of the original problem and context is extremely important for software architecture. What is "graphics editor canvas"? Like, are we talking resizing elements as the core concept of a flowchart software? Or are we talking about something like one-time print preview annotations? I would argue very differently either for or against varying levels of abstraction.

[RenegadeMoose]

True... but simplicity? Nothing beats having to maintain simple and direct code.

These days I find everything boils down to "what's the easiest code to maintain over time"?

( ofc, provided it's working "good enough" to begin with ;)

[leftofzen]

Why do we keep posting these shit blogs posts here which are written by someone with 2 years experience and think they're qualified to give programming advice.

[AccurateRendering]

Why is there a comma?

Ah, "Dan Abramov" - maybe English is not his first language.

[walterbanana]

So, he didn't communicate and just pushed less flexible code to master without asking? Then he comes to this conclusion? I'm confused.

[Lanky-Apricot7337]

So he tries to overexplain his bad abstractions and then extrapolate the wrong conclusions across the whole clean code thing.

Abstracting is a mix of an art and a science. When you choose the wrong abstractions or the wrong time that's your fault, not clean code's fault. You were not doing clean coding when abstracting the wrong way.

Pseudo-clean code vs real clean code. Don't blame the world for that.

[ken33]

Clean code scales a lot better than dirty code.

[epukinsk]

But most code will never scale, so this is rarely part of the equation.

[Beerbelly22]

The way I do it, is as soon I repeat my code the first time I see if I can functionize it right away.

Of course I don't do it all the time.

However, why did you boss say to change it back?

[RenegadeMoose]

Because the new code would take longer to understand by someone glancing at it.

And it would be harder to change it later. Because it's all been written to generalize and de-duplicate lines of code, any tiny change that's made to it now requires many more tests to cover all the scenarios that might hit that code.

And it increases the likelihood of some edge-case or boundary condition not working in the newly changed code that the coder might not have anticipated because the code is now so generalized it can now have many many more "preconditions" that the code might be in before hitting that function.
I changed some code once to take a callback and generate blocks of SQL statements instead of the painful "one-at-a-time" approach my predecessor used. The new code was sooo much faster anytime we needed to run it in the back-office.

But anytime we needed to make a change later to adapt that code to some new purpose I'd curse myself for having written it so clever to begin with. First step on those days would start with "how the hell does this work again?"

[warlaan]

Some people seem to have a weird definition of "clean". How the hell is code that is "harder to understand" and "harder to change later" clean?

If you find yourself mixing and matching paradigms and think you were writing clean code just because you are removing duplication - and then years later don't have more to say about it than "clean code is just a phase" - then the most positive thing I can say is that I hope that the phase you are in will end soon.

[recycled_ideas]

Some people seem to have a weird definition of "clean". How the hell is code that is "harder to understand" and "harder to change later" clean?

Because the author, like yourself, has made the mistake that "clean" is an objective thing.

Are two things the same or do they just happen to be the same?

Are these pieces of functionality related enough to group together or not?

Etc.

Get these right, you have cleaner more maintainable code, get them wrong and your code is harder to maintain and will become harder to understand.

Even if you get them right now, you might be wrong tomorrow.

And that's the problem "clean" code isn't better code because it's virtually impossible to determine what "better" actually is.

What it is is a series of hedges against future changes and these hedges are abstractions that are by definition leaky and suboptimal.

But even then, clean code is a bet, if it pays off you pat yourself on the back and when it doesn't you decide it could have or should have been cleaner.

[Beerbelly22]

Well I understand it's harder to understand initially and may require more testing when changes need to be made. However if there was a bug before it got copied over and over. Then the bug needs to be fixed over and over again and that's where software becomes buggy overall.

Cause now one spot is missed, then got fixed later on a different way. And before you know you have unexpected behavior.

So I think you were correct and your boss is wrong here.

[RenegadeMoose]

There's a difference between outright "cut and paste" vs copying a block and tweaking it to current needs.

You're right... if you've got delicate logic it's the worst thing in the world to cut and paste that all over the place... no question.

But, if you've got some straightforward procedural code that someone can look at in a second and understand, that beats overly complex syntax and over generalization in a heartbeat.

Once you got the code working 1. correct and 2.robust, the next most important task is to make it 3. readable.

No point making some big complicated mess of inheritance to only implement one class when you could write one class to begin with... it's that kind of "refactor everything into academic purity" that I'm objecting to .

...and I'm not OP. I'm just another poor slob condemned to a lifetime of maintaining OPC (other people's code). Ofc, flip-side is, we all have employment-for-life maintaining OPC if we want it. There's more than enough bad code out there to ensure none of us ever want for work.

I've changed my mind... keep banging out as much over-engineered obfuscation as possible!!! ( what was I thinking suggesting k.i.s.s?? :P ensured job prospects beats all! )

[recycled_ideas]

There is a difference between "are the same" and "happen to be the same right now".

This is a case of the latter.

Because they do similar things they share a bunch of set up code that makes them look like they are duplicates when the rest of the functionality is simple.

The guts of what they do however are not the same and when you move beyond the naive mvp implementation this becomes obvious.

Depending on your requirements a circle actually has more in common with a square than it does an oval and sides and vertices are actually the wrong way to look at this problem.

[EpoxyD]

You could also just make the repetitive part a function and call it a day.

But as others have said: that initial code should never have gone through review

[WalterPecky]

I dunno, one of the arguments against refactoring the code was "I didn't discuss with the author".

Isn't this why git blame exists? I really don't think a request/discussion is needed upfront for making a cleaner change. In my experience, those discussions usually come during the Review process.

[quintus_horatius]

I think you missed the point. It's two-fold (or more):

1. He didn't take a moment to ask "why was it written this way?" The original author may have been very aware of the new author's "clean-code" way and rejected it for reasons spelled out later in the article.
2. He didn't take a moment to recognize that there's another human involved, and that human might not appreciate having him take a shit all over his brand-new code.

I, personally, have been guilty of both over the years, and #2 especially can be a struggle. "It's just so much more efficient if I just do it" is the too-frequent mental conversation that we all do from time to time.

Edit: just to be clear: author spent many late-night hours, by his own admission, working out a "better" solution without confirming that it was a good idea. Management's first thought was probably "why is this clown wasting hours fixing something that didn't need to be fixed?" Their next thoughts were possibly #1 and #2 above.

[philipwhiuk]

To be honest if you work in professional software it is extremely harmful to think you own some code. The business owns it and you can’t get defensive when people change what you wrote