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

381–390 of 599 posts

Re: Goodbye, Clean Code

#381

Post by konschubert »

pvorb wrote: post #346
valand wrote: post #311

Earlier quoted context omitted.

Premature abstraction. Rule of 3 helps. But I found better principle for it: 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 log…

I find abstractable parts in code by thinking about potential names for it. If there is no good name for a potential common function, it's probably not a good idea to extract the section into a function. Maybe it can be extracted into two separate functions with good names?

This is a proxy for having a well-defined purpose for a function.

Re: Goodbye, Clean Code

#382

Post by shrimpx »

bjourne wrote: post #274

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

For a software engineer, emotional attachment to code is inversely proportional to net worth.

Re: Goodbye, Clean Code

#383

Post by Stratoscope »

skrebbel wrote: post #361
Stratoscope wrote: post #262

Earlier quoted context omitted.

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…

To be honest I don't get it. What language was this in, that you couldn't duck type the data or make the data structures inherit from some shared interface that encapsulated the commonalities?

That is a reasonable question. You may have noted that I left some details vague, so all I ask is that you trust me that the overall story is true.

Of course if you don't trust me, that's cool too! ;-)

Re: Goodbye, Clean Code

#384

Post by _ph_ »

bjourne wrote: post #274

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

I have a very different view to this. As a disclaimer: I am working in a smaller team where most code is non-trivial, may be it is different in very large teams on large code bases.

This isn't a question of ownership in the sense of a private property. It is about how you interact and respect and professionalism. First of all, a larger change to code a teammate wrote has a ring of saying: it was badly written. So this is partial a matter of social interaction. If the code is badly written, it needs to be fixed, but one should be reasonably polite about that. Usually, the better way of doing this is having a quick chat with that person where you explain why and how you think the code should be changed, before you just do it. Also to pass on any insight you have about the code, the creator missed.

On the other side, there is the big elephant in the room: unless we are talking about rather simple code, the code perhaps was written as it was for a reason. This often enough isn't apparent on the first glance. So a refactoring without checking with the author always bears the risk that you introduce a bug or at least get into the way of planned changes, which would be incompatible with the refactoring.

So there are many reasons to quickly check with the author before doing significant changes. If the proposed changes are good, then the author will be happy about you doing them, if not, then its better to have talked about them before doing them. And in any case, it is good to have communicaton about major changes.

Re: Goodbye, Clean Code

#385

Post by dlkf »

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…

I think that what Burlesona is suggesting is more nuanced (and effective) than the rule of three. We can easily imagine situations where code used twice warrants a refactor, and situations where code used three times does not.

The rule of three suffers the same problem as the pre-emptive refactor - it is totally context insensitive. The spirit of the rule is good, but the arbitrary threshold is not.

Similarly, 99% of your comment is bang-on! My only gripe is the numeral 3. But pithy rules tend to become dogma - particularly with junior engineers - so it's best to explain your philosophy of knowing when to abstract in a more in-depth way.

Re: Goodbye, Clean Code

#386

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

Re: Goodbye, Clean Code

#387

Post by skrebbel »

Stratoscope wrote: post #383
skrebbel wrote: post #361

Earlier quoted context omitted.

To be honest I don't get it. What language was this in, that you couldn't duck type the data or make the data structures inherit from some shared interface that encapsulated the commonalities?

That is a reasonable question. You may have noted that I left some details vague, so all I ask is that you trust me that the overall story is true. Of course if you don't trust me, that's cool too! ;-)

Hah! Totally do, I'm just nerd sniped by the details I guess :-)

Re: Goodbye, Clean Code

#388

Post by speedplane »

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…

> don’t extract repetitive code right away, try and build the feature you’re working on with the duplication in place first.

I have a similar rule of thumb: when you do a thing the first time, just get it working. When you do a similar thing a second time, just get it working. When you do a thing a third time, pull it together as a cohesive abstraction.

This is a silly generalization, but the point is that you generally need to see a handful of examples of a thing before you can make any useful abstraction. What's more common is people creating abstractions during the first iteration, leading to features that are never used, and others that were never considered.

Re: Goodbye, Clean Code

#389

Post by b0rsuk »

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…

From my experience, I would start with a more compact implementation first in hopes that it would save me time. This usually ends with me having to do more complicated operations in my head to plan the code. It's tempting. It's like trying to do math in your head to save time - often leads to a mistake. Now I believe there's no shame in trying out a solution first and rewriting it later when I get a feel what the problem is really about.

John Carmack once said something like "If you're not sure which version is better, code it both ways and compare".

The only downside of later optimization is that in commercial environment they often don't let you clean up.

Re: Goodbye, Clean Code

#390

Post by speedplane »

dlkf wrote: post #385
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…

I think that what Burlesona is suggesting is more nuanced (and effective) than the rule of three. We can easily imagine situations where code used twice warrants a refactor, and situations where code used three times does not. The rule of three suffers the same problem as the pre-emptive refactor - it is totally context insensitive. The spirit of the rule is good, but the arbitrary threshold is not. Similarly, 99% of…

> My only gripe is the numeral 3. But pithy rules tend to become dogma

Agree, 3 is a pretty arbitrary number. If you have a function that needs to work on an array that you know will certainly be of size 2, it takes minimal effort and will probably be worthwhile to make sure it works on lengths greater than 2.

But the bigger point is valid: you need examples to know what to expect, and if you make an abstraction early without valid examples, you'll almost certainly fail to consider something, and also likely consider a number of things that will never occur.

Post reply on HN