How do you handle poor quality code from team members?

Question

I know most people have code review and standards in place, but I work at a place with poor standards. I'm not saying that my code is flawless by any means, but I find the code my fellow members submit is totally lacking of structure, standards, naming convention, etc. Whole code blocks that have been replaced are still left commented out in the final product. I'm not going to get into the actual correctness of the code, but these issues make it virtually impossible to debug when my boss inevitably comes to me to fix my team members' problems before send out their app..

My question is this, how do you guys handle poor quality code from your team members? If you work in a less structured environment, how do you approach coders about improving the quality of their own code (or your boss to get him to implement some kind of standards/code review)?

Thanks guys.

Answer 1

I tend to conduct a code review, from which I fill in a checklist.

If it's purely a coding standards issue I will get them to review our standards document and then sort it out themselves.

If it's a lack of understanding then I find a bit of mentoring goes a long way. I take them through a better approach to solving a problem and do my best to educate.

In our place part of the development process is code review so code is not released to test without one.

Sounds like in your place you don't have a process. Create a process then get people to stick to it. That should help improve quality across the board.

Answer 2

Try careerbuilder.com.

Answer 3

Its unfortunate but you basically have two options available to you:

1. Spend a lot of extra time and effort cultivating the adoption of standards and peer-mentoring your fellow employees to help them become better developers. In short, fundamentally change the culture of your development house. This will be a time consuming, exhausting, slow, and sometimes painful process.

2. Find a new place to work with better standards of code quality and design.

I'm going to guess that you like where you work and don't want to leave. So here's some ideas on what you can personally do to try to convert your culture to be more focused on standards and code quality. I hope others will add more tips like these and we can aggregate them in a list

• Pair programming / mentoring
• Promote Unit tests
• Promote design & design reviews
• Do lunchtime code reviews (those who participate will learn a lot)
• Lunchtime training for better coding techniques
• Encourage coding 'practice' (maybe competitions where the best code wins.)
• If you have a couple good people, encourage them to mentor those who need help.
• Start a workshop to teach specific techniques/technologies.
• Try having food at your workshops/trainings/reviews.
• Try to bring your code under continuous integration, if feasible.
• See what IDE plugins are available for your language & IDE. Maybe you can get people to improve their code with formatting tools, style checkers, and code complexity analysis.

It may sound like a lot of work, but your team isn't going to just get it one day without help and prompting. It may be up to you to build and nurture the team you have into the team you wish you had.

A final warning I'd like to add:

Niccolò Machiavelli, from The Prince, 1513

There is nothing more difficult to take in hand, more perilous to conduct, or more uncertain in its success, than to take the lead in the introduction of a new order of things. Because the innovator has for enemies all those who have done well under the old conditions, and only lukewarm defenders in those who may do well under the new.

Its sage advice. I've worked very hard to bring about a new order of innovation in my organization, and it is slow but measurable progress. I am lucky to have allies in this effort. I hope that you find some as well.

Answer 4

Segregating those individuals is rather like pretending the problem doesn't exist, isn't it?

Assuming some of the devs are new, I'd have to agree w/ @Campbell, a bit of mentoring can turn things around and if you haven't given a person a chance with all the details they'll need, then perhaps your team is a bit of a black box. Are you integrating other new dev members easily?

If after all that, you find yourself spending a lot of time thinking that a dev doesn't belong on the team, they need to go sooner rather than later, or you'll always wind up regretting it and be the one responsible for their mess.

If you're not the decision maker, then I guess most of this depends on your relationship with your boss and if you're comfortable making noise.

Answer 5

Everyone writes bad code from time to time, but having code reviews eliminates a lot of it. Always try to be nice to your colleagues, and never think that you know better. Be humble since it's actually easy to be wrong about stuff. :-)

Code reviews doesn't only eliminate poor code written by team members (and yourself), but is a great way to learn new things from each other.

Answer 6

The main problem with telling people that their code is 'wrong' is that they might get mad or offended etc. So rather than explicitly telling people that their code is wrong just show them the better way to do in a more conversational manner. For example, Lets say that someone is duplicating code by not inheriting properly from a base class. Rather than telling them: 'Hey dummy B should derive from A', you could go up to them and say: 'Hey look at this cool way I used inheritance to solve this problem'.

