views:

2372

answers:

51

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

+8  A: 

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

Phil Wright
and if you tried already, fire him... he might get it, for his future job!
pmlarocque
I have talk to him about a number of times – and even given him written warring
Charles Faiga
In that case you might need to take things to next level and contact the HR department.
Phil Wright
I agree, make it clear that his job is at stake. If you can convince him to start testing, you're doing him a favor. If nothing but the threat of unemployment will help, then so be it.
Adam Bellaire
A: 

Have you tried talking to them about it? Might not be a bad first step. It might also give you some clues as to what step 2 should be if the problem continues.

Josh Wright
+9  A: 

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.

Paul Dixon
+2  A: 

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.

Kibbee
yes, with DVCS you can refuse to accept code that does not meet the minimum quality.
Casey
+41  A: 

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.

Ian P
After reading the edit to your original question -- code reviews become one of the only options remaining.
Ian P
This has probably been the number one improvement we have made in our shop. Regardless of whether or not it helps with this guy, it will help your shop.
Konrad
I've found code reviews priceless. You can spot a large number of errors just by explaining the code to the other person. You also learn a lot with it.
ya23
Code reviews are not a substitute for testing. You should be doing both.
slim
But that guy will learn some things with CR.
helios
while not a substitute for testing it can be an eye opener to the problem developer to really take a look at what he is doing when 90% of the problems found are on him. That or he learns nothing and you need to can him.
mynameiscoffey
+7  A: 

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

Paul Whelan
+10  A: 

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.

Si Keep
Looks like there is little choice but to include the "number of production/live defects" part of the metrics by which you guage his performance. Ensure sure that he knows that he is being measured and it will affect his performance review. :)
jop
not getting pay rise will not encourage good quality code. It will be insatisfactory but he will not know how to do it better.
helios
+2  A: 

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.

lillq
In other words, remove his commit privileges until he has proven his willingness to check in tests with his code.
Bryan Oakley
A: 

Test your spelling! I think you meant "their code".

  • Talk to him. Let him know it's an issue.
  • Have a group meeting to discuss code quality.
  • If it's still bad, force him to have his code checked before he can check it in.
  • If he doesn't get the hint by then, you'll have to let him go.
Oli
+4  A: 

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.

Micah
http://msdn.microsoft.com/en-us/library/bb668980.aspx
slf
Definately voting for this one... All has to meet the same requirement and it has a good potential for improving overall qualityIf this doesn't do the trick: Give him one last warning (including that next time untested code is detected from him, he will have to leave) and then let him go if needed
noesgard
+5  A: 

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.

itsmatt
+1  A: 

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.

danivovich
+1  A: 

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 :)

Remo.D
+9  A: 

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.

RossFabricant
I know a company where they had a screen with mugshots of those developers who most often broke the build. This might be an incentive.
Torsten Marek
This would work if he actually wrote unit tests in the first place. All the developer has to do is not write quality tests to still get away with this behavior.
Konrad
We use CCMenu, Growl and a MacMini to blurt out a very loud fail noise - then everyone downs tools to try and fix the build. When the noise does go off fingers are usually pointed too.
Paul Shannon
If all the other developers write quality tests with a high degree of coverage he'll find it hard slip features through the net.
Paul Shannon
+1  A: 

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.

Mark Lindell
+8  A: 

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.

Nick Sergeant
Good suggestion!
Marcus Lindblom
+4  A: 

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
WW
+28  A: 

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.
The nice thing about making the bad developer "babysit" the build is that they spend less time developing code than watching the build, which realizes performance gains for everyone.
Adam Davis
Disciplinary measures being temporary no-direct-commit access. This is what the OpenBSD open source project does. Before a release all developers with commit access must spend a while testing everything. If they don't then they usually get rejected commit access and thus everything they want to do must be checked in by someone else. If this doesn't work after a while, then I would say a firing is in order.
Earlz
+7  A: 

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".

Rafał Dowgird
We just don't do enough of this in the dev community.
Chris Ballance
My mother used to sing this song, but only the intro. Thanks for providing me with the rest of the words!
Graeme Perrow
There are many many more verses, the sailors seem a creative bunch.
Rafał Dowgird
Stick 'em in the hold with the Captain Daughter might not be the punishment you're looking for.
Rontologist
Depends on what the captain's daughter looks like :)
Sandman
@Rontologist: Whatever floats your boat (pun not indented). See http://www.urbandictionary.com/define.php?term=captain%27s%20daughter
Piskvor
I love the syntax highlighting on that.
jcdyer
+14  A: 

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.

Kevin Fairchild
A: 

