Is it rude to refactor/improve team members' code?

The real answer to your question is MAYBE. You must conform to the practices of your team, and each team is different. I refactored the top responses to generate some pros and cons for this topic, and opened this response up as a community wiki PROs Code is subordinate to goals You can change code of others.

You have the right attitude. No one owns the code It's rude (and over time, even dangerous ) to let problems slide without changing them It depends on the results. If the new code is demonstrably better (faster, fewer resource requirements, fewer bugs, more legible), then people would be foolish to object.

I've had my code improved by more knowledgeable co-workers and been grateful Change if there is a compelling reason Don't change something unless there is a compelling reason to do so. But if there is one, I seldom discuss it Be prepared to explain your changes. If the other programmer does not agree with them, he either gets to keep his version, or you duke it out with the team lead Teamwork If you can change the code, it is a sign of the strength of the team This will only work if the team is really gelled as a team Learning to do this with humility can really help a team gel On the team I work on, it's not rude at all.

We practice collective code ownership -- so if anyone encounters something that needs to be improved, they are empowered to fix it Standardize coding practices Promote good coding practices by sharing them. Communicate with your colleagues to understand why something was done a certain way instead of just changing it. Sometimes there is a reason why something was done in a suboptimal way If the changes are minor/trivial, fix it on the spot.

If its a major change (large scale refactor/design change), best to discuss first with the concerned person who implemented the code/lead before digging in to make changes CONs Beware: Changes aren't guaranteed to be error-free It is important to know whether you are correct about the improvements. If you do things like this (even if they are right) without any team consultation, you are potentially a problem on the team Sometimes after you improve something, it changes back later. This is a sign that not everyone on the team has a common understanding of the situation of the code in question.In this case, the appropriate action is to have a conversation and build a common understanding (which might not match your initial opinion) and then decide what to do It takes too much time It takes too much time to discuss changes, but if you do make a change provide comments explaining the change.

Or be prepared to discuss the change if people ask you about it later If there is lack of openness and inability to reach a consensus agreement on a design issue, this will be difficult and potentially destructive If you don't have anything more urgent to do at the moment then refactoring is okay. If you have a pile of work to do and get side tracked refactoring code which isn't relevant to your current task then you might just be wasting time Egos That kind of thing would honestly bother me and I'm sure it would bother others in the team if I were doing it. That is why we do code reviews and offer up suggestions to each other.

I think doing something like that after the fact can lead to bad blood among team members When dealing with egos in the office, you would probably be best served asking yourself the same question. If you had implemented something, would you feel that it was rude for someone else on your team to re-write it without letting you know? At the extreme would be someone who is neglecting their own task/bug list, and starts inserting bugs into other peoples code while thinking they've improved it.

That kind of trick will get you despised, and hopefully fired.

The real answer to your question is MAYBE. You must conform to the practices of your team, and each team is different. I refactored the top responses to generate some pros and cons for this topic, and opened this response up as a community wiki.

PROs Code is subordinate to goals You can change code of others. You have the right attitude. No one owns the code.It's rude (and over time, even dangerous) to let problems slide without changing them.

It depends on the results. If the new code is demonstrably better (faster, fewer resource requirements, fewer bugs, more legible), then people would be foolish to object. I've had my code improved by more knowledgeable co-workers and been grateful.

Change if there is a compelling reason Don't change something unless there is a compelling reason to do so. But if there is one, I seldom discuss it.Be prepared to explain your changes. If the other programmer does not agree with them, he either gets to keep his version, or you duke it out with the team lead.

Teamwork If you can change the code, it is a sign of the strength of the team. This will only work if the team is really gelled as a team. Learning to do this with humility can really help a team gel.

On the team I work on, it's not rude at all. We practice collective code ownership -- so if anyone encounters something that needs to be improved, they are empowered to fix it. Standardize coding practices Promote good coding practices by sharing them.

Communicate with your colleagues to understand why something was done a certain way instead of just changing it. Sometimes there is a reason why something was done in a suboptimal way. If the changes are minor/trivial, fix it on the spot.

If its a major change (large scale refactor/design change), best to discuss first with the concerned person who implemented the code/lead before digging in to make changes. CONs Beware: Changes aren't guaranteed to be error-free It is important to know whether you are correct about the improvements. If you do things like this (even if they are right) without any team consultation, you are potentially a problem on the team.

Sometimes after you improve something, it changes back later. This is a sign that not everyone on the team has a common understanding of the situation of the code in question. In this case, the appropriate action is to have a conversation and build a common understanding (which might not match your initial opinion) and then decide what to do.

