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

Question

When working on a single project with a small team, say, 3 devs, its common for us to ask each other:

"Oh, how does this class work?" or "What property do I set on this to make this happen?" as the code base grows, and of course, we must make use of the available APIs, classes, controls, et al. that we build.

However, I'm sometimes unsatisfied with some implementations, but I don't want to "tell" other people what to do.

Sometimes it's something as small as "instead of changing 2 properties to make something happen, why not just call a method with 2 parameters?" That kind of thing.

Other times it's something slightly bigger, like the whole way dialog boxes have been implemented throughout the app.

In these instances, I sometimes find myself just going in there once it's all been checked in, and changing the code, and then communicating that I've just changed this or that, and why.

Is that rude? By no means am I saying I always know better, it's purely a case by case thing.

My feeling is that I'd want to work in environments where others feel free to improve on what I've done, as long as they communicate it to me afterwards. Not because I "own" the code, but because I'd like to learn.

Yay or nay?

UPDATE

Thanks for all the input. I wanna address some specific answers when I get some time.

Meanwhile, the main feeling I'm getting is that, as a Team, we really need set some expectations as to how we treat each others work. Whether it be through code reviews, or we just say, "look, its a free for all!". Not that we would do that, but then at least expectations are set.

Answer 1

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.

Answer 2

Isn't the whole idea of working on a team to 'synergize'? 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.

Answer 3

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 : http://smartbear.com/docs/BestPracticesForPeerCodeReview.pdf

Answer 4

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.

Answer 5

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.

Answer 6

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

Answer 7

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.

Answer 8

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.

Answer 9

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.

Answer 10

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.

Answer 11

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.

Answer 12

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.

Answer 13

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.

Answer 14

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.

Answer 15

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.

Answer 16

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.

Answer 17

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.

Answer 18

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.

Answer 19

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.

Answer 20

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.

Answer 21

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.

Answer 22

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

1. You can change code of others. You have the right attitude. No one owns the code.
2. It's rude (and over time, even dangerous) to let problems slide without changing them.
3. 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

1. Don't change something unless there is a compelling reason to do so. But if there is one, I seldom discuss it.
2. 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

1. If you can change the code, it is a sign of the strength of the team.
2. This will only work if the team is really gelled as a team.
3. Learning to do this with humility can really help a team gel.
4. 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

1. 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.
2. 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

1. 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.
2. 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

1. 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.
2. If there is lack of openness and inability to reach a consensus agreement on a design issue, this will be difficult and potentially destructive.
3. 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

1. 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.
2. 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?
3. 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.

Answer 23

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!!!! ;)

Answer 24

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.

Answer 25

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.

Answer 26

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