NCover + Cruise Control, send out automatic reports and then one can prove that as he checks in code coverage goes down.

Quibblesome
A: 

Code reviews and unit tests.

Having been (like many people) the guy who checks in a trivial change and breaks things, I can tell you that unit tests remove any excuse for not testing, if they are setup so you can run the whole panoply quickly, and they help identify who broke the code (assuming a decent VCS). Of course, with informal code reviews, I've checked in trivial code that has been reviewed by a senior (and competent) colleague, and still broken the codebase.

Marcin
+19  A: 

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

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

Ross Anderson
+1 for being funny, -1 for explaining, +1 because it really is funny
abyx
From that I assume you are THE Ross Anderson ?
Martin Beckett
Haha no unfortunately not, just the same name. Though I admire his work.
Ross Anderson
+1  A: 

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.

JohnMcG
+2  A: 

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

bruceatk
+7  A: 
  • 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 Davis
I find that people can always subvert the system. If the employee can't check in his work without unit tests, he'll write the equivalent to assert(1==1), or the most basic and probably very rigid tests.
Nathan Koop
That's true, and it will be caught eventually, showing positive proof that he is not developing to company policy/standards.
Adam Davis
A: 

If talking to him didn't work, and you can't fire him, then he's either lazy or unreasonable. If you can't take the high road and reason with the guy, hit him where it hurts and start docking his pay. Or if you really want to punish him, make him maintain the code.

Aston
+1  A: 

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

D.S.
A: 

Introduce code coverage tools and produce an automated report from your build server of all the code not covered by unit tests. His name will be bottom of the board.

The board should be printed and stuck somewhere every week where everyone can see it.

Stop giving him anything new to do until his coverage is at 85%

Give the guy at the top of the board the most interesting jobs.

Tie your next written warning to a certain code coverage requirement - then you have clear dismissal reasons should he fail.

Keith
+1  A: 

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.

Carra
A: 

Tell him he will be reassigned to the quality team where he will be doing only documentation. That has worked for me more than once for the teams that I was leading... and if that doesn't work, find somebody else to test his code! ..wait, thats lame...o yea.. Fire him!!!

+1  A: 

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.

Tons0fun
A: 

It dependes.

Does his code work? Is he the most productive or least productive member of your team? Is the code buggier than others? How valuable are his/her contributions?

If he is a stellar performer who produces high qualtiy code then who cares. If on the other hand he/she is producing bug ridden code then sit that person down, speak with them, lay out the consequences and they either get on board or they don't

A: 

You mention talking to the developer, but I'm curious to see if you asked them about their testing procedure. If they come from another company then they might be used to writing all of their code, checking it in, then testing, and then checking in the final version of the code. If they are viewing the check-ins as just another way to save their work.

However, if they have been with your company for a significant period of time (say at least six months) then they should be used to how you do things and this wouldn't be a very valid excuse for much longer.

Rob
+2  A: 

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.

Joe Strazzere
Or combine this with an incentive program could get quality up without the peer pressure.
Tom
A: 

Simple. Make the devs responsibility verify bugs that are reported and fix the ones that reproduce. Don't let the person work on new features.

If the person has half a brain, they will quickly develop a certain level of fustration by bone-headed bugs caused by not unit testing code. Additionally, the overall skill of the person will likely grow significantly.

I see where you're coming from but I suspect what happens is that he just ends up locked in his own little bubble of incompetence shipping and fixing crappy code. That's no good for the users (who have to put up with shitty code), it's no good for the testers (who have to put up with shitty code) and it's no good for the team (who get the reputation of shipping shitty code - the outside world will not differentiate between team members).
Jon Hopkins
A: 

If even the code reviews of his code doesn't work, maybe give him a task to review some other "wonderful" ( ;) ) code from which he might relate to his own problems and ask him to compare his code with that awful piece of code.

Usually, the problems with such kind of people is the self-realization, so no matter how you try to make him understand the problems with his own code; until and unless he himself doesn't realize, it's not gonna work. This is of course, if you don't have an option to fire him, infact want to groom him.

Salman Kasbati
A: 

It sounds like you've made it clear to him that this is important to you, the company and the team.

I think you need to find out what is behind his behaviour - is he just not hearing what you're saying? Maybe you need to find another way to say it.

Maybe he's not convinced - there could be any number of reasons for that - find the fundamental reason and deal with that.

Johan
+2  A: 

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.

Crippledsmurf
+3  A: 

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.

Simon Howard
This is a great perspective. Part of the issue may simply be that he doesn't know how to test, or needs help getting started. Pair programming with a veteran tester is a great way to get someone over this hurdle.
Scotty Allen
A: 

