We are practicing collective code ownership. To my understanding this means that any developer can change any line of code to add functionality, to refactor, fix bugs or improve designs.
But what about a complete rewriting of code from a developer who is still in the team? Should I ask him first? What is the best practice?
I think good communication is always the best practice. Talk to the developer and see if there's a reason why it's coded the way it is. It may be that they have been meaning to get back and refactor it for ages, it may be that they did it that way for a very good reason, or it may be that you both can learn something from the conversation.
Going in and rewriting without prior communication is a recipe for ill will.
The very premise of this question raises a number of concerns for me that I don't think any of the existing answers are adequately trying to address. Let me ask some follow-up questions here:
Are you absolutely, positively certain that you are talking about rewriting and not refactoring? By definition, changes to code style or structure that result in an improved (or simply different) implementation without changes to the external behaviour is a refactor, not a rewrite. Breaking a monolithic 500-line routine into a set of 50 subroutines may involve writing quite a bit of new code, but it is not a rewrite. The term rewrite implies throwing away an entire product or feature and starting over from scratch; it is not something to be taken lightly.
If the original code's behaviour is so wrong, or the implementation is so buggy as to require a full-fledged rewrite, why was it not caught by your team's process and addressed in its proper context (i.e. technically rather than socially)? Bad or questionable code ought to be exposed quickly on a healthy team via tests, code reviews, design reviews, etc. Do you have these?
Is the manager/tech lead aware of the problem, and if not, why not? No matter what kind of process you have, code quality is ordinarily the development manager's responsibility. He or she is the first person you should be asking - obviously not in the context of "so and so wrote crap code" but simply "I think I see a major problem here, can we talk about a rewrite?" If it doesn't warrant this type of discussion, then maybe it doesn't warrant a rewrite either?
If the size and scope of the offending code is large enough to warrant serious discussion of a rewrite, why is it exclusively owned by one developer? Most if not all of the times when I've seen code and thought "rewrite", it's been trampled on by a dozen different people and the badness is a direct result of the accumulation of style inconsistencies, mistaken or altered assumptions, and good old-fashioned hacks. Code grows thorns over time precisely because of its shared ownership.
My point here is, it's strange that one person would own such a large swath of code in an environment that supposedly practices collective ownership. Are other people afraid to touch this developer's code? Are they afraid to bring up the subject? Maybe there is in an underlying problem with your group dynamic, or with this developer specifically; if there is, don't ignore it. Or, alternatively, perhaps it is your first time working on this project, and if so, why are you thinking about a rewrite so early in the game? Something about the situation doesn't seem to add up for me.
The question states, in the first paragraph, any developer can change any line of code to add functionality, to refactor, fix bugs or improve designs. Will a rewrite not do these things? Or will it do something extra - and if so, what? Fundamentally, what are your reasons for wanting to do a rewrite, and how sure are you that they'll benefit the product or the team? You don't need to answer for me, but you'd better have some objective points to make if and when you choose to discuss it with the team.
I truly feel that this question entails a huge amount of context and should not be answered without a very clear understanding of that context. The "correct" answer completely depends on your team, your product, your organization, your personality, your process, the scope of the original code, the scope of the changes, and so on and so forth. This is, IMO, an issue that you absolutely need to take up with your team, not on an online Q&A site.
Why are you editing the code?
Are there bugs or is it a more fundamental design flaw?
In the former case, I'd worry about a rewrite to fix what could just be minor bugs in the code.
In the latter case, while I'd expect a heads-up that "my" code was potentially being rewritten, I'd be perfectly happy for it to happen.
That's the point of collective code ownership - the code you write is there to be modified or even replaced if something better comes along.
Generally, if you all agree that you are all responsible for the code, then it's ok to rewrite any code if it's an improvement. Discuss it with the other developer first as a courtesy. There may be valid reasons it's written in a certain way. On a personal level many developers get emotionally invested in what they produce and, as others have said, you want no ill will.
Specifically it depends somewhat on team dynamics. A senior developer about to rewrite a junior developer's code should probably perform a code review (my site) with them before simply rewriting it. That way the junior developer can learn from the situation.
It doesn't really matter if the original developer is on the team or not, or even if you are the original developer. Complete rewriting of any shared code should be run by your team, for the following reasons:
As Matthew Flynn said, it is necessary to talk to the developer first. The initial developer might tell important things about the functionality and explain why this or that solution was given.
But before refactoring or re-writing the code, either make a backup or branch containing that code. If the old code works fine, there will remain a chance that it might be recovered.
If you have a source-control system (which I assume you do have), then, if possible, refactor the whole code and only afterwards, when you're sure it works fine, check that in.
Do not delete the old code, at least before you're sure the new code works fine. But leaving the old code there forever isn't a good thing either. I suggest you delete the code some time later, like in a month after it passed the testing.
What's the scope of it?
If you're reworking a few lines to be more efficient, just do it. Maybe ask if there's some danger of you erasing subtle behavior. Report the change during scrum or via email once it's done, unless it's really trivial (fixing a typo).
If you're reworking a decently sized class, you should probably ask to make sure you understand the design/behavior. Let people know ahead of time so that anyone that might be working in the same area can be aware and coordinate with you.
If you're reworking a larger system, you should definitely work with your team to make them aware of the changes and plan the design.
If your original writers are there, communicate with them. Not JUST for ettiquette, but for additional information, in case there are cases or circumstances that you aren't aware of. Sometimes, they'll tell you something valuable.
We have abysmal code control. I currently have a ton of stuff that needs review and maintenance, and don't even have anyone to do code review with. The previous writers (aside from moi) did not bother to comment any code. And, they're gone from the company. There's no one to ask about the code.
When I re-write, I comment out, with comments that I commented it out, as well as date and name/initials, and why it was being commented out. I comment new code, with name or initials, date, reason is was added, etc.
Additional comments are made at the package head (er, usually), indicating what functions/procs/SQL/etc. were changed, with date. It makes it easy to look up sections that have been changed, and those sections have full documentation.
Comments are cheap. Dates will be an indicator of what changed, and after x-number of (days/revisions/new hires/retires) then the code can be cleaned of old comments and the remaining is the new baseline.
From my observation the general practice is: Never delete the code. Just comment it out and keep it.
My practice is to comment it out while replacing it. (Mainly for reference.) Then delete it on the the final commit of the working change. (This requires version control and an understanding of how to revert changes.)
When I find old commented code, I try to delete it in a separate commit with appropriate comments. This makes it easier to revert or inspect the change should it be required.
For all changes you should be able to use the revision history to determine the developer(s) who created the code. (Annotated revision history helps here.) Contact them if possible to see why the code is as it is. On many projects, cleanup time just doesn't occur and things get left behind. Check the bug tracker to see if there is a record of the required cleanup. Sometime changes need to be cleaned up a release or two later.
If you are changing code, there should be a reason you are working with that code. Check with the original developer to find out why the code is like it is. If there is a legitimate reason not to fix it, add a comment explaining why it wasn't fixed, or why is shouldn't be changed. This will save the next developer some time.
Tags like FixMe, ToDo, Bug, and Hack can be used to indicate code which could be changed later. (It is acceptable to tag limitations as Bug and not fix them if they won't be triggered under the required conditions.) It might be bug if a home accounting program overflows at 20 million dollars, but I wouldn't waste much time fixing it.
Code review changes should be done by the original developer if it is appropriate to modify the code.
It probably depends on why the rewrite is necessary. If it's because there are problems with what's written, and there have always been problems with it, then talking about it is called for, if only to minimise how often code needs to be fixed in the future.
My suggestion in this case is to pair when fixing the code. This way, it provides a good example of the intermediate states, and (hopefully), some context on why the rewritten code is better. Also (if only as a personal exercise), try refactoring the code to be better without a total rewrite. It will be harder, but it's good for you.
On the other hand, there are some other times when a rewrite is called for:
The team has learned a lot since the code was written, and the dev(s) who wrote it would write it differently now, even without your input.
A new feature requirement changes the situation such that a targeted mini-rewrite is appropriate.
In these cases, I probably wouldn't bother with a conversation.
Thinking about it, it seems to me that the longer the code has been there, the less likely a conversation is necessary.
I think @aaronaught made some good points, which really leads to the answer I wanted to give, which is that it really depends on who's making the change (and why) and who wrote the code.
In my personal experience code is normally changed because either it doesn't work as intended, or you simply need to extend what it actually does.
In a Team dev environment, you shouldn't have to (and may not be able to) talk to the original coder, everything should be clear from the code.
That then leads to the question which consumes most of my time, which is what did the original programmer intend, and it is that question which most often leads to code being deleted, and is why we should comment everything, and where inexperienced junior programmers most often fall foul.
Any programmer that is changing someone else's code (refactoring) really should as a matter of courtesy and practice copy the same coding style of the code already in place, and first take steps to figure out how the original code worked, and what it was trying to, and actually going to, achieve. Often this in itself identifys bugs, but certainly it forces people to endure the pain that the next person will have looking at your code.
In my team anyone can delete, refactor or rewrite anything, and I view 'ownership' as a practice which breeds lazyness, as if one person is sure to be notified of any changes, why would they need to make the code readable.
So in short, no, you shouldn't have to ask the original author of the code, and if on looking at the code you do, then it is a sign that either his code isn't readable enough, or you need to improve your skills. However, I find it good form to leave the original code in place, commented out, until you're absolutely sure that in rewriting, you haven't accidentally removed required functionality. Nobody is perfect.
Often code is written a certain way for a reason. Communicating the change to the author always works out for the best.