It takes too much time It takes too much time to discuss changes, but if you do make a change provide comments explaining the change. Or be prepared to discuss the change if people ask you about it later. If there is lack of openness and inability to reach a consensus agreement on a design issue, this will be difficult and potentially destructive.

If you don't have anything more urgent to do at the moment then refactoring is okay. If you have a pile of work to do and get side tracked refactoring code which isn't relevant to your current task then you might just be wasting time. Egos That kind of thing would honestly bother me and I'm sure it would bother others in the team if I were doing it.

That is why we do code reviews and offer up suggestions to each other. I think doing something like that after the fact can lead to bad blood among team members. When dealing with egos in the office, you would probably be best served asking yourself the same question.

If you had implemented something, would you feel that it was rude for someone else on your team to re-write it without letting you know? At the extreme would be someone who is neglecting their own task/bug list, and starts inserting bugs into other peoples code while thinking they've improved it. That kind of trick will get you despised, and hopefully fired.

Cool, cheers. I'm just going slightly off topic, but I'm not entirely sure what the deal is with community wikis...you think this question should be marked as one? – andy Jul 1 '09 at 7:06 I collected a bunch of responses to your question and organized them.So since it's a community wiki, votes for this response will not give me any reputation points, but anyone can edit it.

I don't suppose the question needs to be marked a community wiki, unless you want it that way. – Tom Anderson Jul 1 '09 at 7:43 1 @Andy, Let's say that the question is not a question that can be answered in a simple manner. More discussion oriented so to speak, and yet a question that the community finds important in some way.

– Magnus Skog Jul 1 '09 at 7:44 Another important reason to change code as soon as you spot bad parts is that many times programmers look at existing code and copy it to do new things. I'm not just talking about blindly copy/pasting, but also about just peeking into other parts of the code to see how certain things are done. If a bad practice is present in code that isn't addressed, you'll run the risk that it spreads over the whole code base over time.

The least you can do is clearly comment the code that a bad practice is being used and that it or its style should NOT be copied. – Arjan Tijms Jul 1 '09 at 9:13.

You have the right attitude. No one owns the code. However, be prepared to explain your changes.

If the other programmer does not agree with them, he either gets to keep his version, or you duke it out with the team lead. Generally I don't change something unless there is a compelling reason to do so. But if there is one, I seldom discuss it.

I don't have the time to discuss every little change I make to the code. However, I do provide comments explaining the change.

1 +1 for "I don't have the time to..". Complete agree with you. It would mean we wouldn't do anything else but discussing changes here and there.

– Magnus Skog Jul 1 '09 at 7:40 2 +1 for "don't change something unless there is a compelling reason to do so. " – Steve Rowe Jul 1 '09 at 8:05 2 Perhaps it might be better to explain the change in the check-in comments (assuming source control) rather than the code itself. The comment is only relevant to that particular change, so why clutter the code with comments that aren't relevant to its current state?

– Ant Jul 1 '09 at 9:29 +1 for the pragmatism – annakata Jul 1 '09 at 11:43.

On the team I work on, it's not rude at all. We practice collective code ownership -- so if anyone encounters something that needs to be improved, they are empowered to fix it. Sometimes you may find that after you improved something, it changes back later.

This is a sign that not everyone on the team has a common understanding of the situation of the code in question. In this case, the appropriate action is to have a conversation and build a common understanding (which might not match your initial opinion) and then decide what to do.

This will only work if the team is really gelled as a team. If there is lack of openness and inability to reach a consensus agreement on a design issue, this will be difficult and potentially destructive. I would take whether you can do this as a sign of the strength of the team.It is also important to know whether your judgement is correct in your reasoning about the improvements.

Regardless, if you do things like this (even if they are right) without any team consultation, you are potentially a problem on the team.

A field called Software Engineering can be applied very effectively in your case. How about introducing a design review & code review in the development process? And read this whitepaper for code reviews : smartbear.com/docs/BestPracticesForPeerC....

If you don't have anything more urgent to do at the moment then refactoring is okay. If you have a pile of work to do and get side tracked refactoring code which isn't relevant to your current task then you might just be wasting time. Also you should communicate with your colleagues to understand why something was done a certain way instead of just changing it.

Sometimes there is a reason why something was done in a suboptimal way. If there really isn't a reason but you have communicated with your colleagues regardless, the benefit is that they have learned something. That way in the future the same mistake won't be made.

You don't want to be the cleanup guy. You want to promote good coding practices by sharing them.

