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

151–160 of 599 posts

Re: Goodbye, Clean Code

#151

Post by tsimionescu »

jandrese wrote: post #105
yodsanklai wrote: post #58

Earlier quoted context omitted.

> it's easier to reason about Consider you have 4 times a block of 10 lines of code, they are identical except for a couple of parameters. The person who reads the code has to 1. figure out what the code does 2. see if the duplicated parts differ in some subtle way. The alternative is to replace the duplicated parts with a function that has a meaningful name. This makes the code easier to read. It's not a premature o…

I generally find it pretty easy to reason about code structured like: switch(object) type1: (bunch of code) type2: (bunch of code) type3: (bunch of code) etc... Even if the function is long it's pretty easy to skip over the irrelevant parts. When you get in trouble is when you discover a bug (or have changed requirements) in something that gets duplicated several times and have to remember to hit all of them. The las…

> Even if the function is long it's pretty easy to skip over the irrelevant parts.

> Overall the tradeoff is generally worth it though, because you only need to care about one case at a time.

Which part is irrelevant? As a programmer, I don't generally know which value `object` has, so if I need to understand the whole statement, I need to look at every case, so I often need to check whether they are identical or slightly different.

Duplicate code like this is a well-known source of bugs, one of the cases most often highlighted by static analysis tools.

Re: Goodbye, Clean Code

#152

Post by buzzkillington »

blazespin wrote: post #142
babarock wrote: post #114

Earlier quoted context omitted.

One of the areas where I really like "incidental duplication" is in tests. Tests can sometimes be very repetitive and identical, and it's tempting to want to refactor it in some clever way. That's almost never good. On top of the reasons laid out in parent comment, tests also function as unofficial documentation. I like having everything explicit in there, it makes them easier to read and understand.

Tests rarely have bugs, I find, so generally dry isn’t critical. Also, dry is for security (see below)

Tests always have bugs, it's just that you don't know about the edge cases yet.

Re: Goodbye, Clean Code

#153

Post by lolc »

rdiddly wrote: post #55
scarface74 wrote: post #25

Earlier quoted context omitted.

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…

"Premature optimization something something..." ;)

That's actually "premature generalization".

Re: Goodbye, Clean Code

#154

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

Re: Goodbye, Clean Code

#155

Post by dboreham »

blazespin wrote: post #142
babarock wrote: post #114

Earlier quoted context omitted.

One of the areas where I really like "incidental duplication" is in tests. Tests can sometimes be very repetitive and identical, and it's tempting to want to refactor it in some clever way. That's almost never good. On top of the reasons laid out in parent comment, tests also function as unofficial documentation. I like having everything explicit in there, it makes them easier to read and understand.

Tests rarely have bugs, I find, so generally dry isn’t critical. Also, dry is for security (see below)

Tests frequently have bugs, especially bugs that result in the test passing when it should fail.

Re: Goodbye, Clean Code

#156

Post by soulofmischief »

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…

Another thing you can do when a function becomes overencumbered is to split the remaining similar logic into smaller functions which are composed into specialized functions.

This has benefit in that when analyzing modules, you can spot differences in procedure at a glance instead of needing to dig through 100 lines of somewhat similar imperative code.

Re: Goodbye, Clean Code

#157

Post by dmurray »

Jasper_ wrote: post #110
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…

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…

Well, perhaps you wrote the code quicker because your colleague already wrote all the tests, and it was easy for you to verify the behaviour of your complicated-looking but short code was correct.

Re: Goodbye, Clean Code

#158

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

Re: Goodbye, Clean Code

#159

Post by tsimionescu »

marcus_holmes wrote: post #121
jandrese wrote: post #105

Earlier quoted context omitted.

I generally find it pretty easy to reason about code structured like: switch(object) type1: (bunch of code) type2: (bunch of code) type3: (bunch of code) etc... Even if the function is long it's pretty easy to skip over the irrelevant parts. When you get in trouble is when you discover a bug (or have changed requirements) in something that gets duplicated several times and have to remember to hit all of them. The las…

I am such a huge fan of Big 'Ol Switch Statements over using polymorphism/types/generics/whatever. So easy to understand, and it's all there in a huge scrolling list of cases. If something needs to change, it's easy to change it, and you know what else is affected.

That works until you have 20 different Big Ol' Switch Statements, each switching on (mostly) the same cases, essentially implementing a set of related behaviors in 20 different places instead of grouping the 20 behaviors under the same umbrella.

Overall, I think there is an equilibrium between the number of cases in the switch and the number of different switches with the mostly the same cases. The fewer cases ypu have and the more times you handle the same cases, the more it will help to group these different behaviors in separate places: each case would correspond to a different class, each switch statement to a different method in each class. The fewer classes and the larger they are, the more I think it helps to apply this transformation.

By the way, this has nothing to do with the discussion above. The alternative to the switch that GP commenter presented isn't polymorphism, it is simply extracting the common lines into a separate function.

Re: Goodbye, Clean Code

#160

Post by habitue »

jt2190 wrote: post #103
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…

> 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. Changing one developers _working code_ after they’ve invested a significant amount of time into without discussing it with the team first it is to basically heap on a number of unwritten requirements and also decide that the sch…

> Changing one developers _working code_ after they’ve invested a significant amount of time

How much time they invested is irrelevant. There are lots of times where someone is wrestling with something for so long they just want to get it done and don't want to look at it anymore. Many times it's trivial for someone fresh to tidy it up

> If removing repetitive code is a requirement then the team needs to be informed that code will be reviewed for repetition

This focus on "requirements" is only really trotted out when someone doesn't like someone changing their code and is looking for a defense. In reality, coding is super subjective, there are dozens of aesthetic judgement calls everyone has to make. Saying something "wasn't a requirement" basically means "fuck off I don't like you touching my code".

There's no way to codify concrete requirements for handling every possible way code can be improved. If devs are throwing requirements at each other, that's a culture problem, not a spec problem

Post reply on HN