Question

One of our developers is continually writing code and putting it into version control without testing it. The quality of our code is suffering as a result.

Besides getting rid of the developer, how can I solve this problem?

EDIT

I have talked to him about it number of times and even given him written warning

Was it helpful?

Solution

If you systematically perform code reviews before allowing a developer to commit the code, well, your problem is mostly solved. But this doesn't seem to be your case, so this is what I recommend:

  • Talk to the developer. Discuss the consequences for others in the team. Most developers want to be recognized by their peer, so this might be enough. Also point out it is much easier to fix bugs in the code that's fresh in your mind than weeks-old code. This part makes sense if you have some form of code owneship in place.
  • If this doesn't work after some time, try to put in place a policy that will make commiting buggy code unpleasant for the author. One popular way is to make the person who broke the build responsible for the chores of creating the next one. If your build process is fully automated, look for another menial task to take care of instead. This approach has the added benefit of not pinpointing anyone in particular, making it more acceptable for everybody.
  • Use disciplinary measures. Depending on the size of your team and of your company, those can take many forms.
  • Fire the developer. There is a cost associated with keeping bad apples. When you get this far, the developer doesn't care about his fellow developers, and you've got a people problem on your hands already. If the work environment becomes poisoned, you might lose far more - productivity-wise and people-wise - than this single bad developer.

OTHER TIPS

If you can do code reviews -- that's a perfect place to catch it.

We require reviews prior to merging to iteration trunk, so typically everything is caught then.

Ritual beatings! For each bug, one lash of the whip!

