Hacker News Reader

Comment by bjourne | Hacker News Reader

> 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 disagree one million percent! If you are on my team and you want to rewrite code I wrote (cause it sux) then do it! Don't ask, just DO IT! DO IT NOW! Have a blast, tear it apart, rewrite all my shitty abstractions and see if you can do it better. If the result is better code, then awesome! I learn something and the software gets better. If the result is worse code, then awesome! I'll tell you why, you'll learn something and your improved understanding will allow you to write better code.

I also think that a salaried engineer who thinks that a piece of code he or she (but almost always he) wrote is "his" or "hers" is totally wrong. It's the company's code. Having a false sense of ownership towards that code will just cause grief for that engineer and friction for his or her team.

View on HN

> I also think that a salaried engineer who thinks that a piece of code he or she (but almost always he) wrote is "his" or "hers" is totally wrong. It's the company's code.

While this is _technically correct_, this isn't how humans work. Humans attach their worth to things they do, even when they shouldn't. It's a difficult thing to avoid to most people so if you write some code, commit it and then later see a teammate completely rewriting it, it can come off as either:

1. You spent a lot of time and hard work on code that you kinda identify with (I mean, it's code you wrote. It's your "art") and then someone comes in and re-writes it without even asking you about it can make it feel that they think you're an idiot.

or maybe even worse

2. They didn't know the reason you wrote it that way (maybe it was in support of future changes?) and now they just screwed it all up.

Maybe you haven't run into either of those situations. If you haven't, great! I spent the first part of my career hitting the first one because of how hard it can be to disassociate with the work you produce and the second one I see happening occasionally. It's a breakdown in communication within a team.

This is how some humans work. No one on my current team has ever expressed disatisfaction when their code gets changed.

Same here. And my own code is rewritten frequently as business needs change or better factorings are discovered when we hit the rule of 3, as mentioned in this thread.

It's nothing personal; we all get paid and go home. There's a lot of bruised egos in this thread that would be better spent in therapy, I think.

That’s the behavior of a junior engineer though. It’s a sign of an inferiority complex where any change feels like some kind of judgement on your ability to write it “correctly” the first time.

You are not your code, the code you wrote is not representative of you. Nobody who rewrites your code thinks that either.

You’re on a team working to build something larger than yourself. You’re not a bunch of painters sitting with your own easels in a room hoping to tape all of your paintings together in the end.

The best engineering teams I’ve ever worked on were groups of people that had no issues at all modifying each other’s code because we all trusted each other to do the right thing. The worst were the ones where each module had one “owner” who handled all of the changes to their module and “nobody else’s”.

The fix for this is to have a review system in place; you can then catch blatant code repetition before it goes in, and try to use your soft skills to guide the author toward a different solution such that they think it was their idea all along.

If you're the one who has created it and who touches that part of code more often than others then there is a notion of ownership: you don't want to accept changes that will get it into dead end and slow you down. That's how code review emerges. It's the same as the pull request system in the opensource development.

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.

> First of all, a larger change to code a teammate wrote has a ring of saying: it was badly written.

Not necessarily. In actively maintained projects code change all the time in some area, and if you are doing proper commits very often a piece of code does not magically appears in a perfect state but is rather improved in a series.

If you can improve the code you wrote, others can do too.

Code quality is not a binary thing. Moreover, if code quality actually has been improved, you should be happy with it.

Now how it is done is also important, but neither more nor less than any other human interaction.

But that is exactly my point: there are not only a technical but also social considerations. And yes, in the end a code quality improvement - if the refactoring actually is - should make one happy. But still it is the decent thing to check with the author first, if that is possible.

