views:

204

answers:

7

You are working with someone in one project and this someone writes bad code. Not that bad to fire him ar you can`t work with it, but

  • person do not follow coding conventions
  • code is not always covered with unit tests (and it should be)
  • code is sloppy and do not have comments where it should have them
  • person do not know design patterns well/do not know at all
  • put his clothes on your table just as he needs some space.

So how to tell one he need to upgrade his skills, without hurting him? I described something that is a problem. But imagine everything is just fine, except some things that really hurt the project. What to do so that you are still friends, but guy really understood he need to change something?

+6  A: 

This is a good candidate for having the entire company do code reviews for a month. You won't single him out (which is good, unless all else fails), and you're better engineers can help him along.

Eric Anderson
To add to this, it sounds like you are his office-mate, not his manager. You should speak to your manager about your concerns.
Eric Anderson
+4  A: 

Just stay professional, and point out the problems in your bug tracking, code reviews, or whatever means you have for tracking work items and bugs.

It's the responsibility of the person's manager and the project's PM to determine who is responsible, and to inform the person politely. That is their job, and if they are unable to do this, your project (and probably company) is likely not a very good one to work for.

Reed Copsey
A: 
  • Anonymously leave some books on his desk, like "Code Complete" or "Refactoring".
  • Assuming you're not his lead, inform his lead of your concerns and let that individual take responsibility for the problem.
  • write some unit tests that covers his code. He may not know how to write good tests, so he can use yours as an example. Set up automated scripts that regularly run these tests, and send out mail detailing what failed, so that he cannot simply ignore the presence of these tests.
Ether
+3  A: 

I would recommend "failing" any code that does not meet the quality criteria. For example, if there are missing unit tests, "fail" the code so that it goes back to the developer to be rectified. This would be similar to having the code fail UAT or something, except it's more of an internal review from team leaders in the development team.

As an example, in one place I worked, I was leading a team of developers. We had PHP production code that was writing unnecessary warnings to the log files, which eventually caused disk space problems and difficulties in debugging the real bugs.

So, to rectify the problem, I laid down the rule: if any code that you've written writes warnings to the log file, the release will fail testing, and you'll need to fix it before it goes any further.

We had almost no warnings being logged on the next release. Now, it wasn't technically a bug, just a code quality issue. The point is that someone needs to be the gatekeeper that reviews and enforces quality controls at some point in the development lifecycle.

Even better, write some automated integration tests that check unit tests exist and code is styled correctly. There are some tools that can check this for you (not sure of the names of them, someone else may be able to enlighten us on that). That way, the build fails an automated, reproducible test. Code would get cleaned up quick smart if that was happening I would say.

Dan Harper - Leopard CRM
What language/build process are you using? Here's a tool for Java for code style checking: http://checkstyle.sourceforge.net/
Dan Harper - Leopard CRM
+1  A: 

You can implement code analysis/styling tools into your build process that enforce certain design and style guidelines. The benefit of this approach is it doesn't single anyone out and it applies to everyone.

Depending on the language your project is using, there are probably tools out there to do this for you. Ex. StyleCop and FxCop for C# projects.

Dan Rigby
The advantage of automated and authoritative tools like these is that they don't play favourites -- nobody is being singled out, everyone has to meet the same mark -- which makes them less controversial to introduce than a human-moderated system.
Steve Gilham
A: 

We had a guy at work who only bathed once a week (and no, he wasn't French). And towards the end of the week his BO got pretty bad. We left a bunch of hygiene products on his desk for him to try and give him a clue. Didn't work. I think he started going 2 weeks between baths after that. Phew. This was 20+ years ago.

Coding conventions? Like what? Where to put curly braces?

Missing/bad comments? Well, sometimes no comments are better than gratuitous or wrong ones.

Sloppy code? Maybe so. Post an example or two. Maybe he's a good coder, and you're not that bright. After all, PM's aren't usually that bright, they just know how to kiss ass better than some of us care to.

Design patterns? Bullshit bingo fodder.

But taking his clothes off and leaving them on your desk is strange. IMHO.

xcramps
A: 

Relax, just invite him for a drink go out and get stoned then explain him the whole situation. The next morning if hopefuly he can still remember, he's gonna understand. If not this time he's gonna take off all his clothes and put them on your damned table :)

ZeroCool