On my first full-time programming job ever I was corrected a lot by other team members. They would enlighten me on standard practices and conventions used in the company. I would think that changing someone's code without letting them know that you've done it is not only rude but also stupid (they may have to come back to the code to add something that their particular design accounted for and realize that they need to undo work that you've done.) Typically whenever someone at my work wanted to change someone's code they would politely ask why they implemented something the way they did.

Or why they didn't choose this particular way of doing it. You won't always be right nor will the other person always be right. The best way (imho) is to politely ask why a certain implementation was chosen.

1 +1 "No one owns the code" sounds great, but in a lot of situations, there was a reason it was done a certain way that you can't see, or, at least, complications making a change rather difficult. – David Berger Jul 1 '09 at 4:15.

That kind of thing would honestly bother me and I'm sure it would bother others in the team if I were doing it. That is why we do code reviews and offer up suggestions to each other. I think doing something like that after the fact can lead to bad blood among team members.

When dealing with egos in the office, you would probably be best served asking yourself the same question. If you had implemented something, would you feel that it was rude for someone else on your team to re-write it without letting you know? In any case, if you find yourself changing others' code often, you'd be doing yourself a favor by participating in code reviews (of their original and your fixed version).

Otherwise your teammates will not progress and you will constantly be doing this sort of work.

It depends on what you mean by "refactoring. " Some programmers that I have managed feel that they can't work on code unless they've "fixed" the indentation style, made the variable names consistent with their personal style, changed classes into structures (or structures into classes), ... You get the idea. Other programmers I've managed feel that it's their mission to get the code to work, and to change as little as possible, because every time you change a line of code, you create the chance of a new error where an error didn't exist before.

The problem is that refactoring is frequently needed --- not just to get code to work, but to make it a strong foundation for future development. In my experience, this is where "team programming" or "pair programming" really excels. Sitting down the original programmer and another programmer together, having the first programmer explain the decisions, and having the second programmer make comments --- this tends to remove the ego component and get people to focus on the work product.It's also a great way to do internal training.

I would consider it rude for you to let things like that slide without changing them.

Nay. However, are all your assigned bugs fixed and peer reviewed? If no, then you have better things to do than refactor other parts of the code base.Is the refactoring done to "thump your chest?

", then it is rude. If you think there are lots of refactors that need to be done and you are not the system architect/designer, {what ever that means}, then I would take up your refactors with the group so you can get buy in to your changes. There may well be reasons why certain pieces of code are written a particular way.

Of course some documentation as to why this is the case might help.

I think that different teams have different norms and expectations. Explicitly establishing norms can go a long way to resolving these kinds of issues. If everyone has agreed beforehand that it's OK to improve other people's code, or that these issues should be addressed in reviews, that makes it a lot simpler.

If it's inappropriate in your team to fix crappy code, you've got a problem.

I think that the best place for this is in a code review. If you have such a thing, you can get them to give you the go ahead, or the reason for why the code is the way it is (it may not be obvious at first glance, in which case they should have at least provided a comment). That way, you're doing your critiques of the code in an environment set up to give out critiques; no one should feel especially put upon, and you can probably avoid potential rudeness.

Who knows, they may feel the same way about your code, so a code review may help you as well.

This is a very difficult thing. My opinion on this is that some times it is okay sometimes it is not. Codeownership is one of the topics that should be communicated in the team and if the whole team is fine with the changing then it is ok.

But changing without having the permit is rude. The best thing would be to get a general permit for the whole team and document this at some place, maybe together with other guidelines like code style etc.

When I see something I don't like I always ask myself. "Is it code style, or will this perform faster/safer if I change it? " If it is code style I just leave it be, and ask why during a code review.

If the code is "unsafe" sometimes I will change it right away, other times I let the developer know why it should be change and try to get them to see the light, and other times I let it go until the code review and bring it up there. To answer your question as long as you are in a position where you mentor younger developers I do not think it is rude. Part of my job is to mentor and part of that is pointing out things that could be done better.

You all should be making each other better and have a constant flow of information. That being said, there are trade-offs. Not every piece of code needs to be completely critiqued.

If it is good enough, no need to waste time on it or critique it unless the individual truly is looking for the criticism.

No its not rude. Show some one a better way is a great way to have learning occur. But also be open yourself to feedback in the follow up discussion.

Need to avoid unproductive personal style refactoring back and forth.

I absolutely do not think so. However it is good to follow certain obvious conventions in my opinion: -If the changes are minor/trivial, fix it on the spot -If its a major change (large scale refactor/design change), best to discuss first with the concerned person who implemented the code/lead before digging in to make changes.

It depends on the results. If the new code is demonstrably better (faster, fewer resource requirements, fewer bugs), then people would be foolish to object. I've had my code improved by more knowledgeable co-workers and been grateful.

If the new code is arguably better, then you ought to have argued for it in a code review rather then just unilaterally changed it. Who knows? You might be unaware of some obscure requirement and inadvertently screw things up.At the extreme would be someone who is neglecting their own task/bug list, and starts inserting bugs into other peoples code while thinking they've improved it.

That kind of trick will get you despised, and hopefully fired.

I think you absolutely need to talk to the person before you tell them you did it, or check in the changes. And if you can't explain sufficiently to be sure they won't be surprised, don't do it. If they don't support your changes, it only takes one minor problem to make your changes blow back at you.

There's good reason for this in that they're likely on the hook for the code working. You can still refactor the code though, just don't tell them and don't check it in. Its likely the process of your refactoring the code will deepen your understanding of how the original code works, and it may reveal some actual bugs.

Report the bugs in a productive manner and you'll gain the persons trust for future changes. Some shops may be more open to other people churning code for no reason, some shops also have sweet code coverage so it'd be a mute point.

It’s not rude, but you'd have to make sure that your changes are an improvement to the original code, and that you explain your reasons to the individual who originally wrote it. Don’t forget, they could also benefit from being shown a better/more efficient way of coding things. Of course, it can get very political changing other people code, especially if they feel that 'they' own it.

Sometimes in this job it feels like I need to have a degree in politics! ;).

