Microsoft recently (on my birthday) released a tool to analyse coding structure and formatting. No - its not FxCop - StyleCop analyses syntactical formatting. Let me explain:

Background

When Microsoft released FxCop, some people cringed. I remember at my previous job when having a code review and the conversation following:

  • Consultant: "Have you FxCop'd it?".
  • Me: "No...."
  • Consultant: "Hahahah - see you in 3 months!"

Those days, for me anyway, have gone. I started to use it in my everyday coding and am now pleased I use it without a second thought.

In my position with this company, I have tried to promote its use and benefits, because it promotes good coding guidelines standard approaches to software development.

FxCop works by analysing compiled code and therefore can look into its improvements at a detailed view. This is all good, but what about coding standards for syntax!?

Overview

StyleCop is a tool written and used by Microsoft to ensure all of their code is formatted, commented and laid out in a fixed format. It has quite a rigid structure to it, but so it should. I've read lots of threads about people complaining about its flexibility, but that's the whole point of standards - they don't change (often) .

Before I provide a link, I just want to show you how much you need to do, in order to pass one of these successfully. Here is a typical piece of code:

using System;
using System.Diagnostics;
using LogicAndData;

class User {

private string _username;

public string Username
{
get { return _username; }
}

[DebuggerBrowsable(DebuggerBrowsableState.Never)]
private string _password;

public bool IsCorrectPassword(string guess)
{
if (String.IsNullOrEmpty(guess))
return false;

return guess.Equals(_password, StringComparison.CurrentCulture);
}

public User(string username, string password)
{
if (username == null)
throw new ArgumentNullException(Resources.Username_ParameterName);
else if (username.Length == 0)
throw new ArgumentException(Resources.Username_ParameterEmpty, Resources.Username_ParameterName);

if (password == null)
throw new ArgumentNullException(Resources.Password_ParameterName);
else if (password.Length == 0)
throw new ArgumentException(Resources.Password_ParameterEmpty, Resources.Password_ParameterName);

_username = username;
_password = password;
}
}

Pretty standard stuff really. Okay, its missing comments but it does pass FxCop! So lets have a look at the errors generated for this class:

styleCopErrors

So, what are the problems?

  1. SA1101: If you are referring to member variables (variables at class level), then you should use the keyword "this." to be explicit (2 errors)
  2. SA1201: This is a formatting issue - the order is - private fields, constructors, properties, public methods, private methods. (2 errors)
  3. SA1309: Don't use underscores! The "this" keyword implies it. (2 errors)
  4. SA1400: Class must have an identifier instead of the default (protected internal) (1 error)
  5. SA1503: If statements must use { }, even if they are only 1 line of code (6 errors)
  6. SA1600: All classes, interfaces, methods, properties and fields (optional) MUST have an XML header for documentation purposes. (4 errors)
  7. SA1633: Related to the above, there is a preset format for the documentation for the header of the file that MUST be at the top of each file created. (1 error)

Using StyleCop

What does this improve? relating to the above:

  1. It is clearer what you are working with. You don't need to use prefixes, as by using "this" you are describing which one you are referencing anyway.
  2. This just standardises the layout of your code.
  3. For clarity. It doesn't like Hungarian notation either, but DOES allow you to add acceptable notations should you wish.
  4. Be explicit about the access type, as you may not understand the access of an unmarked class.
  5. By laying out if statements this way, you are totally consistent across your code.
  6. Documentation - who doesn't document their code!?
  7. Add a header to just clarify what's doing on, any copyright notices, authors etc.

So lets see what this looks like without any errors at all!

[40 minutes later and 3 cups of coffee].... only joking - it probably took 5 minutes, mainly because I have used it before:

// <copyright file="User.cs" company="Interakting">
//     Copyright (C) 2008 Interakting. All rights reserved
// </copyright>
// <author>Dominic Zukiewicz</author>
using System;
using System.Diagnostics;
using LogicAndData;

/// <summary>
/// The User class describes a single username/password combination. The password
/// is kept secret, but timescales limit security.
/// </summary>
internal class User
{
    /// <summary>    
    /// The username of the user.    
    /// </summary>    
    private string username;

    /// <summary>    
    /// The password of the user.    
    /// </summary>    
    [DebuggerBrowsable(DebuggerBrowsableState.Never)]
    private string password;

    /// <summary>    
    /// Creates a user with a specific username/password combination    
    /// </summary>    
    /// <param name="username">The username of the user.</param>    
    /// <param name="password">The password of the user.</param>    
    public User(string username, string password)
    {
        if (username == null)
        {
            throw new ArgumentNullException(Resources.Username_ParameterName);
        }
        else if (username.Length == 0) 
        { 
            throw new ArgumentException(Resources.Username_ParameterEmpty, Resources.Username_ParameterName); 
        } 
        
        if (password == null) 
        {
            throw new ArgumentNullException(Resources.Password_ParameterName); 
        } 
        else if (password.Length == 0)
        { 
            throw new ArgumentException(Resources.Password_ParameterEmpty, Resources.Password_ParameterName); 
        }
        
        this.username = username; 
        this.password = password;
    }

    /// <summary>    
    /// Gets the username of the user.    
    /// </summary>    
    public string Username 
    { 
        get 
        { 
            return this.username; 
        } 
    }
    
    /// <summary>    
    /// Checks to see if the password of the user matches that of the stored password.    
    /// </summary>    
    /// <param name="guess">The guess for the password.</param>    
    /// <returns><code>true</code> if it matches, else <code>false</code>.</returns>    
    public bool CheckPassword(string guess)
    {
        if (String.IsNullOrEmpty(guess))
        { 
            return false; 
        }
        
        return guess.Equals(this.password, StringComparison.CurrentCulture);
    }
}

So what's changed - really?

What I have noticed, is that there is a good separation of the code, a very clear layout structure and of course, forcing you to comment your methods, which I really like. You'll find that the code is clearer because of the white-space implied by the curly brackets.

It can seem quite daunting to see all of these errors, but like FxCop, once you start to take these rules onboard, you are more likely to code in this style rather than turn away from it.

Here is the link to Microsoft's StyleCop: http://code.msdn.microsoft.com/sourceanalysis/Release/ProjectReleases.aspx?ReleaseId=1047


Bookmark with :
Digg It! DZone StumbleUpon Technorati Reddit Del.icio.us Newsvine Furl Blinklist
posted @ Wednesday, July 23, 2008 3:23 PM | in Good practices

Comments

No comments posted yet.

Post Comment

Title *
Name *
Email
Url
Comment *  


Please add 5 and 6 and type the answer here: