101–110 of 224 posts
Not a single word about what the boss objected to. Reading between the lines, I suspect the author still may not know. That's the problem, not Clean Code.
When we don’t feel confident in our code, it is tempting to attach our sense of self-worth and professional pride to something that can be measured. I don't think assuming people who disagree with you lack confidence and are "compensating" is an effective way to reach an audience. Rewriting your teammate’s code There should be no such thing as "your teammates code", there is only your team's code. If the changes were…
You should 100% talk to the original author before rewriting though. Not doing so communicates that you think you know better than them, so much better than them that you don't even need to talk to them to understand why they wrote it the way they did, and the tradeoffs. This is very arrogant and would most likely bother the original developer, which is bad for team dynamics.
This one was a bad example. But past 5 or so years of experience, when someone has experience with other languages and coding styles, it's more interesting and I often learn a lot from them.
I used to be much more of a curmudgeon about wanting to clean up existing code on sight but a few bits of reality seeped in over time, such as: 0. Existing code is probably working fine, and making changes (for any reason) risks adding a bug. Worse, reviewing code with a bunch of reformatting is tedious and the reviewer may assume you’ve only done reformatting and not notice the accidentally-changed behavior either.…
Cleaning code is often wasted effort when you don’t have the full picture or understand future variations. Like you mentioned with ripping out your refactor later.
This improves the speed of implementation one and two because we don’t have to think as much. Then third iteration is faster as well because it’s obvious how to abstract.
I'm shocked that from all discussion nobody noted the root of all evil: "It was already late at night (I got carried away). I checked in my refactoring to master and went to bed, proud of how I untangled my colleague’s messy code." No PR, no code review, no CI. Just a cowboy pushing to the master..
This guy was working at Facebook at the time too.
When we don’t feel confident in our code, it is tempting to attach our sense of self-worth and professional pride to something that can be measured. I don't think assuming people who disagree with you lack confidence and are "compensating" is an effective way to reach an audience. Rewriting your teammate’s code There should be no such thing as "your teammates code", there is only your team's code. If the changes were…
You should 100% talk to the original author before rewriting though. Not doing so communicates that you think you know better than them, so much better than them that you don't even need to talk to them to understand why they wrote it the way they did, and the tradeoffs. This is very arrogant and would most likely bother the original developer, which is bad for team dynamics.
Change for change's sake may be bad for team dynamics. But if the change is one that everyone agrees is for the better, no one should be offended by the improvement.
This seems close to illustrating a thought I've been toying with turning into a blog post but need some more examples of before I do: "Abstract over data, not behaviour" This seems to fall into the trap of abstracting over behaviour. The best other example I've thought of for this is the 'generic repository pattern' common in C# and I'd guess Java. Just because CRUD on types is all similar behaviour you can't really…
leave the objects alone, but write pure helper functions that implement the most common expressions inside the mathy parts, and have all the similar-looking objects use those helper functions.
There was a discussion a couple of years ago on this article with 500+ comments, for those interested: https://news.ycombinator.com/item?id=22022466
No thanks, we're doing it all over again here. This time will be better!
Earlier quoted context omitted.
Would you go with a simple comment/tag to denote both piece of code are identical but not abstracted away on purpose ?
I do this // @note this code is duped in ../../some/other/file.code
"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." While undeniably true, this feels completely orthogonal to the question of clean code. You could ruffle someone's feathers in the exact same way by taking code into the opposite direction. "Goodbye, Clean Co…
(Putting common sense in scare quotes to acknowledge that this is new information to some, no matter how obvious to others)