20

What should you do, if a co-worker is editing your code?

Without the purpose of adding functionality or fixing bugs, just to change how it looks...

gnat
  • 21,442
  • 29
  • 112
  • 288
Tamara Wijsman
  • 8,259
  • 14
  • 58
  • 94
  • 9
    I assume you have a problem with this. If so, why? Does it make the code *worse*? – Zaz Sep 15 '10 at 18:19
  • No, just brainstorming useful questions so when someone asks them in the future there will already be nice answers for them. These two were based on someone who asked me why I changed something in his code, and I thought they would fit nice here... – Tamara Wijsman Sep 15 '10 at 18:45
  • 4
    @Josh: Yes in fact it does make code worse, because it's harder to maintain by some other programmer than the guy who wrote it. – Robert Koritnik Sep 15 '10 at 19:22
  • 4
    give him more work to do – Oscar Cabrero Sep 15 '10 at 21:14
  • 4
    @Robert - I think you miss @Josh's point. Changing the appearance of code *may* make it objectively *easier* to maintain ... especially if it was poorly formatted to start with. – Stephen C Sep 16 '10 at 06:32
  • 3
    you might be doing something clutter or ugly with your code, that's why he's fixing it – Junior Mayhé Oct 24 '10 at 23:53
  • 4
    Is it really *your* code, or does it belong to the team? – Eric King Oct 25 '10 at 03:48
  • @Eric King: I would think that any code belongs to the team, it's just that this can lead to frustration for some. He should be doing something else more useful instead.... – Tamara Wijsman Oct 27 '10 at 22:32
  • 1
    @TomWijsman: Ask them why. You might learn something (or your collegue might). – JacquesB Jan 19 '16 at 09:44
  • Possible duplicate of [Dealing with co-workers who do not have a consistent coding style?](http://programmers.stackexchange.com/questions/23472/dealing-with-co-workers-who-do-not-have-a-consistent-coding-style) –  Apr 22 '16 at 01:43

13 Answers13

29

Talk to them about it. Go into the conversation with the attitude of "They're not doing this to annoy me or because they have some form of obsessive-compulsive disorder; they're trying to make my code better."

Because you could be wrong. That could be a subtle bug fix and you just didn't spot it.

Or, it could be that there's a coding standard you don't know about that you're violating, and they're just correcting it.

Or, it could be that they're trying to annoy you, or they have some form of obsessive-compulsive disorder. If that's the case, ask them nicely to stop, and if that doesn't work, take it up with your boss.

But you'll never know unless you ask.

BlairHippo
  • 8,663
  • 5
  • 41
  • 46
  • 18
    It's worth noting that a lot of IDEs have auto formatting features. I use them all the time without thinking about it. Some will even format all the files in your project or all the files you have open, depending on how it's configured. It can be easy to accidentally auto format. – Matt Olenik Sep 15 '10 at 16:28
  • @Matt: Excellent point. – BlairHippo Sep 15 '10 at 17:23
  • 2
    "They're not doing this ... because they have some form of obsessive-compulsive disorder;" Speaking primarily of myself, that isn't always the case! When it comes to code I'm two things: a perfectionist and a neat freak. Despite that, I generally endeavor to avoid applying this mentality on my colleagues' work. – Nathan Taylor Sep 15 '10 at 17:28
  • 6
    Oh, I'm not saying Tom's colleague ISN'T an OCD neat-freak with a poor sense of boundaries. I'm just saying that going into a conversation with a mindset of "What the hell is WRONG with you?!" isn't a good way to have a productive conversation. :-) – BlairHippo Sep 15 '10 at 17:33
  • @BlairHippo Since when? =D – Nathan Taylor Sep 15 '10 at 17:41
  • @Nathan your definition of neat perfect code might be far from the same as your teammate. So next time I see your touching my code you better be ready to justify! LOL jk. – Chris Sep 15 '10 at 17:53
  • 1
    @Chris no need to worry about justification, I always Ctrl+K+D! – Nathan Taylor Sep 15 '10 at 18:48
  • @Nathan Taylor, use Ctrl+E+D, easier on your hands – Matt Olenik Sep 15 '10 at 20:56
16

I'm not so married to how my code looks for it to bother me. :) I try to learn from the changes. Did my coworker adjust variable names? Write a more efficient loop? Make the code more readable?

If I can't see how the changes improved what was already there, I usually ask the coworker who made the changes what the motivation behind them was. It's possible that the advantage is just not obvious to me. And if I'm right and they're wrong, then perhaps I can explain why I wrote it the way I did.

If all else fails, revert the check-in. ;)

