Implementing and Enforcing Coding Standards

Question

My team (of which I am the newest and most junior member) has increased in size from 3 to 9 developers in just about 1 year. Our primary product has increased in complexity and we are about to undertake a year long port/re-write to Silverlight. In the past there has been no specific style/standard enforced.

I suggested to my boss that now would be a good time to implement such standards. I passed on IDesign's document to him and he likes the idea. He has 2 concerns.

1. This is a big document to absorb. My thought here is to develop a slimmed down cheat-sheet for the most common items that we're likely to run into, with the understanding that the IDesign standard is the "Master" and anything not covered in the slimmed down version should be looked up in the "Master" document.

2. What is the best way to enforce this. It's not a question of trying to dictate; it's a matter of trying to get folks used to developing to a particular standard. There are at least 2 folks on the team that have been developing to the current non-standard for several years now. In order to address this concern, I'd like to see if there's a tool that can be configured to enforce these standards, or at a minimum warn of "violations" of the standard at either compile-time or design-time. I found Microsoft'd StyleCop, but from what I've been able to determine, it isn't configurable and is set up to follow Microsoft's standard, which doesn't completely mesh with IDesign's.

Any input on tools or the approach I'm looking at would be appreciated.

Answer 1

Try ReSharper, it can format your code to your style. Even reformat the whole solution at once.

Answer 2

Regular code reviews would be a good way to go...

I've found developers respond better to face-to-face discussions about a particular standard rather than leaving it solely to a tool.

Using a tool together with code reviews should be good.

Answer 3

StyleCop would be a big help.

Answer 4

If your team has a continuous build (e.g. using Hudson), you can enforce style constraints by failing the build or making it unstable when someone commits changes that violate the style guidelines. Hudson has a Violations plugin that can be used with tools such as Checkstyle and StyleCop.

Answer 5

