Once in a while, a co-worker will check-in bad code which blocks my application from initializing properly.

To get around this problem quickly, I comment out the offending code. I then forget to uncomment the offending code at the time of my check-in, so I want to prevent this from happening again.

Do you have any suggestions on how to:

  1. Disable bad code that stop you from working
  2. Prevent yourself from checking in unwanted changes?

I'm currently using Visual Studio as my IDE and TFS as my source code repo.

  • 8
    Can't you talk to the guy and work with him so that he doesn't commit 'bad code'? Don't you use Continuous Integration with a build server? I'm confused how 'bad code' gets into the source. – ChrisAnnODell Nov 8 '12 at 15:52

This sounds like a internal process issue more than a check-in issue. You should work with the person to ensure that they are doing proper testing and debugging to ensure that the code works. If you have a group of testers, make sure he is sending them his changes to test, they are usually good at finding problems.

You could also be bringing these discussions up in meetings - "Hey, will your code changes affect our changes in any way?", "How will we integrate our changes together without issue once we both are done?".

These are core to working as a team.

| improve this answer | |
  • 3
    If commits of "bad code" (as in not-working code) are allowed, that's a fundamental failure in your checkin process. Prior to checkin, your co-worker should be running all unit tests (you DO write unit tests, right?), and then the build-bot (you DO have a build-bot, like TeamCity or CruiseControl, right?) should run those same tests, and also calculate coverage metrics etc. to make sure the code is properly exercised. – KeithS Nov 8 '12 at 16:03
  • @KeithS I agree, this is a process issue. I've noticed too that some people seem to assume that the errors reported by those systems could not possibly be their fault, so they don't even read them and discover that it is in fact their recent check-in that broke it. – GalacticCowboy Nov 8 '12 at 21:17

You should consider using feature branches in your source repository.

Also consider a continuous integration server. It will help ensure that only compilable code is checked in to the source tree. Even better if it passes automatic unit tests.

| improve this answer | |

Do you have a trick to prevent yourself from checking in unwanted changes?

Before committing a file, get a diff of your current version to the parent and double-check each change to make sure they're what you intend to commit. This is also a good time to analyze your changes as to whether it is the best solution, whether it will be hard for future developers to maintain, etc.

When I'm at a point where I want to commit, I typically take a look at each modified file to verify changes as described above, and as I go along, decide which files to group together in a single commit. After I've gone over them all, I commit files as planned.

This process requires that you commit more often, but that's often a good thing.

| improve this answer | |

Bad code shouldn't be committed in the first place. Breaking the build isn't a nice thing to do when one works in a team, and different teams have different approaches to encourage the developers to check twice before committing the code.

If the code breaks the build, you shouldn't comment it. Either you see where the problem is, so you can easily solve it yourself, or you should discuss the issue directly with the person who committed bad code. Just committing the code will not make the problem disappear. It's like turning your back to a monster: you wouldn't see the monster any longer, but the beast is still here.

Also note that:

  • The bad code is already committed. If you don't want this code to be in the build, instead of commenting it, delete it. You can always recover the code from the version control; that's the point of having one.

  • The issue may come from a developer who usually use version control systems like Git. In those systems, one can commit as many times as he want, without affecting the work of other people. Once the code is checked for correctness, it is committed to the wild, affecting the whole team.

    Developers who work this way are usually lost when it comes to older version control systems, like SVN. If they are committing their changes too often, they cannot check the code for correctness every time; at the same time, it's not very correct to force them to commit less frequently.

| improve this answer | |

To answer (1) how about:

// the dodgy code

This, obviously, is C syntax, but similar is no doubt available for whatever language you are using.

Never (and I mean NEVER) "comment out" code to disable it, as (a) this will get lost/forgotten about, and gets confused for genuine comments, and (b) comments within it might accidentally end the comment.

| improve this answer | |

Taking all of the other advice about process and code quality into account, if you're going to comment out broken code, put a big, blinking sign on it.

// FIXME: Commented out due to broken build

Then, before checkin, search for FIXME and undo those changes.

| improve this answer | |

Not the answer you're looking for? Browse other questions tagged or ask your own question.