> If someone spends a week or two writing a patch and you come in and rewrite it in an evening, that, in and of itself, is telling me something: You think your teammate is a worse coder than you, given you were able to solve it with "cleaner" code. You assumed that your solution was better, without talking to the person who authored it to see if they did things that way for a reason.

The time they spent developing the solution is a sunk cost. The only thing that should matter is whether the afternoon rewriting it is the most productive use of your time.

Also, rewriting something is far easier than writing it from scratch. Just because you can rewrite it quickly doesn't mean the original developer didn't save you a bunch of time by letting you see a workable first pass.

The same gut reaction that causes you to go "ugh, crap code, I'm going to rewrite" is probably similar for the other developer, but with your rewritten version.

People tend to have an aversion to code they didn't write in general, and actually reading and understanding existing code tends to be a rare skill.

> Also, rewriting something is far easier than writing it from scratch.

Given the amount of times I see companies rewrite their product with half the features missing and more bugs than before, I'm not sure I agree with that.

Yup, the upfront effort to come to an implementation is like 90% of the work. Refactoring existing code is a lot easier than juggling product, design and technical requirements into a functioning solution. All the moving parts are already there to look at.

> If someone spends a week or two writing a patch and you come in and rewrite it in an evening, that, in and of itself, is telling me...

Or, what it might be telling you is the second person wouldn't have been able to clean it up in an evening without the first person having already spent a week or two on it. The second person was building on the first. Just because it was (hypothetically) less lines of code doesn't mean it could have been written without starting with the more lines of code and refactoring; it often takes more time/steps to get to fewer lines of code.

But don't get me wrong, I'm in favor of colleagues discussing code, not just changing each other's code without discussing it.

This blows my mind. If I had to discuss with my teammates every time I changed their code I would be way less productive. Everyone’s a little bit watched to the code they write, but iterative improvements and bumping the quality of the codebase is way more valuable than risking a fragile ego. It’s the teams code, we all benefit from improving it.

You can certainly go too far in either direction (talking or not talking), but to the extent talking matters when it does, it's not about "ego", it's about 1) "What did they know that I don't" and 2) Building shared understanding of the context and decisions and architecture.

As the OP says "A healthy engineering team is constantly building trust." Sometimes you already have enough trust and shared understanding to not need a discussion, other times a discussion is good.

I've humbled and have been humbled before because sometimes what looks like an ugly, unclean solution is the correct solution. There might be weird edge cases in the system that you catch, but the person doing the rewriting doesn't see until they push the code out and it breaks something in an entirely different part of the codebase.

That's the biggest reason why you should always, always discuss those changes with your coworkers first instead of going cowboy on it.

Or have tests. If your push comes with tests and I reduce the code by half while still passing all of them it means that 1). you didn't actually test everything you've done 2). you didn't write code as well as you should have 3). I'm an idiot and made the code less robust.

1) happens all the time. 2) happens some of the time 3). happens as often as 2.

This seems more like a post-hoc justification than any real rationale for change. There's nothing stopping your new code from introducing new testing requirements that weren't needed for the original code and it sounds like for 1) it could be equally a case of you blaming another developer when its actually 3) that occurred.

This also assumes that everything is actually testable, sometimes you write unclean code because the underlying framework or module you're working with has an entirely separate issue that you have to work around. Which again, might not show up because your work around code sidesteps the underlying issues.

This all boils down to the fact that you should just talk to the developer and see what they had in mind rather than assuming that you're right and they're wrong by rewriting it yourself.

Well, perhaps you wrote the code quicker because your colleague already wrote all the tests, and it was easy for you to verify the behaviour of your complicated-looking but short code was correct.