Several years ago, I wrote a big chunk of code to analyze engineering data from an engine test cell, and display a graph of the results. Someone else had done the hard part; I was merely coding up a gloriously-complex Excel spreadsheet in C++. I grabbed data from a MySQL database, and labeled the row data like: row[combustion_air_mass_flow] + row[fuel_mass_flow] * row[specific_gravity_of_diesel]. (Or whatever; it's been awhile. You get the idea.)

I had HUNDREDS of lines of calculations, and each variable was very clearly understood so that you could trace the whole process. The code was running and producing replicated results from the spreadsheet. We started trusting it to process new test runs, instead of copy-pasting into Excel.

The next morning, I came in to find the "other" programmer had stripped EVERY variable reference, and replaced them with the column numbers. There was absolutely NOTHING in the code that could help you understand that "combustion_air_mass_flow" was column, say, 54.

I turned to him and asked him what happened, and he said it was inconsistent with the rest of the code base. I was literally gobsmacked. I racked my brain for a response. In the awkward pauses, he admitted that my code was better, but he couldn't bring himself to use it, because that would mean that he would have to go back and recode every other place that worked like that, and there were many.

He was the guy responsible for most of the system; I was just writing this part because I'm an engineer who codes, and could understand the actual science going on. In the end, I re-replaced my equations with my previous code, and wrote another couple hundred lines defining column number to engineering variable, to "translate" his column numbers to something that made sense.

So, no, I don't believe in "do it; don't ask."

And, if, by some chance, you see this, Chris, I still think that was the weirdest flex I've ever seen.

This reminds me when a developer took over my codebase while I was on holiday. When I returned I had discovered that he converted all tab indents to spaces across the entire project. He completely destroyed my ability to perform diffs against earlier commits, because his preference was evidentially more important. Of course this was all justified with a link to Google’s coding style guide.

The commonality in both your and the parent's situation is that you reacted defensively. Your response was "You destroyed my tabs!" when a proper response would have been to try and understand your coworkers motivation. Perhaps he had some really good arguments for preferring spaces over tabs? Tabs can be problematic in heterogeneous environments where developers with different tab settings and different editors work on the same codebase. Have you read the arguments in Google's coding style guideline?

Maybe your coworker was a fucknut that did it just to mess with your code. I wasn't there so I can't know. My point is that you shouldn't assume that.

Frankly, the justification doesn't matter here: A change of that scope should be discussed and agreed on with teammates before you even put up a patch, not committed while the primary developer is out of town.

> you reacted defensively. Your response was "You destroyed my tabs!"

> you shouldn't assume that.

He didn't say what his reaction was. You assumed it.

Am I missing something?

It destroyed your ability to exclude whitespace from diffs?

If this happened to be Python, there is no such thing. Two pieces of Python code that have different semantics can look identical under a whitespace-excluding diff, so you must not habitually use such a thing as your go-to comparison method.

For instance if we edit:

   if condition:                     if condition:
     blah                              blah
     blah                   ->       blah
     blah                            blah

then nothing shows under "diff -b" or "diff -w". With a different kind of language there will be a non-white-space difference due to the changing position of a brace or other delimiting token.

Otherwise, exactly the remark I was thinking of making.

If it was Python then it would be highly unlikely to have used tabs in the first place.

Replacing symbols with meaningless numbers is almost absolutely unjustifiable outside of the job description of a compiler.

I need to disagree with you, I've been in a few positions where one engineer suddenly decided to rewrite parts of the code base without any input from other engineers. It's a huge blow to team morale, and it gave me a fear of writing code in this team.

Every time I wrote a piece of code, I wondered how long it would be there for, I understand that code evolves, but seeing your code being rewritten after a week is no fun, and it's a huge blow to your confidence as well.

I understand that code needs to be rewritten, because of requirement changes, or different understanding of the original problem, but talk about it, bring it to the attention of the team as to why you feel we need to rewrite. Make sure that the team understands why, and sees the value in it.

Just rewriting code on your own is a big no for me, and to me, breaks the trust that we had.

> seeing your code being rewritten after a week is no fun, and it's a huge blow to your confidence as well.

The people doing the rewrite benefit from your work though. They might would have done the initial writing "worse" than you did.

It is good to have fresh views. I agree it is good to communicate, too.

The main thing is indeed the communication, it would solve most issues.

I know that programmers are humans but I think paying too much attention to people's feeling is what is dragging the IT industry down, if you have self esteem issues go see a shrink

It’s not about personal feelings or self esteem, it’s about trust within the team, and confidence that when you do something the team sticks to it. When a rewrite is required, the team talks about it and then acts.

> Every time I wrote a piece of code, I wondered how long it would be there for.

The fix for that sort of thing is to have a satisfying, enduring side project where you're the boss: nothing is upstreamed that you don't like.

Then you can stop caring if your code at work gets replaced, and how fast.

I care more about the way it gets replaced. I constantly learn from my code being replaced, and replacing code myself. However I doubt anyone liking code being pushed onto them. That’s how I feel when someone decides to rewrite code without anyone’s knowledge. They already did the work, so they’re expecting it to be merged in.

Reading this made me smile. TXR is obviously that for you, and I wish all software were developed with such care.

> Every time I wrote a piece of code, I wondered how long it would be there for, I understand that code evolves, but seeing your code being rewritten after a week is no fun, and it's a huge blow to your confidence as well.

Here's some tough love. If people frequently rewrite code that you write it is because they are stronger developers than you are. You, the junior developer, can either sulk about it and feel miserable or realize what an amazing opportunity you have to improve.

Swallow your pride and approach the person who rewrote your code: "Hey, I noticed that you rewrote the code I wrote last week. Mind telling me what was wrong with it so that I can learn from you?" If the person answers "No" or "I don't have time" that person is an asshole and you are in the wrong place. But if the person answers "Sure! Let's schedule a meeting in a conference room with a whiteboard this afternoon and I'll explain what was wrong with it!" then you are in a great spot!

> Just rewriting code on your own is a big no for me, and to me, breaks the trust that we had.

Not for me. I trust my teammates judgement. They know when it's a good idea to rewrite my code and when it's not. No point wasting time holding meetings and bikeshedding over minutiae over work that can be done in a few hours. 9 times out of 10 they will do the right call. 1 time out of 10 they won't and we revert - no damage done.

I think you’re missing the point as to why I might lose confidence. I’m fine with people being better than me, especially when they explain why they did certain things. But we work as a team, that means that before we do work, we agree on one thing. If suddenly an engineer decides a week later he wants to do something else, then that’s not cool. Even if he has a valid reason for it. Instead you talk to the team, and agree to change.

Again, there’s a reason why we work in teams, and not as individuals. I trust my teammates too, but rewriting code without any discussion breaks that trust for me.

Should every change be talked over? If not which ones?

If you run scrum, then every ticket should be talked over, and large changes should be discussed. Every change? No. Architectural changes? For sure.

A rewrite of someone else’s code? Most definitely.

You can discuss when the rewrite is done, that way it wastes less of your time: you can talk about non-hypothetical existing thing. If the discussion shows that the rewrite is bad then it's his problem: he has to fix or revert.

Plus even the initial rewrite is more time-consuming to do if he doesn't discuss it with the original author beforehand.

> Here's some tough love. If people frequently rewrite code that you write it is because they are stronger developers than you are. You, the junior developer, can either sulk about it and feel miserable or realize what an amazing opportunity you have to improve.

Why is this condescending tone and unfounded assumption acceptable here?

It's like, Well, lookie here, a stranger with a generic opinion! What an amazing opportunity for me to assume specifics and talk down to him! You're welcome!

> I totally disagree one million percent! If you are on my team and you want to rewrite code I wrote (cause it sux) then do it! Don't ask, just DO IT! DO IT NOW! Have a blast, tear it apart, rewrite all my shitty abstractions and see if you can do it better. If the result is better code, then awesome! I learn something and the software gets better. If the result is worse code, then awesome! I'll tell you why, you'll learn something and your improved understanding will allow you to write better code.

I see your point of view.

There is a need to detach from your code while still being passionate in programming.

The key is the mindset: programming so that people enjoy what you're programming and not just for the sake of programming.

Also, to note, the refactor presented should objectively serve the same cause/requirement better than current code.

I've been a lead myself here and what danabramov nailed is that he realized this:

> My code traded the ability to change requirements for reduced duplication, and it was not a good trade.

His refactor is a form of premature abstraction

> ...and you want to rewrite code I wrote...

I think the key to most of this situation is right there. It's also something the original author largely misses.

The key is this: want.

If you want to rewrite some code you find, then kindly leave that keyboard and go play with some toys. This, "wanting", is the actual origin of the author's anecdote. "I saw this and I thought it was bad and I felt the urge to rewrite it". Ok, but don't.

What is missing is knowledge on the circumstances of the code. You see some piece of code and you're only seeing that, the code itself. But why is it that way? Is this code supposed to be modified/extended/reduced/deleted in a near future? Frequently? Or simply, as a first question: Does this code need to be better?

And even if the answer is yes, then in what aspects does it need to be better? What does "better" mean for this piece of code? Did you want to just make it cooler looking because you didn't like it? Or have you actually detected that some of the needs this code was supposed to fulfil aren't being fulfilled? Of course, if you only now saw the code and do not know what those needs are then you're in no position to rewrite it, just because you want.

But then again, if there is indeed motive to rewrite it, by all means, do.

Following this idea will probably lead to having to ask, sure. But you won't be asking for permission, you'll ask because you need information before you can decide.

> What is missing is knowledge on the circumstances of the code. You see some piece of code and you're only seeing that, the code itself. But why is it that way? Is this code supposed to be modified/extended/reduced/deleted in a near future? Frequently? Or simply, as a first question: Does this code need to be better?

That's exactly why you should always be very careful refactoring old code that you don't fully understand. It might look like spaghetti code that contains a lot of weird artifacts, when in reality it once was clean code that had to be edited dozens of times to handle edge cases. Not that it can't be refactored, just that it's likely not to look that much cleaner if it's to provide the same functionality. Refactoring old cold just because it looks ugly is often a waste of time and money.

This is fine in some environments, but not all. I worked on a code base once that had over 150 engineers. Teams were broken up mainly along feature and UX lines, but there were a few cases where some features were used in multiple user experiences. I worked on one of those features.

There was an engineer on one of the user experience teams that didn't understand this and decided to rewrite a portion of our code to make it more performant in their UX. In doing so, this engineer introduced bugs into our feature that adversely affected other user experiences within the application, but was not apparent in their own implementation. Had our team known about this, we would have very easily 1. been able to point out the bugs 2. helped the engineer with a better solution.

This wasn't a small effort, the engineer's team received a requirement from their PM, groomed the ticket, architected the plan, assigned the ticket to the engineer to work, the engineer wrote the code, then had at least two reviewers on their team approve the change before merging it. So we're talking multiple points of failure, I don't blame the engineer individually.

The fallout came a week after the code was deployed when the UX team flipped on the feature toggle. They checked their UX, it looked good, and continued on. Meanwhile other UX teams started seeing crashes coming from our feature. (the crash didn't manifest in testing because it was toggled off, which was a failure on the UX team)

This wrecked collaboration and trust across our codebase in multiple ways and lead to increased overhead processes. We dealt with the initial fallout from UX teams not trusting our feature, which then evolved in to UX teams not trusting each other, UX teams not trusting feature teams, feature teams not trusting anyone, an no one trusting the procedures in place.

Yes there were multiple failures. But at the end of the day, this whole scenario could have been prevented with a one line message: "Why did you implement this in this way?" No one was emotionally invested in the code that was changed, it was for all intents and purposes crap code, but the crap code worked, the new code didn't, a simple courtesy check would have saved a ton of time, money, and trust.

If you are on my team and you want to rewrite code I wrote (cause it sux) then do it!

Sure, rewriting is fine, but ideally it would come in the form of a PR rather than being checked in, so if you had valid reasons for writing it that way, it can be reverted before anyone else layers code on top of the bad rewrite.

clearly these guys are trunking though, and when they have decided to go that way, its part of the trade-off. Has its pluses and minuses.

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

I find your position a bit idealistic.

Redoing work you just did is tantamount to critisicm. I agree that everyone should welcome constructive criticism, but some tact is necessary in applying it.

> I also think that a salaried engineer who thinks that a piece of code he or she (but almost always he) wrote is "his" or "hers" is totally wrong. It's the company's code. Having a false sense of ownership towards that code will just cause grief for that engineer and friction for his or her team.

I think some places value a sense of ownership because that person then becomes responsible for maintaining that code and reviewing changes. There is someone accountable for it. Not that they have any special rights to that code.

Please don't consider refactoring or rewriting equivalent to redoing. Refactoring or rewriting means that I'm adding my name to the code, but I'm NOT removing yours. You still did the hard work of coming up with the original functionality even if it is later changed. Refactoring does emphatically not mean that your original work had no value.

Like if someone were to change all my spelling and grammatical errors in my top comment. I wouldn't mind at all. Hell, someone could even rephrase the comment entirely and remove the weak points and emphasize the strong ones as long as the main message is the same. I'm not a native English speaker so I'd probably learn from the experience.

> I also think that a salaried engineer who thinks that a piece of code he or she (but almost always he) wrote is "his" or "hers" is totally wrong

This. If some developer get offended when there is some issue in their code or the way it got implemented either they are not mature enough or there is a cultural issue in the team.

The point is, code "ownership" shouldn't be understood as a term of property - of course it isn't the property of the programmer but the company - but in terms of responsibility. This might be different in very large teams, but usually, once you write a piece of code, you are the the prime responsible person for maintaining it. And as long as this is the case, I would expect to be involved in any significant change to the code. Of course I am fine with changes, which also transfer the responsibility :)

What projects is everyone working on that they have the time to keep rewriting each other's code all the time? :)

If such teams did code review, maybe they'd get it right on the first commit and would only have to refactor when the requirements change or there's a clear benefit.

I get giddy with excitement whenever I see someone has rewritten/touched my code.

That sense of "ownership" in some small piece of the product? Helps build loyalty to the project, and the company. Seeing my baby with that brand name on it helps me feel a part of the team. I've left many a project that didnt offer me the loyalty of letting me fix my own mistakes (after some guidance).

Toe-stepping aside... How else are you going to develop this apparently inefficient coder without including them into the process of reforming this code? This isnt just about ego, this is about skill. This is about the code Im gonna write in the future. And here's your opportunity, written in our codebase.

Unless, of course... Im disposable as your coder.

Reply on news.ycombinator.com