Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

We have a small dev team (only 3 developers) and we recently got a new team member. While he is a smart coder, his coding style is completely different from ours. Our existing code base contains mostly readable, clean and maintainable code, but the new team member is quickly changing many files, introducing ugly hacks and shortcuts, using defines all over the place, adding functions in the wrong places, etc.

My question is if others have experienced such a situation before, and if anyone has tips on how to talk to him.

share|improve this question
1  
Considered using peer review to catch the ugly hacks and shortcuts before they reach the repository? –  user1249 Nov 13 '11 at 14:57
    
Use good, unbiased automated tools whenever you can. –  Job Nov 13 '11 at 16:06

5 Answers 5

I work with a team that grew from 2 developers to 10 in less than a year. I was number 3, and the first to raise a coding standards issue. The two original developers had been working side by side for a few years and they've adopted a common standard that seemed alien to me. We had exactly the same problems you are describing.

What we did was:

Research coding standards

We spent a few days checking out established open source projects. We knew the team would expand rapidly and we where looking for real solutions based on real projects not some generic guidelines. Also we didn't care for the optimal coding standards, but for a set of rules and guidelines that would make sense and not call for the refactoring of all of our codebase. We were looking for a coding standards hack if you will.

The three of us decided that the best coding standards out there for an established php project where those followed by Zend Framework. Fortunately the Zend Framework people provide a very comprehensive coding standards document.

Creating our own coding standards

Of course applying another project's coding standards on our project as is didn't make sense. We use the Zend Framework document as a template:

  • First we removed everything that didn't apply to our project
  • Then we changed everything that we perceived as a matter of style to our style
  • And finally we wrote everything down

So we had a fairly large document at our hands, stored in our fancy wiki, it was a nice read, agreed upon by all of us. And completely useless on its own.

Staying true to our promise

Our codebase at the time was about 1*10^6 sloc. We knew that since we adopted formal coding standards we had to start refactoring our code, but at the time we where pressed with other issues. So we decided to just refactor our very core libraries, a mere 5*10^3 sloc.

We assigned one of us to be the coding standards master (we used local profanity in place of master) with the responsibility of checking and enforcing the standards. We recycle the role every few sprints. I was the first, and it was a lot of work, as I had to monitor almost every commit.

We had several new discussions and small addendums to the original document during my tenure, and finally we had a somewhat stable document. We change it every now and then but most of these changes are on new features of the language, as PHP 5.3 was a major release in all but name.

Dealing with the new guy

When the next new guy arrived, it was time to put our coding standards to the test. After a small introduction to our codebase, we asked him to evaluate our coding standards document. He almost cried. It appeared that he did everything differently.

As I was the coding standards master at the time, it was up to me to evaluate his input and revise the document accordingly. His proposals were:

  • Matters of personal style (summarily dismissed)
  • Standards that made sense to his Java background but no so much with PHP (dismissed)
  • Conventions that he carried from his brief exposure with PHP (some where dismissed, but a lot proved to be popular conventions that we never thought of or found out in our initial research)

For the next couple of weeks he was assigned a simple task: Bring several parts of our codebase up to date with the standards. I had to carefully choose those parts based on a few rules:

  • Code should be relatively easy for someone unfamiliar with our codebase (and PHP in general)
  • Code should be on what he was hired to do.

I monitored his process and he did a fine job. We identified several parts of code that was impossible to fit our document and revised accordingly (code and / or standards, whichever made more sense)

And then another new guy arrived. We repeated the process (different master this time), and it work again. And again.

In conclusion

  1. Create a coding standards document, but make sure that your standards are not exclusively your own but do reflect common standards in the wider community of your platform.
  2. Assign a similar role to our coding standards master. Someone to monitor at least new code, and especially new code from new members. Recycle the role, as it's extremely boring.
  3. Always evaluate input from a new member. Always revise your standards if it makes sense. Your coding standards document should be evolving, but slowly. You don't want to re-refactor your codebase at each iteration.
  4. Allow for some time for each new member to learn and adapt to your standards and conventions. Learn by doing works best in these situations.
  5. Wiki works wonders for such documents.
  6. Code reviews work wonders for any situation!

Update: At some point in the process it was suggested that we use a pre-commit hook to automate checking of the standards. We decided against it for a variety of reasons, there are some interesting discussions on StackOverflow on the issue:

Some are PHP specific, but the answers apply to all platforms.

share|improve this answer

Yes, I've experienced that before. When working in a team, team members must agree on certain rules and conventions, and that includes the style.

You should have your team sit down together and draft a set of rules, coding standards, that you would require every piece of the checked in code to adhere to.

Most probably, the base for your set of rules, re the styling at least, would be the existing code. Once its done, everyone must comply, and it should be inspected as part of the code review. Code not adhering to the standards should not be allowed to be checked in.

It doesn't have to be a democratic vote, by the way, its one of the things where the team leader can actually execute some authority. But having said that, I don't think that you can impose standards that the majority of the team rejects. You can impose standards that a single person, especially a new one, rejects.

