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
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:
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.
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:
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.
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.
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.
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
Why not just talk to him? He probably won't actually bite you.
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".
Publish stats on test code coverage per developer, this would be after talking to him.
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.
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.
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:
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.
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:
All of this of course requires the company and developers to be receptive to this process which they may not be.
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
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.
It seems pretty simple. Make it a requirement and if he can't do it, replace him. Why would you keep him?
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.
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 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.
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:
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.
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.
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.
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.
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?
You might find some helpful answers here: How to make junior programmers write tests?
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.
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.
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...
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.
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.
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 :)
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 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.
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.
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.
I would suggest (as others):
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.
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.
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.
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.
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.
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
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!!!
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.
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.
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.
NCover + Cruise Control, send out automatic reports and then one can prove that as he checks in code coverage goes down.
Test your spelling! I think you meant "their code".
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.
© 2022 - 2024 — McMap. All rights reserved.