Coding Standards Shouldn’t Be

Occasionally I get asked to contribute to a discussion about coding standards, or write and/or review a coding standards document that will be used by a team for all future projects. It is around this time that I start looking from side to side for open windows to dive out of.

Its not that I dislike coding standards, after all the goals of increasing maintainability, testability and performance (among others) are good, but unfortunately the delivery quite often sucks. Your typical coding standards document usually consists of a dozen to a thousand pages (I’m not making this up) of text and code trying to describe exactly how code will be written.

Unfortunately, despite the stated goals of coding standards, they do very little to improve the quality of the code produced. If you are lucky this is because whilst you have a coding standard, everyone is smart enough to ignore it and still remain productive and write relitively good code. If you are unlucky it is because the standard was poorly written with absolute rules and is now being enforced by an in-experienced team leader. OK, I’m getting a little melodramatic now, but I wanted to make a point.

Coding standards that I have seen tend to focus things like naming conventions, and I’ll agree that naming conventions are good things to have, especially when it comes to the externally visible parts of the API. In that instance I will typically refer people to some pre-existing documentation provided by the platform vendor, for .NET that would be the Design Guidelines for Class Library Developers, with a particular emphasis on the Naming Guidelines. Beyond that Chris Sells coding standards plus my experience gives me everything I need (in fact I’m not even that fussed about the first rule).

When I tell people this, the usual reaction is “but Mitch! you can’t be serious! there will be chaos! and lots of exclaimation marks!”. And the truth is that there probably will be, but coding standards aren’t the only way to improve code quality. Coding standards a little bit like backups, you don’t know they are working until you go to restore them, and you know what most people that try to do a restore for the first time when it really matters end up being disappointed by the results.

So how do we test our code quality? Well, this is where the human factor comes in, I believe that the only way to improve the quality of code is to review it. There are definately a lot of tools out there which can detect simple things like name standard violations and common mistakes in local code regions but more abstract things like API usability really require an experienced team member.

So how does that work? Well, you can get as sophisticated as you like with code reviews and do things like bundle up the current version of the code with the last reviewed version (if it isn’t virgin code) and fling it across to someone. They can then send back a modified version of the source file which contains the review comments which the developer would use a diffing tool to find. In C# one effective way of doing this is putting in a // REVIEW comment that gets picked up by the VS IDE.

public void DoSomething()

{

    // REVIEW: Hi Jim, I have a few concerns about this code. Maybe you

    //         could read these review comments and see if you want to

    //         restructure the code a little bit.

    //

    //         The first issue is that I think the method is too long, there

    //         is a lot of complex initialisation logic here with external

    //         dependencies. So if something goes wrong its going to be hard

    //         to determine from a Release stack trace where it went wrong.

    //

    //         I’d suggest extracting the initialisation logic and the

    //         calls that use those locals into a seperate method.

    //

    //         Unfortunately the way the code is structured now, our

    //         refactoring tool can’t help you. Because you won’t be able

    //         to select on of the continious region of text to extract.

    //         Co-locating variable declarations where they are used can

    //         help here and might be something to look out for in the

    //         future. Thanks for listening!

    int a = <insert complex init logic>;

    int b = <insert complex init logic>;

    int c = <insert complex init logic>;

    int d = <insert complex init logic>;

    int e = <insert complex init logic>;

    int f = <insert complex init logic>;

    int g = <insert complex init logic>;

    int h = <insert complex init logic>;

    int i = <insert complex init logic>;

    DoThis(a, d, g);

    DoThat(b, e, h);

    DoThese(c, f, i);

}

Other review techniques I have seen include using Word and the review feature which looked really good with the call outs but there was a tendency for longer lines of code to wrap in nasty ways. The other issue with the above review is that is quite lengthy, and I can tell you from experience that I make this particular review comment quite regularly. This is concievably where you might want to reference some *shock* coding standards documentation.

Before you all race off and start quoting doc ID’s and page numbers in your review comments, can I suggest that you look at the above review comment more closely – what does it consist of? Well, for starters it mentions context (complex initialisation is making the method long), it mentions a scenario (if something goes wrong it’ll be hard to figure out from the stack trace where it went wrong) and a possible solution (extract methods), and just to make it interesting, it overloads the comments with some feedback on how the positioning of variable declarations can impact refactorability.

So, you should probably change your coding standard from a “standard” to a number of related contexts, scenarios and solutions. Hopefully this is all sounding very pattern like to you. So while we are stealing our templates from the patterns community why don’t we steal the means of distribution too! One of the most popular (and first) pattern repositories on the web is the Portland Pattern Repository.

What I propose is instead of killing trees writing and printing multiple versions of word documents, you instead use an internal Wiki Wiki Web a repository for your coding contexts, scenarios and solutions, you can then reference URLs on that Wiki inside your review comments, so the above would become.

public void DoSomething()

{

    // REVIEW: Hi Jim, I have a few concerns about this code. Maybe you

    //         could read these review comments and see if you want to

    //         restructure the code a little bit. See:

    //         http://codereview/Default.aspx/CodeReview.ExtractMethod

    //         http://codereview/Default.aspx/CodeReview.MethodExtractability

The beauty of using a Wiki is that it becomes a living document where other developers can get in there an challenge it if it doesn’t make sense and related concepts become connected through the use of keywords. By the way, if the URLs above look a little bit funny thats because they are from FlexWiki which is a .NET Wiki Wiki Web implementation.

One of the issues with my form of review comments (encoded in source files) is that it still doesn’t let me select multiple non-continious regions of text from a single source file or across source files. A specialised tool would probably be needed for that. MichaelW of Microsoft provided a few pointers in that regard.

Finally, notice how I got threw a whole post on coding standards without even declaring my preference? Interesting . . .

About these ads

One thought on “Coding Standards Shouldn’t Be

  1. David Bennett

    We use this:

    1. No tabs, just spaces.

    2. Match existing.

    3. If the editor does reformatting, use it and like it. Otherwise, just make it look nice.

    4. Give external/global variables Big Important Names and vice versa.

    I don’t think Chris Sells pinched the first two from us, but you never know.

    David

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s