I agree that not every "smart approach" is worth it if you sacrifice legibility.
But I don't think you necessarily need to ask permission to refactor code. On the projects I had the past couple of years everyone understood that code was open to be changed by anyone.
In practice we'd often ask "why did you do X" instead of just rewriting it.
I trusted the people I worked with on those projects though and if they thought they had to rewrite part of my code for whatever reason, that was fine by me. And vice versa, I changed their code and that was fine.
Well in his case, did refactoring the code to clean it up add business value?
There has to be a very good reason for me to refactor existing/working code for instance for performance.
I won’t refactor code to reduce existing duplication but I will refactor code if I see there is some functionality that I need elsewhere so I won’t just copy and paste.
Well, in that case, why do it now instead of waiting until “the future”?
If it’s needed in the future, the cost is not more than it is now. If it isn’t needed in the future, then he’s wasted his time. If something similar is needed, but it needs to be refactored in a different way, they are going to have to refactor it
There is no special case “trap”. Business requirements necessitated changes.
> Well, in that case, why do it now instead of waiting until “the future”? If it’s needed in the future, the cost is not more than it is now.
They're familiar with the code and its gotchas when it was written. In "the future" most of that context will be gone (people forget stuff, and change companies/teams/roles).
Better to get rid of the gotchas before you get burned.
> If it isn’t needed in the future, then he’s wasted his time.
The new version was still more readable to newcomers, and resizing stuff is pretty core functionality for a drawing program.
> If something similar is needed, but it needs to be refactored in a different way, they are going to have to refactor it
A second pass of refactoring tends to be easier than a big ball of mud. And in the worst case, they can always inline it and start over from the same state.
> There is no special case “trap”. Business requirements necessitated changes.
When requirements change you can either start adding special cases or rethink how it fits into the bigger picture. That matters in every layer, but it's especially important in the UI layer.
Unless you want to end up with an unusable Apple-style hodgepodge. In that case, go wild, I guess.
It depends on context. Ideally you have an automatic code review system that would remove a lot of these discussions and create sense that everyone is involved in the code bases.
In this context however it is pretty special because it it was a single commit that that he immediately refactored. In that case the more reasonable approach in my opinion is to take a post-commit code review where you talk to the dude about your thoughts how one can improve the code. It was also a pretty significant change when it comes to coding philosophy, so if you think you know more it is also a potential opportunity to mentor your colleague (or learn something yourself)
If it would have been a bug fix or an added feature it is no problem. Or if some time passes and most code is written in style X but this was not and then you rewrite it, also fine.
But I don't think you necessarily need to ask permission to refactor code. On the projects I had the past couple of years everyone understood that code was open to be changed by anyone.
In practice we'd often ask "why did you do X" instead of just rewriting it.
I trusted the people I worked with on those projects though and if they thought they had to rewrite part of my code for whatever reason, that was fine by me. And vice versa, I changed their code and that was fine.