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

71–80 of 599 posts

Re: Goodbye, Clean Code

#71

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

Re: Goodbye, Clean Code

#72

Post by Nullabillity »

scarface74 wrote: post #8
Insanity wrote: post #4

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

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.

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

Re: Goodbye, Clean Code

#73

Post by lilyball »

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's actually a reason why the code is the same. Which is to say, attempt to predict the future: if a change is made to one instance of the code, do I expect it to be replicated to the other instances too?

Re: Goodbye, Clean Code

#74

Post by slavik81 »

danabramov wrote: post #18
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 think there’s a balance. There’s a difference between gradually improving something, or improving it at some point later — and literally rewriting 100% of code someone has just landed over the night.

I agree with you. To me, the action felt wrong when he described it. I wasn't sure why at first, but I think perhaps because the work was not in service to any actual task.

If he was in charge of writing the next piece of functionality for that code, it would be poor teamwork to completely refactor it without discussing the reasons for the design, but at least it would have been within his area of responsibility.

This is also the reason why I like having code review. The review would have been a good place to raise his concerns. They could have had their eventual conversion earlier, before feelings were hurt or wasted efforts were made. It would even help in the opposite case, where the author's code was legitimately poor and the reviewer had good suggestions.

It's not a panacea, but I find code review helpful. In my experience, review was the time when a lot of knowledge-sharing occurred. Some might feel that only senior developers should review, but as a junior I learned a lot about why the author made the choices they did. I learned how people expected the code to change, about language features or pitfalls, etc., and it helped me grow as a developer.

Re: Goodbye, Clean Code

#75

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

Re: Goodbye, Clean Code

#76

Post by tcbawo »

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…

The quality of a team is the quality of the communication. If the architecture and design isn't well communicated, you get people going off in the wrong direction, or polluting a clean design. Also, people are most attached to their code based on their time investment. If you trash someone's code/changes, you trash their time, which could have been more efficiently decided with a conversation ahead of time. If you find yourself surprised by design changes, find design flaws in a code review, or have different definitions of "improvement", the communication in your company may be lacking.

Re: Goodbye, Clean Code

#77

Post by renox »

mfateev wrote: post #42
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…

Go error handling is a good example of this. So far all the attempts to reduce the repetitive `if err != nil { ... }` through some abstraction failed. Look at https://github.com/golang/go/issues/32825

In the language maybe but both Rust and Zig show that it's possible to have much less 'bloat' for error handling even without using exceptions.

I'd say that go designers have still work to do: Zig especially show that you can be a 'simple' language and yet have both sane error handling and generics.

Re: Goodbye, Clean Code

#78

Post by daxfohl »

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 call these "spaghetti abstractions", and they're the worst kind of spaghetti.

Re: Goodbye, Clean Code

#79

Post by monksy »

Insanity wrote: post #4

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

That and follow it up on a code review pointing out that it's duplication that could lead to bugs later.

Re: Goodbye, Clean Code

#80

Post by specialist »

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…

Upvoted. We'd probably come up with different implementations, I strongly prefer composition and "interpreter" style code, but whenever I've seen casual mutations in different dataflows, it was because of doing one off changes while ignoring the whole.
Post reply on HN