Hopefully they will see the similarities and will fix it them selves. If they don't see the similarities you could be a little more heavy handed and ask them if there is anywhere they could use this method.

Answer 7

Take the initiative and start a weekly code review. Tell everyone you just want some feedback on your own stuff, but encourage them to bring their own code too. Start small, and maybe the idea will take root.

Answer 8

You need to get someone else on your side. You are unlikely to effect much change if you are working solo. If you have some other good programmers on your team, people who commiserate with you about the poor quality of some of the code, they will be your biggest allies. Get them on-board with establishing code reviews and promotion pair programming etc.

If you don't have any other good programmers on your team, it's very likely a lost cause. If you are in this situation, though, and you really want to try to fix some of the problems, then you need to get your boss on board. Phrase the problems in ways that make sense to him (or her). If you talk about how much it sucks when you have to fix code, you're going to get tuned out. You need to talk about how making these changes will bring in more money, increase morale, and save the whales.

In any event, you will definitely need to get your boss on board with any changes. I strongly recommend reading Controlling your boss for fun and profit by I. M. Write (Eric Brechner)

Answer 9

Code reviews are good, and can be introduced relatively informally.

You can automate some rules that you all agree to, using tools like FxCop.

Before you can accuse anyone of failing to meet standards or naming conventions you have to get everyone together to discuss what those standards should be. For instance - a new team member might have come out of Uni thinking Hungarian notation is the only way to go, while the team might allow single letter variables as long as it's in a method's scope and that method is short. Neither is wrong, as long as everyone knows the standard.

In a language where there is a strong IDE, such as all the .Net ones, it's always relatively easy to debug code as long as it compiles. That shouldn't be a reason (on its own) to standardise.

Answer 10

I'm amazed at the number of comments that haven't remarked on this:

Automatic documentation

It's not a code review, except everyone reviews (at least some aspects of) the code. I've seen this work exceptionally well, and it'll affect you as well. It works because everyone wants to look good, and if autodocs of the code is displayed publicly, at least those aspects will be much improved.

Looking good vs. being good

If the code looks good, even to your completely untutored bosses, odds are that even if the algorithms are dumb, it will be a lot easier to fix simply by being more readable. Another benefit of showing the autodocs is that it's a lot easier to talk about module xyz using bogosort and why it's a problem when xyz is a black box rather than an oil-spill.

One thing though: You need to pick an autodoc format that makes the difference visible, and get people to see it. You can try to work this in by showing and referencing it when reporting your work. You want people to read new docs, much like they would read the news.

It's related to code review and tends to grow in that direction, yet noone thinks of it that way...

Answer 11

As you try to bring code reviews into an environment like this, remember that they need to work both ways. You need to get one of those less-effective programmers to review your code and learn by example. They also need to make comments in reviews of your code that you actually incorporate. Make sure that they understand that the code reviews aren't there to give you a chance to tell them when they're screwing up, but to give everybody on the team a chance to learn from everybody else.

If the code reviews and the stuff everybody else mentioned don't have any impact, it's probably time to get rid of some bad apples. (Assuming you have the authority to do that.)

Answer 12

I agree with everyone regarding code reviews, but code reviews rarely ever reveal bugs.

In our team I found out that the person who was least open to test-driven development, and don't even write tests after his code, is the one who generates the most WTFs in the code. His liability isn't just poor code -- some people just refuse to learn anything new. How to handle them? Elicit the help of the project manager. Some people need to feel the threat of being fired so that they will change.

Answer 13

Use the formatter of your IDE, and get the manager to enforce its use by all programmers. If everyone is using the same format settings, and the directive is to check in code with no errors or warnings, at least code format can be improved.

Unfortunately this won't improve design, such as lack of object oriented design.

Edit Justin Standard

You've got a point that this helps a little bit, but I'm guessing that formatting is only a minor problem compared to a lack of automation and a poor design. Still, I suppose it is an easy step to take. Taking it a step further is using tools like FindBugs for java (or similar tools in other languages) to actually analyze the complexity of the code. Some of those tools have IDE plugins as well.

Answer 14

Does your workplace have no standards or just poor standards? No standards would be no formal documentation on what the standards are, poor standards would be standards writen down but deficient in some way. It is much better to have some formal standards then no standards at all.