Edit: All bets are off if the desire to make cosmetic changes introduced a bug, though.

Adam Lear
  • 31,939
  • 8
  • 101
  • 125
9

IMO you and your team should be using a coding standard anyway. If this is the case, then the questions becomes 'did your original code conform to the standard?' If 'yes' then your colleague should not be touching your code unless to change it functionally. If 'no' then I'm afraid your colleague has every right to tidy up your code. As a project lead I find myself doing it all the time.

If you're not using a coding standard then the whole argument of what constitutes 'good code' becomes way too subjective. Hence why you should be using a coding standard :)

8

As one of those people (the people who occasionally reformat other people's code), the main reason I do it is readability. Some people are just extremely sloppy with their indentation or with mixing tabs and spaces.

The main thing I have a habit of changing is reducing long lines so that I can read the whole thing without horizontal scrolling. I'll break complex statements up into separate statements or reformat method calls/declarations to list one parameter per line if it doesn't all fit comfortably on a single line. I'll also edit comments, either to fix English errors or just to make things clearer.

Yes, I could leave it alone, but I'd rather reduce the mental effort required to read the code.

What should you do about it? Firstly, consider that maybe this person is making your code better. Also, you should ensure that you have some consensus in your team about how code should be formatted. If each person has different habits it will slow everybody down. If they are not making your code better and they are going against the grain, then you need to confront them about it. If that doesn't work then you might need to get others involved.

Dan Dyer
  • 3,546
  • 2
  • 23
  • 20
  • 1
    THat's your readability, I find unindented SQL code to be much easier to read because I read fast and indented code slows me down and makes it harder for me to focus. – HLGEM Sep 15 '10 at 18:51
6

Ask them why they are doing it; a valid explanation may diminish your frustration, but you should let them know how much it bothers you. Who knows, maybe they thought they were doing you a favor and will stop when they learn it offends you. Or you may be dealing with someone who is truely suffering from a medical condition.

JeffO
  • 36,816
  • 2
  • 57
  • 124
5

Is s/he allowed to? Do the changes improve the code? If so, swallow your pride. If you feel the code quality is worsened, take it up with the co-worker and ask them why they felt the need to change your code with no obvious benefit. If it's being done out of spite or because the person mistakenly feels they're better than you, and you can't work it out with them, take it up with your boss.

Chinmay Kanchi
  • 6,173
  • 2
  • 39
  • 51
5

IDE's like Visual Studio have an option called Format Document that will format code according to the rules the user has set in the IDE. It could be your co-worker is using this (either automatically without knowing, or by deliberate application). Perhaps their IDE uses spaces instead of tabs, or vice-versa, and these are being applied automatically without even knowing? But you need to talk with them to find out.

Incidentally, I will often re-format code of co-workers if it is obviously not following some kind of formatting scheme (i.e. it is all over the place). It's a hopefully subtle way of making them notice. (However, I wouldn't reformat it if it was neat, but not to my liking).

Dan Diplo
  • 3,900
  • 1
  • 27
  • 30
  • 1
    *"(However, I wouldn't reformat it if it was neat, but not to my liking)"* - very important rule to follow, +1 – Zaz Sep 15 '10 at 18:22
  • Our development guidelines tend to put code style more in the 'recommended' corner. I auto-format code to the recommendations at the file level if I find it hard to read. – Joeri Sebrechts Sep 15 '10 at 20:42
3

If he's changing it so that it meets your team's coding standards, you should follow the standards next time.

If he changes it such that it no longer follows your team's coding standards, inform him what he's doing wrong and have him change it back.

...Your team does have a set of code formatting standards that are used by everyone, right?

Daenyth
  • 8,077
  • 3
  • 31
  • 46
2

Code conventions is the answer. You should have one at work. If you don't, start right now (a good starting point is google style guide) . When there are written (or at least commonly known) rules the answer to your question is trivial.