It's your moral and technical responsibility to improve the code you see, if you see a way to improve it. Even at the cost of causing friction. Nobody owns code; programmers have to learn to be as close to egoless as is possible.

If you're the lead developer, obviously this is much easier. But even if you're the most junior person on the team, you should at least advocate changing the code. The key issue is changing things without seeking permission or approval, in code you don't have responsibility for.

This is dangerous and asking for trouble. It's actually much better to tell people what they should be doing.

It's not rude at all. Our aim is to get the best possible project - apart from the fact have do we achieve it.

No. It is not rude to 'refactor/improve team members code'. However, it would be rude if you weren't willing to deal with the bugs linked to or introduced by your change.

Agreed, though that goes without saying I think. Of course, once I change code, although I don't own it, I know share in the responsibility – andy Jul 1 '09 at 23:31.

It really depends on the personalities, responsibilities, and org culture...and, just as importantly, the nature of the changes being made and the stage of the project. At one end of the spectrum, assume you're on a team which shares responsibility for a chunk of the codebase: if you're fixing actual problems which have been agreed-to-be-fixed-now (or are in the scope of the current work), then that's awesome. In any case I want the 'changing' team member to communicate what they're changing -- depending on the nature of the change, informally is generally fine...but in any case, to a level of detail which is appropriate to the nature of the change.At the other end of the spectrum, "fixing things" can be very rude if the stuff you're fixing isn't an appropriate use of time given the state of the project/component (e.g. , we need to be doing X and Billy is off making tweaks to unrelated code for Z).

Alternately, in a setup where a person/group of people own (as in have responsibility and authority for) a certain chunk of functionality, coming in and "improving" it unilaterally can lead to hard feelings: someone "playing outside their sandbox" or being a "know-it-all." This is especially bad if the changer is doing "personal preference" style changes. At the end of the day, one thing engineers sometimes lose sight of is that in the real world there is no "perfect code" -- there's always SOMETHING ELSE you can do/fix.

And even if there IS such a thing as perfect code, nobody on the business side cares/is willing to pay the high price for it."Good enough" is the name of the commercial game. The art of engineering is -- once again -- striking the right balance. In this case, between fixing things and letting "good enough" be good enough.

Agreed. Though I guess in my case, it isn't about perfect code. Rather, it about building APIs which just make sense intuitively, because in a team, you want the particulars to be abstracted.At least, thats the case I was referring too specifically.

But of course, the question covers much more than just my one scenario – andy Jul 1 '09 at 23:30.

It is rude not to. To not improve bad code is rude to: - The project head who put his trust in you. The company that gave you a job.

The customer who paid for your project. And above all, it is rude to the art and craft of programming that you practice and revere.

I cant really gove you an answer,but what I can give you is a way to a solution, that is you have to find the anglde that you relate to or peaks your interest. A good paper is one that people get drawn into because it reaches them ln some way.As for me WW11 to me, I think of the holocaust and the effect it had on the survivors, their families and those who stood by and did nothing until it was too late.