As to how to talk to him... Every experienced programmer knows that each place and team has its own conventions and style, that should be followed. You can tell him that he's more than welcome to suggest improvements, but he has to comply with the rules the team has, and he shouldn't change the style of the existing code but rather use the same style when adding new code.

Also, you can tell (if you're the manager, or talk to your manager about it) to that person not to do certain things that you deem inappropriate (you mentioned defines, order, hacks and shortcuts, and such).

share|improve this answer
    
That's how we did it in our team: we discussed and approved a coding standard document and we use code reviews for each check-in. It works pretty well. –  Giorgio Nov 13 '11 at 8:15
  1. Someone is in charge - they need to act like it.
  2. If coding style is so important, why wasn't this explained to this person and let them know that they will not have access to any code until they learn the rules.
  3. Code Review - apparently you don't have any or it is very weak. See #1.

Make a note in your hiring process, that following the accepted coding styles is a requirement for employment. Now what do you do to those who don't follow the rules? Start by removing their access to live code until they get with the program.

.

share|improve this answer

Here's what can be done:

  1. Write a document explaining the coding style required and make everyone in the team learn it. Collect information from every team member.
  2. divide tasks in such way that every team member is responsible of their own piece, and can decide conventions of that part of the code. If any problems are found, whoever wrote it will fix the problems.
  3. add an automatic tool to version control which fixes indentation and other stuff every time code is committed to the version control
  4. Different programmers always have different programming style, and later it can be difficult to change. Best way to handle it is to share information between team members so that everyone learns what styles people have used. If you have a team member who writes different code, it's a chance for your existing team members to learn the new style.
  5. One good trick is to never modify existing code. Instead of modifying the code, write new code by replacing empty lines with new code. And once the code is ready, do only smallest amount of modifications to existing system to take the new code into use. This avoids tweaking existing code, possibly breaking what was already working fine.

Here's what to avoid:

  1. deciding that someone's code is better or worse than other team members. It just doesn't work like that -- everyone knows certain subset of the language well enough to use it in code. Every programmer has chosen different subset to learn, and unless they learned it together, it's going to look different.
  2. Changing how someone writes code. All you get by forcing people to write unfamiliar style is that you get large amount of bugs in the code. People just do not know enough details of something they use the first time. Programmers always choose a subset of language and use that alone. If your programmers have written thousands of lines of code that's filled with gotos, then gotos are going to give you code that has least amount of bugs.
  3. You also shouldn't think that your existing codebase is nice, clean, maintainable stuff. There is always things to improve. But also every change blurs the original design idea that was written to it. Aim to write perfect code the first time, so that the changes won't be needed later. (the new guy wouldn't need to "break" your perfect code, if it was done correctly the first time)
share|improve this answer
    
so to use your answer in OP's original context... there's a programmer who inserts hacks, uses macros and has other bad coding habits, so you are suggesting to carve out a part of the product, give it to him and instead of calling his code "bad", call it "different". I couldn't disagree with this more. When working as a team, constant communications, design/coding discussions and reviews are an important part and as the team matures, your team members will all INCREASE in their skill because you as you pointed out, we all start out with different subset, but by talking to each other, we... –  DXM Nov 13 '11 at 4:57
    
... teach each other, so the skill and competency of your entire team goes up. Otherwise, you will have parts of the product which are good, but you will have many more parts which become unmaintainable messes, and your "owners" of those messes will simply continue hacking away fixing those bugs as they come in. With this isolation model, I've seen people take years working on the same component that was never done right. –  DXM Nov 13 '11 at 4:59
1  
No, the problem here is not that someone uses bad coding habits. The real problem is that they already decided they have to change how one person writes code, while the rest of the team thinks their own code is perfect. People will improve their coding style if you give them chance, but these people decided to force someone to improve quickly, while they never bother to do the same. –  tp1 Nov 13 '11 at 5:03
    
@DXM A lot of great language features get called 'ugly hacks and shortcuts' by people who haven't seen or used them before. Best thing is to talk about the standards rather than just decide that the new guy is a hacker. –  Kirk Broadhurst Nov 13 '11 at 15:05
    
we could be basing our answers on different experiences here. Among other things, OP said "using defines all over the place". If that's instead of typed constants, not that bad, but could be improved. But I've seen people #define a chunk of code because they were too lazy (or no skill) to refactor the class properly and put common code into a function that could be debugged. Absolutely no way, would I ever consider that "a different style" and allow them to continue to do that. Furthermore, all other answers focus on converging the team towards a common style/convention. This answer... –  DXM Nov 13 '11 at 21:07

Our existing code base contains mostly readable, clean and maintainable code

One thing I've learned over the years is that readability is in the eye of the beholder. I've seen many cases where someone's chickenscratch coding style is justified as being "readable", and I've seen perfectly reasonable people argue about which coding styles are the most "readable". Maybe this guy doesn't view your style as readable?

That said, the new guy should conform to your standards, not the other way around.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.