How you improve or introduce standards depends on your role in the organization, if you are junior it will be a challenge and you will need a soft approach. If you are senior, well it will probably still need to soft, but as a senior developer you should be at least partially responsible for setting your organizations coding standards. If you are a team lead or manager it will be easier as you can dictate the standards you want. Though I have found it is much better to use a collaborative approach to most things. Some you may not want to leave up for discussion, like the correct use of source control.

If you have no formal code review process you can always start an informal one. Ask one of your coworkers to help you out on something, ask them questions on how they think your code is. Hopefully they will have some useful suggestions. When done suggest to them that you could look at some of the code they are working on, if they need any help. In an information situation you don't want to use "review" or "code review", look, help, suggest.

I was recently on a short project that involved reviewing a 3rd party's code, one of the issues with the code was there were no standards, from the 3rd party or our mutual client. Because there were no standards I could not provide any feedback on if the code met standards which lowered the value of my feedback. I did identify inconsistencies in their code base but without a standard to could not tell them how they should change things.

Answer 15

I'm not sure if it depends if you've got a leadership position (from official company hierarchy or from technical merit) in the team or not. Getting the other team member to review your code and taking on their comments and ideas might better help them understand the wonders of code review and proper structure.

If you just try to force standards on people I've often found that code reviews descend into people complaining about how you've placed a } on the wrong line or haven't included a space after a , while missing actual problems like unchecked return values or the like.

(Edit: On the other hand, if everyone agrees where the } should be, and that's where it is, then you cannot possibly waste time on such trivial matters.)

Answer 16

Reputable, external verification of good coding practice may also be an option depending on your workplace.

Not only can a third party assume the role of mediator/arbitrator, a useful aide if there are particularly obstinate team members, but they can also add the kind of momentum to a drive for better coding standards in the future that may not achievable from within the team alone.

Answer 17

