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.
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”.
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.
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.
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.
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.
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.
> you shouldn't assume that.
He didn't say what his reaction was. You assumed it.
Am I missing something?
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.
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.
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.
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.
Plus even the initial rewrite is more time-consuming to do if he doesn't discuss it with the original author beforehand.
A rewrite of someone else’s code? Most definitely.
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!
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 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 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
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.