Ilia K.
  • 394
  • 1
  • 4
2

I occasionally reorder code written by messy coworkers (or fix typos in comments). They know that I'm obsessive in code formatting and order and therefore they let me do that without complaining too much. Sometimes they also give me a free soda or cookie.

Of course this is occasional work, as it broke the "blame" functionality in SVN.

This is also a very basic way to do some kind of code review (I usually read most of the code committed by my coworkers in the modules I'm working on).

Wizard79
  • 7,327
  • 2
  • 41
  • 76
1

I feel you are thinking it is offensive to do so... ? For example, I myself would immediately fix this code

int myFunction( ) {

    int i ;
  return  0;

}

to become

int myFunction() {
    int i;
    return 0;
}

so... should I be punished because of my action? In real life, I actually have tons of SVN logs read 'Formatting'. ;-)

tia
  • 965
  • 5
  • 9
0

this is a thought I saw on internet talking about refactoring and maybe explain why would someone touch your code to make it better:

Why?

There are two main reasons to refactor:

  1. To improve the code/design before building on it’s top: It’s really hard to come up with good code on the first attempt. The first try to implement any initial design will show us that we misinterpreted or forgot some logic.

  2. To adapt to changes in the requirements. Change happens in the software development; to be responsive to change is better to have a good code base. We have two options for both scenarios, path the code or refactor it. Patching the code will lead us to unmaintainable code, and will increase our technical debt, it’s always better to refactor.

When?

  1. The sooner the better as is easier.

  2. faster and less risky to refactor over a recently refactored code rather than waiting to refactor for the code to be almost completed.

What?

  1. All the code and all the design are candidates for refactoring.

  2. An exception for not refactoring something could be a working piece of code which is quality is low, but due to be close to a deadline, we prefer to keep our technical debt rather than risking the planification.

You just have to let him do his best, if it would be great for both and save your time in future!

cheers

Junior Mayhé
  • 1,842
  • 2
  • 17
  • 35
0

Use a style checking tool

Start using StyleCop or similar and enforce code style rules and also make it an obligation for all developers to use it. All code will look the same without exception. And get together with wiseheads to discuss most appropriate rules for your organisation. Even though default rules are very similar to .net framework code itself already.

It's the easiest way of doing it. I found myself correcting someone else's code at one of my previous employers because this other guy was writing code with excessive amounts of empty lines and no indentation rules whatsoever. Code was actually unreadable by an average developer. If StyleCop would exist back then it would make many of us really happy.

Robert Koritnik
  • 3,511
  • 1
  • 16
  • 26
  • I have to down vote this. StyleCop is a terrible implementation of a decent idea. 1) It runs after build which makes it a major time killer on large projects 2) It's default rules actually contradict VS's defaults in some cases 3) Some of the rules are purely inane. It complains about "//" without a trailing space and then tells you to use "////" Again, after a build. That's the worst part. It wouldn't be bad, but the after build part really kills you on a large project with a long build time. – MIA Oct 01 '10 at 01:41
  • I would disagree with you on many aspects you've pointed out. You can configure the way style checking works. I've even implemented a couple of my own rules that provide formatting that I want. Regarding speed I can't say it's great. but on a decent machine it should work just fine. Just think of C++ copiler speed in the beginning of the 90's where you were actually able to go and prepare a cup of tea in the meantime. While your build failed!!!!!! ;) – Robert Koritnik Oct 01 '10 at 03:31
  • I've just started using StyleCop seeing how it goes and while it is a bit annoying at times it also helps find a lot of errors that otherwise would go unnoticed. You don't have to run it as part of the build and can just run it on your local machine, also for single files only. So you could use it without annoying your fellow developers. Plus, if you don't like the rules, you can just disable them, so no real harm done. – Anne Schuessler Apr 13 '11 at 15:51
  • +1 This isn't explicitly about StyleCop.. (StyleCop _or similar_). And that's a very good idea. Define a set of rules, configure your tool of choice and be done with it forever. – Bruno Schäpper Apr 21 '16 at 19:47
  • These days we have Grunt, Gulp etc that can do this step just like StyleCop did it in the past. – Robert Koritnik Apr 21 '16 at 21:06