Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Goodbye, Clean Code

overreacted.io

Post reply on HN
Print view View original

161–170 of 599 posts

Re: Goodbye, Clean Code

#161

Post by buzzkillington »

fzeroracer wrote: post #136
Jasper_ wrote: post #110

Earlier quoted context omitted.

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…

I've humbled and have been humbled before because sometimes what looks like an ugly, unclean solution is the correct solution. There might be weird edge cases in the system that you catch, but the person doing the rewriting doesn't see until they push the code out and it breaks something in an entirely different part of the codebase. That's the biggest reason why you should always, always discuss those changes with y…

Or have tests. If your push comes with tests and I reduce the code by half while still passing all of them it means that 1). you didn't actually test everything you've done 2). you didn't write code as well as you should have 3). I'm an idiot and made the code less robust.

1) happens all the time. 2) happens some of the time 3). happens as often as 2.

Re: Goodbye, Clean Code

#162

Post by slavik81 »

temac wrote: post #13

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

> I'm against the idea that people should be attached to "their" code

I agree with this, but it goes both ways. The codebase belongs to the team, not any individual developer. Making changes for the sole purpose of making the team's code correspond to your personal preferences is the opposite of professional detachment.

Re: Goodbye, Clean Code

#163

Post by AlexCoventry »

kelnos wrote: post #138
marcus_holmes wrote: post #116

Earlier quoted context omitted.

I thought all of this until I got used to Go's error handling. There's a couple aspects to this: 1. After a while, the "if err != nil {" becomes a single statement in your mind, and you only notice it if it's different (like trapping things that should error with "if err == nil {"). In other words, it only feels verbose if you're not used to it. After a while, the regular rhythm of "statement, error check, statement,…

The downside I see with go's error handling is that you can forget to check. With rust, if the function being called returns Result, you have to deal with the error (even if dealing with it just means propagating it out). Missing error handling is such a common source of bugs that go really turns me off here.

> you can forget to check

Linters can help with this.

Re: Goodbye, Clean Code

#164

Post by jrumbut »

lilyball wrote: post #73
burlesona wrote: post #10

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 ne…

I have 2 rules I use when determining whether to duplicate code or to refactor: 1. How many duplications are there? If the code is duplicated once, that's fine. If it's duplicated twice (so 3 instances of it), then it's time to consider refactoring, subject to the next rule. 2. Why is the code duplicated? If it's "incidental duplication", i.e. code that happens to look the same, don't refactor. Only refactor if there…

It's also useful to look at not just the duplication but the code itself. In this case, it was code for geometry which is not like to change all too much.

Often the difference between harmless but ugly looking duplication and duplication that is actually harmful relies on the semantics of the code and not just its appearance.

Re: Goodbye, Clean Code

#165

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

Re: Goodbye, Clean Code

#166

Post by leoc »

mntmoss wrote: post #51
cmdshiftf4 wrote: post #21

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…

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 p…

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

This rule seems not to be correct though. For example it would mean that one of the most common and widely accepted (as far as I know, and admittedly I know nothing) changes—replacing an explicit for or while loop with some kind of iterator construct, for example a foreach or even a call to a map function—was a bad idea. By and large most individual for loops are pretty easy to understand and rewrite if you look at them. The first problem is that naturally one hardly ever has to read just a single for loop, and that the impact of small insults to abstraction and readability really adds up when repeated tens to thousands of times in a codebase. Second: that a piece of code is easy to read, blow away and rewite is very far from a guarantee of no bugs in either the old or the new version, and AFAIK the history of the vanilla for loop is a classic example of that. Again the impact of this depends on the fact that the for loop can be repeated many times in a codebase.

OTOH the example code in TFA was not repeated with (or without) small variations many times in the program. (I'm not talking about how often it was called or about repetition inside the example code here, ofc.) So your test probably does correctly show that changing or not changing this piece of code on its own is only a small-stakes decision, unlike having say 500 vanilla for loops in the program. But if you consistently let individual bodgy code segments pass then surely you're liable to end up with a large and diverse body of them in the codebase, and that's in some ways even worse than 500 for loops, which can at least all be found with a simple search.

Re: Goodbye, Clean Code

#167

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

Re: Goodbye, Clean Code

#168

Post by jdnenej »

scarface74 wrote: post #25
burlesona wrote: post #10

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 ne…

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…

Every time I have seen a feature that was written general to handle possible future uses, after a year of sitting unused there will certainly be modifications or surrounding code that doesn't support the planned extensibility. So it can never be used without a lot of work changing things anyway.

Re: Goodbye, Clean Code

#169

Post by zmmmmm »

yodsanklai wrote: post #38
burlesona wrote: post #10

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 ne…

> 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 In which case, you should split the helper function ( extract sub-part common to all cases, and report the differences where the helper function is called). I think I would most of the time go with de-duplicating as early as possible, as long as the helper fun…

> Duplicated code causes many issues. You mentioned introducing bugs, but it also makes the code harder to read

That is I believe contestable. Yes, it can end up easier to read but there is a big tradeoff - when you remove code from its context it is much harder for a reader to reason about it. Once something is extracted into a function you can't see, you have to mentally ask the questions like "could this return null / None?", "how will it behave if the input is negative?", "does it throw exceptions", "is it thread safe?" etc etc. All this is directly observable in context when the code is inline, not so when it is removed to a central place.

Re: Goodbye, Clean Code

#170

Post by Noumenon72 »

nathell wrote: post #101

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

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.
Post reply on HN