Code reviews can definitely help, but it's important to get buy in for the strictness of the review, what's off-limits (you don't want to be fighting about indenting patterns in SQL code, but obviously that matters a lot in Python code), and whether problems identified in the review are must fixes, or just recommendations.

The other important thing is to have a structure in place that stops person A who writes bad code from reviewing person B's bad code and rubber stamping it. While I think code review is a good learning tool and the more eyes the better, people will try to get away with circumventing real review by only having under qualified or, worse, unqualified reviewers looking things over. Then you get the "You have to promote this it passed review" excuse popping up. It's better to have certain key "gurus" or Devs that have a proven aptitude for spotting bad code representing the hurdle for promotion, not just any other peer on the cube farm...

Answer 18

I always teach my colleagues the core focus to have when writing code:

You are writing intructions primarily for a human to follow, then, only then, for the computer to execute.

Answer 19

In our project, we have voted that code reviews are optional where pair programming has been used, and mandatory when only one person has worked on a specific work-item.

We are using Eclipse, and for the current team size, we have found the Jupiter plugin to be a very easy to use code-review tool.

See the Jupiter User Guide

P.S. Always offer your own code for review first!

Answer 20

If this problem is endemic within the company you work for, get another job. No ifs, buts or maybes.

If this is just like one developer, try and get someone with some authority to have a word with him, and ensure he is given mentoring / direction of some kind. The reason I specifiy someone with authority is people often resent seemingly being told what to do by people they perceive as being on the same rung of the ladder as them.

The mentoring is important because it may be a case of them coding badly because they don't know any better, in which case just telling them, "Don't code like this" is no good, they need to be shown the correct way to do things.

Answer 21

Apathy works for me.

Answer 22

And while you're implementing all these wonderfully technical suggestions, don't forget to follow the

• 10 commandments of egoless programming

and if you're the new guy

• How to fit in

Answer 23

Assuming you actually want your team to improve, I would essentially recommend to work it out the way every successful team works: use their strengths and try to cover their weaknesses.

I think watching team sports will give you a better idea of what I'm talking about.

Answer 24

An important thing is to help a developer understand why bad code is bad. Maybe have the developer work on a support ticket on someone else's crappy code, so they can see first-hand how much harder problem determination is with crappy coding. Or, if they're receptive to apprentice-style learning, have them look at a particularly crappy piece of code and discuss what it does, then "pair-refactor" to better code.

Answer 25

Lets not forget one thing. There are bad programmers out there, but what if you don't understand your co-worker code ? Does that make it bad code or you a bad reader ?

I have seen people taking over some code and yell and scream this is some f*ed up code and so on. They get permission to rewrite it and either the result ends up being almost identical or worse.

But if the code is WTF-daily quality the best way is to call the programmers bluff and tell him/her to rewrite his stuff.

This is what meetings are. One thing I like about scruum meetings is they are never more than 15 minutes, no sitting down, ask for one such meeting and ask for an explanation of the code and then agree on either a rewrite or a re-read.

Answer 26

The main problem with telling people that their code is 'wrong' is that they might get mad or offended etc.

I just want to add to this. Rather than saying "wrong," I just say our code has to be "consistent." People don't like to be told that they are wrong, but I don't think people will object to adjusting their ways so that they can feel like they are part of the group.

Obviously, your group will have to hold meetings so that you guys can come up with consistent practices. This is where you present your ideas. If they are truly better than your coworkers' practices then they will bubble up to the top to become the new standard.

Of course, having a standard means nothing. You need to actually enforce the standard once it is in place.

Answer 27

Have you tried bringing beer into the office?

The Ballmer Peak: http://xkcd.com/323/

Answer 28

Where I work, whenever someone checks in code or a document, an e-mail is sent to all other devs on their team with a link to the diffs. We're all encouraged to just give 5 minutes here and there to scan through each others submissions so if anyones formatting or code is a bit off at any point, they're likely to have someone just pop over for two seconds or drop me an email to point it out. It happens to everyone, so theres no shame (unless it's atrocious!). This constant reenforcement means it's easier to learn and practise.
I admit it requires participation from everyone and the willingness to take constructive critisism but it seems to work well here!

Answer 29

I heartily recommend Joel Spolsky's Getting Things Done When You're Only a Grunt:

No matter how good your code is, your coworkers write such bad code that you're embarassed to be associated with the project. [...] But there are strategies for improving your team from the bottom, and I'd like to share a few of them.

Answer 30

using unit tests as a design tool is the best way i think you can help someone learn to be better oo programmer. By having to have their code testable they learn stuff like interface abstraction, dependency injection and testable simple APIs. Pairing is another way to help share values as a programmer. Ultimately you need to list out basic agreed standards among a team and document them and follow them to the extreme as once you slip on standards, its a slipperly slope. there is a concept of broken windows with software that i think is very important to a development team.

Answer 31

I like the guy who said the weekly code review. Good practices can be taught and hopefully the person in question is new and anxious to improve and learn from others. If not, well then, craft them a nice utility project to work so they are out of the way.

Answer 32

This will definitely need a lot of effort on your and your team's side. But, to make life easier you may use some of the tools.

I am listing down a few tools for Java and Perl:

Java:

• Jalopy -- for formatting
• Findbugs -- for best practices
• PMD -- for static analysis of the code

Perl:

• perltidy -- for code cleanup. See this to get started.
• Perl::Critic -- for critic's opinion :) .. I am looking for alternatives here.

Answer 33

Where I work, we have mandatory code review for anything going to our production servers. Our organization's main business is not producing code but we must maintain an ERP system. For that reason we have developers on staff. Our review process is fairly informal (just a little background).

My strategy for code review has been classify things I find as either:
! - show stoppers. ie: flaws in logic, unhandled cases, security holes, etc.
* - suggestions. ie: more concise methods of performing same function, more efficient, etc
? - I don't understand some portion of the code as it relates to the business need

The "suggestions" are where I try to impart (what I believe to be) better coding practice without saying "you must do it this way or FAIL". I keep it open ended and try to explain why I think its a better method. This is my attempt to improve coding practices without being being overbearing or bruising egos. We also don't have the resources available to force all our code to be pristine.

I try to make sure that the suggestions would provide significant improvement. Often people know there is a better/cleaner way but they just need to get something out the door.

How you write your review (choice of language and tone) can play into the reaction of the developer. Some people take critique better than others. Pick your battles for those who take it poorly. People who take it well (and enjoy learning) are often appreciative.

Answer 34

You sound like you are ready to help your teammates get better at their jobs, but you're not sure if you can.

When I was a manager of a software team, one of the things I looked for was the impact each person had on the rest of the team.

If you wrote great code, but got in to arguments or complained a lot, it would hurt the rest of the team's productivity.

If you make your team work smoothly, helping folks get along, or contributing ideas for teammates problems, or teaching them to be better at their work, then you have a huge effect on what the team creates. Your influence on others can far exceed your own code contribution.

In my opinion, whether your boss is able to recognize this, facilitate it, and reward it is a huge factor in whether I would stay where I was and try to improve things or quit and try my luck elsewhere.

Answer 35

If you're a .Net shop, I recommend using Team Foundation Server code check-in policies. The Static Code analysis (aka FxCop) with the unit test policy is a great gate keeper for quality code/builds.

You can also consider the Visual Studio 2008 code metrics features (e.g., Cyclomatic Complexity, Depth of Inheritance, Class Coupling, and Lines of Code).

Answer 36

We've developed our own coding standards at our company. Periodically, the senior members will do code reviews for others and let them know what things they need to fix and improve on. You can't be too harsh, but you can't let it slide either...

Answer 37

tease them unmercifully until they shape up or quit - or you find another job where you're not pulling all the weight

"I'd like to congratulate Tim on finally putting a comment in his code..."

"John wins this week's Obfuscated C Contest prize, unfortunately we're supposed to be writing VB.NET..."

;-)

Answer 38

Fire them, or fire yourself.

Answer 39

My question is this, how do you guys handle poor quality code from your team members? If you work in a less structured environment, how do you approach coders about improving the quality of their own code (or your boss to get him to implement some kind of standards/code review)?

Since your sphere of influence appears to be limited to your own team, you have to start in your own backyard, ie; lead by example. Be vocal with your peers about what you consider to be successful for you, share that with them, and avoid being arrogant.

Poor quality code usually indicates poor fundamentals. If that is the case you have to short circuit developer bad habits and tendencies. Source code has structure and syntax, so should a development team. Here are some of the things my team does:

• TDD, write your unit tests first and relentlessly test your code because you know yours and everyone elses code sucks. Say it with me, "I KNOW MY CODE WILL SUCK, I MUST WRITE TESTS FIRST." Resist the urge to integration test. Mock external classes and data to the class being tested to the point that the ability to test every line of code becomes more important than the design itself.

• KISS, keep it simple and be consistent! The simplest way is the best way every time. It will be easier to understand in 6 months and far easier to maintain. Think Fisher Price easy. Also, don't allow anyone to design in a vacuum, discuss as a team how something is going to get done. If you have a hard time doing that, you are not breaking things down to small enough tasks. This will be painful at first, but it will be worth it in the long run.

As far as coding standards go, don't develop your own from scratch. Instead ask your teammates to check out the Philips Medical Systems standards: http://www.tiobe.com/standards/gemrcsharpcs.pdf. Ask your team to discuss what you want to take away or add to that. Get everyone to participate in it's formation because this is critical for buy in. Then, don't stick it in a drawer when you're done! Post it around your office and use it when you develop. An excellent time to browse the coding standard is right after you pass your tests. You are hopefully about to refactor at this point.

Your bosses job is to hold people accountable for following these principles.

Good luck.

Answer 40

Public ridicule. Send an email around with the code in question along with "Ha Ha Ha! Great candidate for the daily WTF!" as the email subject. People will soon learn.

Answer 41

it is really nasty to have someone else complaining about your code, for that reason a more "neutral" opinion might be better and by neutral I mean non-human, at my current job we use pylint (python) and we have approval before prod, the pylint seems to be very fair since it's based on rules and policies defined globally and the programmer can check it personally before submitting (avoiding the shame of being corrected by someone else).

Answer 42

Give your team more training sessions on how to improve their quality... and a little motivation might help

Answer 43

I had a heart-to-heart with a developer one time when he checked in code that didn't even compile much less work correctly.

I asked him how he wanted to be perceived ny his peers and people who looked at the product code years later. I gave him two options:

• your code didn't work well and was terrible to maintain
• your code is a pleasure to work with and rarely have to make fixes

What type of person do you want to work with and what type of developer do you want to be?

He thanked me a year later after I had left the company.

Answer 44

"A superior man is modest in his speech, but exceeds in his actions." - Confucius

You are not going to achieve much through ridicule and the cost of replacing an employee is probably going to outweigh the cost of training that employee.

Teach them the best practice - You might just learn something yourself in the process.