(A joke for anyone who doesn't get it)

As a developer who rarely tests his own code, I can tell you the one thing that's made me slowly shift my behavior...

Visibility

If the environment allows pushing code out, waiting for users to find problems, and then essentially asking "How about now?" after making a change to the code, there's no real incentive to test your own stuff.

Code reviews and collaboration encourage you to work towards making a quality product much more than if you were just delivering 'Widget X' while your coworkers work on 'Widget Y' and 'Widget Z'

The more visible your work is, the more likely you are to care about how well it works.

Code review. Stick all of your dev's in a room every Monday morning and ask them to bring their most proud code-based accomplishment from the previous week along with them to the meeting.

Let them take the spotlight and get excited about explaining what they did. Have them bring copies of the code so other dev's can see what they're talking about.

We started this process a few months ago, and it's astonishing to see the amount of sub-conscious quality checks that take place. After all, if the dev's are simply asked to talk about what they're most excited about, they'll be totally stoked to show people their code. Then, other dev's will see the quality errors and publicly discuss why they're wrong and how the code should really be written instead.

If this doesn't get your dev to write quality code, he's probably not a good fit for your team.

Make it part of his Annual Review objectives. If he doesn't achieve it, no pay rise.

Sometimes though you do just have to accept that someone is just not right for your team/environment, it should be a last resort and can be tough to handle but if you have exhausted all other options it may be the best thing in the long run.

Tell the developer you would like to see a change in their practices within 2 weeks or you will begin your company's disciplinary procedure. Offer as much help and assistance as you can, but if you can't change this person, he's not right for your company.

Using Cruise Control or a similar tool, you can make checkins automatically trigger a build and unit tests. You would still need to ensure that there are unit tests for any new functionality he adds, which you can do by looking at his checkins. However, this is a human problem, so a technical solution can only go so far.

Why not just talk to him? He probably won't actually bite you.

  • Make him "babysit" the build, and become the build manager. This will give him less time to develop code (thus increasing everyone's performance) and teach him why a good build is so necessary.

  • Enforce test cases - code cannot be submitted without unit test cases. Modify the build system so that if the test cases don't compile and run correctly, or don't exist, then the entire task checkin is denied.

-Adam

Publish stats on test code coverage per developer, this would be after talking to him.

Here are some ideas from a sea shanty.

Intro
   What shall we do with a drunken sailor, (3×)
   Early in the morning?
Chorus
   Wey–hey and up she rises, (3×)
   Early in the morning!
Verses
   Stick him in a bag and beat him senseless, (3×)
   Early in the morning!
   Put him in the longboat till he’s sober, (3×)
   Early in the morning!

etc. Replace "drunken sailor" with a "sloppy developer".

Depending on the type of version control system you are using you could set up check-in policies that force the code to pass certain requirements before being allowed to check-in. If you are using a sytem like Team Foundation Server it gives you the ability to specify code-coverage and unit testing requirements for check-ins.

You know, this is a perfect opportunity to avoid singling him out (though I agree you need to talk with him) and implement a Test-first process in-house. If the rules aren't clear and the expectations are known to all, I've found that what you describe isn't all that uncommon. I find that doing the test-first development scheme works well for me and improves the code quality.

They may be overly focused on speed rather than quality.

This can tempt some people into rushing through issues to clear their list and see what comes back in bug reports later.

To rectify this balance:

  1. assign only a couple of items at a time in your issue tracking system,
  2. code review and test anything they have "completed" as soon as possible so it will be back with them immediately if there are any problems
  3. talk to them about your expectations about how long an item will take to do properly

Peer programming is another possibility. If he is with another skilled developer on the team who dies meet quality standards and knows procedure then this has a few benifits:

  1. With an experienced developer over his shoulder he will learn what is expected of him and see the difference between his code and code that meets expectations
  2. The other developer can enforce a test first policy: not allowing code to be written until tests have been written for it
  3. Similarly, the other developer can verify that the code is up to standard before it is checked-in reduicing the nmber of bad check-ins

All of this of course requires the company and developers to be receptive to this process which they may not be.

It seems that people have come up with a lot of imaginative and devious answers to this problem. But the fact is that this isn't a game. Devising elaborate peer pressure systems to "name and shame" him is not going to get to the root of the problem, ie. why is he not writing tests?

I think you should be direct. I know you say that you've talked to him, but have you tried to find out why he isn't writing tests? Clearly at this point he knows that he should be, so surely there must be some reason why he isn't doing what he's been told to do. Is it laziness? Procrastination? Programmers are famous for their egos and strong opinions - perhaps he's convinced for some reason that testing is a waste of time, or that his code is always perfect and doesn't need testing. If he's an immature programmer, he might not fully understand the implications of his actions. If he's "too mature" he might be too set in his ways. Whatever the reason, address it.

If it does come down to a matter of opinion, you need to make him understand that he needs to set his own personal opinion aside and just follow the rules. Make it clear that if he can't be trusted to follow the rules then he will be replaced. If he still doesn't, do just that.

One last thing - document all of your discussions along with any problems that occur as a result of his changes. If it comes to the worst you may be forced to justify your decisions, in which case, having documentary evidence will surely be invaluable.

Stick him on his own development branch, and only bring his stuff into the trunk when you know it's thoroughly tested. This might be a place where a distributed source control management tool like GIT or Mercurial would excel. Although with the increased branching/merging support in SVN, you might not have too much trouble managing it.

EDIT

This is only if you can't get rid of him or get him to change his ways. If you simply can't get this behaviour to stop (by changing or firing), then the best you can do is buffer the rest of the team from the bad effects of his coding.

If you are at a place where you can affect the policies, make some changes. Do code reviews before check ins and make testing part of the development cycle.

It seems pretty simple. Make it a requirement and if he can't do it, replace him. Why would you keep him?

I usually don't advocate this unless all else fails...

Sometimes, a publicly-displayed chart of bug-count-by-developer can apply enough peer pressure to get favorable results.

Try the Carrot, make it a fun game.
E.g The Continuous Integration Game plugin for Hudson
http://wiki.hudson-ci.org/display/HUDSON/The+Continuous+Integration+Game+plugin

Put your developers on branches of your code, based on some logic like, per feature, per bug fix, per dev team, whatever. Then bad check-ins are isolated to those branches. When it comes time to do a build, merge to a testing branch, find problems, resolve, and then merge your release back to a main branch.

Or remove commit rights for that developer and have them send their code to a younger developer for review and testing before it can be committed. That might motivate a change in procedure.

You could put together a report with errors found in the code with the name of the programmer that was responsible for that piece of software.

If he's a reasonable person, discuss the report with him.

If he cares for his "reputation" publish the report regularly and make it available to all his peers.

If he only listens to the "authority", do the report and escalate the issue to his manager.

Anyway, I've seen often that when people are made aware of how bad they seem from outside, they change their behaviour.

Hey this reminds me of something I read on xkcd :)

Are you referring to writing automated unit test or manually unit testing prior to check-in?

If your shop does not write automated tests then his checking in of code that does not work is reckless. Is it impacting the team? Do you have a formalized QA department?

If you are all creating automated unit tests then I would suggest that part of your code review process include the unit tests as well. It will become obvious that the code is not acceptable per your standards during your review.

Your question is rather broad but I hope I provided some direction.

I would agree with Phil that the first step is to individually talk to him and explain the importance of quality. Poor quality can often be linked to the culture of the team, department and company.

Make executed test cases one of the deliverables before something is considered "done."

If you don't have executed test cases, then the work is not complete, and if the deadline passes before you have the documented test case execution, then he has not delivered on time, and the consequences would be the same as if he had not completed the development.

If your company's culture would not allow for this, and it values speed over accuracy, then that's probably the root of the problem, and the developer is simply responding to the incentives that are in place -- he is being rewarded for doing a lot of things half-assed rather than fewer things correctly.

Make the person clean latrines. Worked in the Army. And if you work in a group with individuals who eat a lot of Indian food, it wont take long for them to fall in line.

But that's just me...

Every time a developer checks something in that does not compile, put some money in a jar. You'll think twice before checking in then.

Unfortunately if you have already spoken to him many times and given him written warnings I would say it is about time to eliminate him from the team.

You might find some helpful answers here: How to make junior programmers write tests?

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top