Thank you all for your advice. I'd love to hear more. As it happens, when I was looking into Resharper as recommended by Ilya Ryzhenkov I happened to re-download the IDesign standard, which comes in a zip file with a link to this blog entry. The good part is it defaults to the standard I want to use. It's inexpensive (read free if you don't donate) and Ihappend to be a big fan of Code-Rush! and Refactor, so I already have the DXCore loaded!

Answer 6

Without knowing what your configuration management setup looks like, I would say you should first look for tools that integrate with your version control system that evaluate code before the code is committed to the repository. I have done this with our SVN setup and have seen it done with CVS as well...it's effective to a certain degree because it forces the developer to submit "correct" code and keeps your repository tidy. A simple example of this would be rejecting a commit if the user did not supply specific information in the commit message (i.e., bug #, project task, etc).

Tools aside you will have to find a way to portray the value of a coding standard to the developers. Sounds simple but it has been the toughest part for me to do so far. Show the developers that a little bit of effort can go a long way when it comes to maintaining a code base.

Answer 7

A combination of ReSharper, FxCop/StyleCop (there is a way to define custom rules at least for FxCop), clear code guidelines and monthly reviews should do the job for a team of nine people I think. If someone breaks the rules, you'll have no way but to use a whip :)

Answer 8

Its difficult to overstate the importance of coding standards. The key is that you should have a consistent code base that everyone can read and understand quickly. Its less important which set of standards you choose (so long as everyone signs up) - but if you choose a standard that's widely used then fewer developers will have to adjust their style. The Microsoft Design Guidelines for Class Library Developers are my favorite (and very ReSharper friendly).

A colleague of mine (Howard van Rooijen) and his open-source team are developing the excellent StyleCop for ReSharper which shows you style violations in real-time by highlighting them as you type! There has been a recent release which is heartily recommended.

Answer 9

I've just written a blog on using ReSharper and StyleCop together, and enforcing it via Continuous Integration (using NAnt).

http://www.robertbeal.com/archives/47

See what you think of it. It works very well for us. A few grumbles as at current I fail the build if a single code violation is introduced. We do have tens of thousands of violations though (mostly in old legacy code) so my response is to go cleanup one of them, and that are code should be getting better not worse.

I got sent this in response once, Hitler's Nightly Build Fails: http://www.youtube.com/watch?v=Azl4nqLn4-Y

Answer 10

In our company we use reference cards for C#. Reference cards are made in Powerpoint using 2 slides. A frontslide and backslide (2 page printing). Each slide has 3 columns (newspaper setup). Between each column 1 cm of white is made to achieve folds.

Place the most important aspects of the full coding guideline of the company in the 2 slides (for example we have: coding style, namespaces & solution structure, naming conventions).

You chose IDesign's out of the box coding guidelines. Maybe it is easier for you to adept the MS coding style, but that's your choice. Most developers are familiar with the MS guidelines and therefore easier to adept.

Answer 11

Find something that is way shorter than that. Developers have so much to remember that it is a waste of time to expect them to memorize more than a page of rules. Even if you give them a cheat sheet, unless that is the official and only document you just end up with developers that invent their own style for the rest.

Better to have a simple convention that is followed than a big complicated document that isn't followed.

As an aside, have you ensured proper by in from the team?

Al most all the code standards I have seen have mandated silly stuff like all if statements must contain braces, which ruins stuff like

public bool compare (Object other){
  if(other == null) throw new NullPointerException("You twit, don't give me a null pointer");
  if(!(other instanceof CustomerObject)) throw new UnsupportedArgumentException("Give me the right argument, dammit");
  ...

Because such stuff now takes up three lines, and therefore don't get written (or if it does, prevent the programmer from figuring out what the method is doing).

Answer 12

Notes from my experience getting buy-in on coding standards at my current company (small sized developer of multiple projects, each with 1-6 programmers apiece; we're primarily C++ but I imagine the answers will still apply):

A code review cheat sheet is a great idea. Tailor it toward your organization (e.g. what is easy to overlook or get wrong), and update it as you go (once a month, come back and remove stuff that's enforced in other ways). If you have a wiki, include links to "why" for each point, if you can!

Split your reviews. Some commits beg formal reviews, some don't. We use a few types:

• Mini-Design-Doc reviews wherein a short (usually one or two pages on a wiki) are commented on by the group before implementation starts. Great for spotting "reinventing the wheel" early.
• Guided warm reviews where one or more peers sits with the original author before commit (great for spreading expertise, bringing coops/interns up to speed). The original author tends to spot more problems than the reviewers in these. =)
• Formal post-commit cold reviews where someone with no guidance (other than the checkin comment and any documentation) reviews the code. This tends to reveal logic or error-handling problems, as well as boundary bugs.

Automate, and push, don't pull useful info. We mail out reports from our buildserver -- it usually builds once per commit (depending on how busy it is). These reports can include e.g. the differences between a current Gimpel PC-Lint run and the last. This addresses the "too much info" concern: you get just the warnings/errors you are possibly responsible for, along with description. When the info is narrowed down and easy to understand, people use it as a learning tool.

I can't emphasize this bit enough: don't sweat the small stuff. (See point #0 of the marvelous C++ Coding Standards book.)

• Split your coding standard into two or more sections, a "required" and "recommended" section -- allow people to read the recommended section at their leisure, and keep the required section small.
• When reviewing, explicitly delineate stuff that is really important to fix right now, and stuff that isn't. For our "warm reviews", for instance: bracket placement and naming inconsistencies are explicitly "fix this later", because we don't want to invalidate any testing people have done before the pre-commit review. Logic bugs are "fix now, before commit". Error-handling inconsistencies or missing cases vary. If you require people to fix even piddling violations immediately, you'll get resentment.

Finally, encourage participation and mutate your coding standard over time. (This is where a wiki can be useful.) If someone has a legit reason for not following a piece of the standard (either they have something better, or it's too much of a PITA to follow), listen and respond. If people actually actively contribute to and understand the standard rather than just being handed it, you'll get a lot better response.