Hacker News Reader

Comment by breckenedge | Hacker News Reader

This article really buries the most important lesson about refactoring:

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

Also (2020).

View on HN

While that is undeniably an important issue, personally, I would pick this as the more important point in this particular context:

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

The reason for my choice of this point is that provides a direct, technical counter to one of the simplistic technical rules on which the Clean Code movement is founded, and so might make a impression on someone who has, heretofore, regarded the case made by Clean Code proponents as irrefutable.

That's also why I say "in this particular context", as in the broader scope of software development practices generally, the interpersonal dynamics of the organization are almost always more important than coding style.

Why one should ask to the original developer if the code can be modified?

If I see a code I want to change, I do it. I don't ask for permission. And colleagues can do the same with code I wrote. We trust each other.

If they were still there, why wouldn't you engage them about a modification you think was better than the original? Everyone learns in that situation.

they may not be 'on that project' any more, and if the burden of support is on you at that point, and you're the one dealing with the code, and they're not, them saying "meh, it was good enough" doesn't really help. I've hit that before, and mgrs have said "devX said it's good enough, they wrote it before, don't try to change stuff". Well.. they wrote it 2 years ago when there were 300 customer records, not the 14200 today, and they're not fielding support issues from people complaining about delays now.

There's plenty of reasons to not engage with someone. There's also plenty of reasons why it may make sense, and I think it's highly context dependent. For me, it's about 50/50. And... in the cases where I reach out, it's about 50/50 as to whether they have any time/inclination/memory/ability to help anyway.

Because it wasn’t necessarily an improvement on the original code. A discussion should have happened either during the original commit or before the refactoring commit.

Trust would also include to consider that there might be a reason for why the code is the way it is, even if it is not directly obvious.

A brief chat can then help to clarify things.

Agreed, it's always worth remembering Chesterton's Fence. https://fs.blog/chestertons-fence/

Two reasons, one technical, one social. Firstly, the original developer probably spent more hours thinking over and working on that bit of code than you have -- so maybe they had a good reason for doing it the way they did. You won't know unless you ask them. Secondly, I think it's a fairly natural and common human behaviour that when you spend a lot of time working on or making something that you feel some degree of satisfaction, pride and investment in the result. Even in a cooperative joint endeavour that is striving to avoid an idea of individual "ownership" of parts of the project, I think it can be helpful sometimes to acknowledge that feeling of investment by consulting the original author before walking in and radically changing something they did.

I've heard this many times... 'take ownership of your code' ... but the problem is the vast majority of devs work for corporations who own the code, and unless you're a shareholder that code certainly isn't yours, it's corporate property, that's the deal in most employment scenarios.

Realistically, people doing such work care very little about writing 'great code' because they know they have no real 'ownership'. There hopes for higher pay and recognition rely on climbing the corporate ladder by whatever means available. Team member, team leader, division manager, VP of whatever, etc. Blame bad results on someone else, that's the normal tactic for these types. Don't hold up production over code quality concerns, because delays in pushing product to market upset the shareholder board, which they see as lost profits.

The whole notion of a 'skilled technical individual who takes pride in their work because they own it' sounds like some awful corporate in-house propaganda campaign to be honest. And this accounts for much of the current mass exodus from the corporate workforce, I imagine.

I didn't say "take pride in your code because you own it", I said that because you feel some pride in what you've made you feel some sense of investment in it. And I wasn't trying to speak to legal ownership but to the kind of psychological feeling that might explain people getting a bit cranky if you waltz in and rewrite things. That doesn't happen to everybody or in all situations, but I don't think it's that rare.

The code I write is corporately owned in the legal sense but I still feel some attachment to it and care about good workmanship.

The other developer usually has reasons why they designed it that way. If you just rewrite other people's code, it makes it seem like you think there is One True Way to writing code and that you're smarter than them.

I'd ask first if they agree with a certain improvement. It shows I value them as an engineer and they might bring up critical information that I'm missing, making my "improvement" actually worse. It doesn't hurt to talk to people.

I just assume that they got one pass at it. Agile!!

I agree Agile steers a team towards hastily bodged together code.

The student said to the teacher:

"This fence is in the way and obstructing the flow, it should be removed"

The teacher said to the student:

"If you can tell my why someone made the fence in the first place, I will allow you to remove it."

Note that this has nothing to do with "code ownership". If you worked for me and randomly changed code that you did not like, I would fire you.

> If you worked for me and randomly changed code that you did not like, I would fire you.

From one extreme to the other? People make mistakes, educate them instead of brutally punishing them. Talk it through - isn't this exactly the mistake that was made by the developer (they didn't talk to their peers)? If they do not respond to feedback, that can eventually lead to a firing.

Of course I would talk to a junior dev right out of college and tell them these kind of "drive by" refactorings were counter productive and dangerously susceptible to the introduction of lack of context bugs... ONCE. The hiring process is too expensive and time consuming to punt on a junior dev or intern for such a transgression.

However, if the behavior persisted then yes, I would fire them.

If anyone sees code that they hate they are free to submit an issue and I will be happy to review it and prioritize it with the other work to be done.

If they hate code that smells so much that they want to volunteer their time to refactor it, then they can participate in code reviews for me instead.

> I would fire you

Ha ha ha. That's not how it works with me.

Then I guess I would fire you :-)

No you won't.

If I ever see you again rewrite someone else's code without asking, I will fire you on the spot.

Ironic. You learned nothing from the article.

So deluded. :D

i'd fire you for micromanagement, mismanagement and encouraging bad practices.

Years ago (2005?) I worked at a call center. Most agents were on one coast - data center was in another coast. Agents in our call center complained about speed. When they'd pull up an account, it was taking seconds... sometimes 15-20 seconds... to pull up the account.

This was all a bespoke CRM system. I poked in the code (it was something I had access to) and noticed that ... the entirety of the whole screen was duped - the output of the client's history was embedded in an HTML comment tag. So... if the client had, say, 3 years of info/comments, it was rendered, then rendered again in HTML comments. It was a single line duplicating the entirety of the info. It was obviously a debug remnant. I removed it. I tried to talk to the original developer beforehand - he was on the phone, and kept waving me off. I emailed him. No answer. I committed it and got it to a testing server. The test guy immediately loved the speed improvement, and we got it out to the floor. There were about ... 50-70 agents live at any one time, and everyone's hold/wait times were cut by 20-30% overnight. Clients and agents were happier. I found out later that, over time, we cut bandwidth bills by a measurable amount.

I was raked over the coals and nearly fired for that. "unprofessional", "insulting", etc. It was primarily because I'd made someone else look bad. Other people on the team also made changes now and then to each others' code without asking permission, when it was needed/obvious (emergencies/etc). It was a simple oversight in removing some debug code, and it made a huge difference (it had been tested and had other eyes on it as well - this wasn't "change prod by hand without telling anyone").

Still bugs me to this day (as you can tell).

You mailed the original developer, he didn't answer. And you were fired for doing your job right. What you must conclude is that they don't care about quality but about ego. They have a philosophy that don't favor quality. You should be glad they fired you.

I wasn't fired, just... the threat was raised. That original dev left soon after (in no way related to that - he'd been looking around for a while from what I heard). It was a strange situation because in the group of... 8 or 9 of us, I was the last one in, and got along OK with everyone, except him.

The point your parent post was making was that regardless of whether or not you actually got fired doesn't actually matter. The signal you should be paying attention to (which you are surely aware of, since you say this episode still bothers you) is that the environment was a toxic one so if they did fire you they would have been doing you a favor. At least from a silver linings point of view.

A lot of people attach their ego to the work that they produce. I did when I was younger and would try and defend poor choices because I had made them. They were my children. Once I put that aside and separated my ego from my code, I got much more comfortable pointing to my own mistakes and learning from them. I am a far better engineer and teammate because of that.

There's a big difference between fixing a problem that's been in production for two months, and rewriting code your coworker pushed two hours ago. For one thing, your coworker might be still working on the next iteration of the code, and adding an unnecessary diff could easily cause hours of wasted time.

The reasonable thing to do is often to add the original developer as a code change reviewer. They should know about what the code is doing and about any pitfalls involved. If they don't like the changes, then it is easy to take the discussion then.

And... > It was already late at night.

Probably not the best time to refactor someone else's code that currently works.

Reply on news.ycombinator.com