If talking doesn't work, put a policy in place where code checked in without accompanying tests are simply backed out of the repository. After they have to rewrite their code a couple of times, they may get the message.

Terry Lacy
You can do this with good-old scmbug, so the commit goes into a per-developer branch and get's 'trunked' if it passes. This policy "change" should be a mega-public thing. Then after a month, he's done no work that got committed and take that to your manager as grounds for termination. (He's done no work for a month.)
Tim Williscroft
A: 

I would suggest (as others):

  • code review,
  • pair programming,
  • SCM commit policy.
mouviciel
A: 

You can tell your version control system that this user has no permission to upload anything, so he must ask someone to do it for him. That should teach him.

Marc
A: 

Establish an agreement within the team about what will be tested and how it should be tested, and when it should be tested (before check-in, before it pushes, before it gets merged to trunk).

Then, when a check-in doesn't meet the set of standards the team agreed code should meet, simply roll it back, and ask the developer to fix it. Rolling check-ins back are an incredibly effective way to both preserve the quality of the codebase in the face of poor quality check-ins, and as a lightweight way to signal to people that their code doesn't meet the standards set by the team.

The nice part about rollbacks is that it's really easy to check the code back in - just rollback the rollback, fix whatever the issue is, and then check the change again.

I would be careful to do it in a very objective way that doesn't signal anyone out. This means applying it to the whole team, not just your problem member, and focusing on making it more about the quality of the code and having the code that gets checked in meet the standards the team set with each other, rather than as a punishment.

Scotty Allen
A: 

If nothing works out , you FIRE HIM!

Ayreej
+1  A: 

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

Jonas Söderström
A: 

You said he doesn't test his code. Does this mean he doesn't create unit tests? Or he doesn't test his code AT ALL?

If he doesn't test his code at all, then this a is fundamental problem with his development. Testing the code you write is part of the job. A developer who does not test his code is not acceptable and is slowing down your project. Testing is part of the job description of a developer (even the so-called stars).

If, however, he is testing his code but is not creating the 'correct' number of automated unit tests, then this is a different problem, which needs a different solution. As others have said, you need to find out why, and fix it. Code reviews are a good way to find these problems. But it sounds like you already know the problems.

MatthieuF
+1  A: 

You might find some helpful answers here: http://stackoverflow.com/questions/7252/how-to-make-junior-programmers-write-tests

abyx
+1  A: 

If you've genuinely talked to him and you've given him all the support and training he needs to understand why this is a big deal then I'd look at getting rid of him (how that works depends on where in the world you are). I know you said you wanted something aside from firing him but sometimes there aren't "nice" solutions to a problem.

Without being harsh programmers always talk about leaving companies who don't take software development seriously.

If this is reasonable then why should a company put up with a developer who has been given every reasonable chance but still clearly isn't taking software development seriously?

Jon Hopkins
+1  A: 

I'd be tempted to suggest elaborating a bit on what you've tried and what results you got as this may have changed a bit but here are my initial suggestions:

  1. Is it any tests or comprehensive tests? Some may code blindly and do zero tests, but this is rather rare, IME. Usually there are some tests done but not enough to cover most of the cases that would be comprehensive testing.

  2. Group dynamics may help. I'd assume he is part of a team and that the team's view may be of some help here. In a way this is trying to get peer pressure which is usually a bad thing but sometimes it can be used in good ways.

  3. How well spelled out were the warnings? In a way this can seem childish but there is a chance that what you think of as testing may not be the same as his. Do you want nUnit tests, an excel spreadsheet, logs from his computer, or something else as proof of the existence and use of tests? From what you've described there isn't anything to confirm that he did understand what you meant, was going to use tests and provide evidence of doing so.

  4. Check-in policy question. Some places, such as my current workplace, encourage committing often which can mean that one does commit code without tests. Is there a known, accepted and well-followed policy where you are? That's another aspect here.

JB King
+1  A: 

If you have automated builds set up, then make sure that the failure notifications are both as obvious and annoying as possible. Wallboards or audio notifications in the development common areas are a good start. Then, make sure that one a build is broken, make sure that no one checks in code until the offender has fixed the problem.

Granted, this will only catch it when his code breaks the build, but the peer pressure for him to continually be spotlighted for this will be an incentive for most. In the event that this does not help, take the next discinplinary actions available through your human resources department. You have already talked to him, you already have given a written notice - find out what the next steps are. A developer who goes his own way is either a visionary or not a team player - and I have never personally had the pleasure of working with a visionary in that regard.

joseph.ferris