Coding and Developer Quality Standards

Purpose and Scope

The purpose of this article is to prescribe style and naming standards as well as quality metrics for developing Microsoft.Net C# v4.0 code.  This applies to any application written using the Microsoft.Net framework version 4.0 and above and C# version 4.0 and above.  

Introduction and Basis

This guideline is based on the recommendations of the book Framework Design Guidelines: Conventions, Idioms, and Patterns.  The book is a highly respected resource in the .Net software industry written by the Microsoft architects of C# and the CLR. It is endorsed by dozens of industry leaders. 

All the rules and standards in this document are an executive summary of the rules and best practices described by this book.  For more information about any rule and the reasoning behind it please refer to the appropriate section in the book.  This document is laid out identically to the book and also available online here. 

Who are the authors of this book?

1. Brad Abram – Founding member and Architect of the Common Language Runtime and .Net Framework. He has been architecting the .Net framework since 1998 and was employed as the .Net Framework Program Manager by Microsoft. He has since left Microsoft and has taken an undisclosed R&D role with Google.

2. Kryzysztof Cwalina – Founding member and Architect of the Common Language Runtime and .Net Framework. He is currently the Program Manager for the Microsoft.Net Framework. Kryzysztof wrote both FxCop and the StyleCop tools.

This article does not (and should not) contradict the book it is based on. Corrections will be applied if any such discrepancies are found.

There is also some influence on my own personal experience at numerous software development companies over my 20 years writing software.


Coding Standards Goals and Overview
This Coding Standard aims to achieve two primary goals:
1) Define how to write code that is readable and consistent. (Limit the use of acronyms, and symbol prefixes).
2) Define how to write code that is as unambiguous as reasonable. (If possible one line of code should be interpreted one way only).
This guideline is intended to be used in conjunction with other measures to increase the quality to a new level. This includes but is not limited to: QA handover checklist of developer outputs, peer review process, and controlled change/release management.

Tools: Resharper and Stylecop
Resharper and Stylecop are used to enforce the style conventions and make them easy to implement while typing code. Both of these tools will be used to more easily comply with this document. It is far easier to be guided to write code according to a standard from the start as you are typing than it is to resolve violations over thousands of lines of existing code.

Another useful tool is Regionerate, this tool sorts members within a class into grouped sections (optionally with region tags) and sorts members within into alphabetical order.


Conventions Used In This Document
The guidelines are organised as simple rules using Do, Consider, Avoid, and Do Not. Each guideline describes either a good or bad practise, and all have a consistent presentation. The wording of each rule indicates how strong a recommendation is.

A Do guideline is one that should always be followed.

Consider should generally be followed but if you fully understand the reasoning behind the guideline and have a good reason not to follow, exceptions are allowed.

Do Not guidelines indicate something you should almost never do.

Avoid guidelines are less strong than Do Not, and indicate something that is generally not a good idea, but there may be cases when exceptions must be considered.

Naming Standards
Definitions:
The following are definitions of the terms Pascal and camel casing. They describe capitalisation conventions using upper and lower case letters when naming.
Pascal Casing
camel Casing


Global Naming Rules

AVOID using abbreviations, prefer to use full English words spelt correctly. Acronyms and abbreviations decrease the readability of the code.

DO Capitalise only the first letter of an acronym if one is used. For example Html. Except for 2 letter acronyms.

DO Capitalise the both letters of a 2 letter acronym. For example System.IO.

DO favour readability over brevity.

DO NOT use underscores or hyphens or any other non-alphanumeric characters.

DO NOT use Hungarian notation. Hungarian notation is defined as symbolic prefixes that were used to indicate scope and type before the IDE and language provided better mechanisms.

string m_variable; // Hungarian notation
int mi_variable; // Hungarian notation

DO NOT prefix local variable names.

DO always use an access modifier. Ie always specify public, private, protected, or internal.

AVOID names that conflict with Keywords in C#. You can add square braces to avoid the compiler error, but you shouldn’t.

DO NOT Use acronyms UNLESS they are widely accepted.  Using a full word makes the code more readable and more approachable to new and more junior developers. Remember the popular saying “the English language is a good language, let’s use it effectively.”

DO use semantically interesting names rather than language specific or domain specific terms. For example use GetLength() over GetInt().  Avoid ambiguous names.

CONSIDER wrapping long lines of code to make them more readable without having to left-right scroll.  A long line is 150 characters or longer.  Resharper can be configured to automatically wrap based on a user defined right margin.

DO wrap method parameters on separate lines IF a long line is wrapped. This is only necessary if the line is wrapped.
public void SomeMethod(

    int aReallyLongParameterName1,

    int aReallyLongParameterName2,

    int aReallyLongParameterName3,

    int aReallyLongParameterName4)

{

}

DO wrap implements and generic template restrictions if there are more than 200 characters.
public class Derived : BaseClass,

                IInterface1,

                IInterface2,

                IInterface3,

                IInterface4

{

}

CONSIDER placing multiple attributes on separate lines for clarity.

[Serializable]

[FooAttribute]

Public class Apple
{
    ...
}

C# to Win32 or C# to C++ API Interface Naming

Win32 and C++ libraries can be called from C#; when doing so, you must declare the function prototype in C# using the extern keyword.  It is best practice to declare this function prototype to have the same argument names as they are defined in the C++ API.  Quite often the external function will require the use of a specific type. These must be named correctly for the function call to work. In this case the naming required by the external interface trump these guidelines.

CONSIDER placing all wrapper code that calls into Win32 into a separate folder and namespace. For example .InterOp.

Public Naming Casing

Identifier	Casing	Example
Namespace	Pascal	namespace System.Security { ... }
Type	Pascal	public | internal | private class StreamReader { ... }
Interface	Pascal and are prefixed with I	public | internal interface IEnumerable { ... }
Method	Pascal	public| internal | protected | private virtual string ToString() { ... }
Property	Pascal	public | internal | protected | private int Length { get { ... } }
Event	Pascal	public | internal | protected event EventHandler Exited; private event EventHandler exited;
Field	Pascal	public | internal | protected TimeSpan InfiniteTimeout; private TimeSpan infiniteTimeout;
Enum Value	Pascal	public | internal | protected | private enum FileMode { Append, ... }
Parameter	Camel	int ToInt32(string value);

Class Names 
DO NOT prefix classes with a prefix. For example do not prefix with “c” or “cls”.
DO Use nouns or noun phrases for your classes (and avoid plural names).

Common Names:
These are conventions followed throughout the .Net Framework.

Base Type	Derived / Implementing Type
System.Attribute	DO suffix with “Attribute”.
System.Delegate	DO add the suffix “EventHandler” if it is used as an event type. DO add the suffix “Callback” to names of other than those used as event handlers. DO NOT suffix with “Delegate”
System.EventArgs	DO add the EventArgs suffix
System.Enum	DO NOT derive from this class. Use the C# keyword. DO NOT add the suffix “Enum”
System.Exception	DO add the suffix “Exception”.
IDictionary<TKey, TValue>	DO add the suffix “Dictionary”
IEnumerable ICollection IList	CONSIDER adding the suffix “Collection” or “List”. If not, use a plural name. However, plural names have always been used for arrays, to avoid confusion it’s better to suffix with List or Collection or Set.
System.Stream	DO add the suffix “Stream”.
CodeAccessPermission IPermission	DO add the suffix “Permission”.

Enum Names

DO use a plural name for [Flag] enums.

DO use a singular name for non-[Flag] enums.

AVOID using the Flag or BitMask suffix for [Flag] enums.

DO NOT prefix enums, for example:
don’t do this:
public enum ImageMode
{
    ImageModeBitmap = 0,
    ImageModeGrayScale = 1,
    ...
}
rather do this:
public enum ImageMode
{
    Bitmap = 0,
    GrayScale = 1,
    ...
}

Property Names

DO name arrays with plural names. string[] apples;

DO name Boolean properties with affirmative phrases rather than negative phrases. For example:
IsVisibile, IsEnabled, CanSeek etc.  A badly named property and use case would be InActive = false; This is not immediately understandable and can easily lead to misunderstanding when quickly reading it.

Event Names

DO name events with a verb or verb-phrase. A verb is a “do-ing” word, for example, Closing, LogEntryAdded.

DO use present and past tenses where appropriate in event names. For example: Closing, and Closed.

DO NOT use Before and After prefixes. Instead use past and present tense.
For example Microsoft throughout the framework use events names as follows:
Connecting, Connected, Clicked, Sent, Received, Changed, Changing... etc.

DO name event handlers (delegates used as types for events) with the EventHandler suffix.

public delegate void LogEntryEventHandler(object sender, LogEntryEventArgs e);

DO always use two parameters for events: sender of type object (or a generic template type) and a derivative of EventArgs.  “sender” must always be declared as object, and “e” can be any EventArgs class that derives from EventArgs.  This follows the paradigm set out by Microsoft for all events. All throughout the .Net framework they follow this paradigm religiously. This makes the event very familiar and easy to consume for all use cases and developer skill-levels.

public event EventHandler<LogEntryEventArgs> LogEntryAdded;
public void Initialise()
{
    LogEntryAdded += this.OnLogEntryAdded;
}
private void OnLogEntryAdded(object sender, LogEntryEventArgs e)
{
    // Do work
}

CONSIDER naming event handler methods prefixed with the word “On”. See above example.

DO NOT prefix your events with the “On” prefix, “On” should be used only on event handlers not the event name. See the above example.

CONSIDER naming the method that raises the event prefixed with the word “Raise”. For example:

public event EventHandler<PropertyChangedEventArgs> NotifyPropertyChanged;
private void RaiseNotifyPropertyChanged(string name)
{
    var handler = this.NotifyPropertyChanged;
    if (handler != null)
    {
        handler(this, new PropertyChangedEventArgs(name));
    }
}

Field Names

DO use pascal casing for public, internal and protected fields. Because these are public they must use Pascal casing, to give a consistent external interface.

public string PublicField1;
private string privateField2;

DO use camel casing for private fields.

private string field1;

DO name fields with a noun, noun phrase or adjective. Nouns are naming words, for example, Fred, Knife, Vehicle, Book, Map etc...

DO NOT prefix fields. For example do not use “m_” or “g_” or “s_” or any other prefix.

DO prefix field usages with this. Aderant have used this standard for some time with excellent results. (In fact they enforce this usage where ever this can be used). For example:

private string field1;
public void Method1()
{
    if (string.IsNullOrEmpty(this.field1))
    {
        this.field1 = “Apple”;
    }
}

Parameter Names

DO use camel casing.

public void Method1(string name, int age);

DO use descriptive names. Prefer readability over brevity.

CONSIDER using a parameter’s meaning as a driver for its name not its type.

public enum FoodCategory
{
    Vegetable,
    Fruit,
    Meat
}
public class SaladRecipe
{
    private FoodCategory foodCategory;  // less preferred
    private FoodCategory ingredientType;  // better
}

Resource Names

DO use Pascal casing for resource keys.

DO provide descriptiveness over short identifiers.

Namespace Names

AVOID deep namespace hierarchies. Three is considered good, four is acceptable, and five may indicate a rethink is required.

AVOID having too many namespaces.

DO have at least 5 types in a namespace, unless there is strong belief there will be growth in this namespace in future releases.

AVOID having advanced types in the same namespace as types intended to be used in simple use cases.
For example:

System.Collections;                // Basic collections
System.Collections.Specialized;    // Specialised advanced collections
System.Collections.Generic;        // Generic collections

DO use a .Design namespace if a type is only intended to be used at design time. Ie by developers only, code generators, for example.

DO Always declare types enclosed in a namespace specifier.

CONSIDER use the .Interop namespace for interop with non-.Net code.

DO place using statements inside the namespace. This better encapsulates the namespace’s dependent using statements and keeps them together.

Generic Template Names

DO prefix generic template placeholder names with T.

public class Dictionary<TKey, TValue> { ... }

DO use T as a generic template name if there is only one type parameter.

public class List<T> { ... }

CONSIDER restricting the scope of your generic template type to be explicit as possible while still complying with known usage scenarios.

public class List<T> where T : new, class, ICloneable, IEquatable, IComparable { ... }

Constant Names and Static Readonly Fields

DO use Pascal casing for constant names. This applies to public, internal, protected, and private constants.

DO use Pascal casing for static readonly fields. This applies to public internal, protected, and private static readonly fields.

DO NOT prefix constant names.

DO NOT use all uppercase constant names.

CONSIDER using constants over static readonly fields if possible.

DO use static readonly fields when initialisation is required. Constructors are allowed to set readonly fields.  The only difference between a constant and a static readonly is statics are allowed to be initialised in the constructor with any code, where as constants can only be set in their declaration.

Style Conventions
General

CONSIDER using C# aliases instead of Framework types. For example prefer the use of string over String.

CONSIDER preceding a comment line with a new line for clarity.

DoSomething():

// A comment explaining a new section of code
if (someExpression)
{
    Foo = Bar();
}

Tabs And Spaces

DO use 4 spaces only for a tab.

DO NOT use actual tab characters. Pressing the tab key should insert 4 spaces.

DO indent contents of code blocks.

if (someExpression)
{
    DoSomething();
}

DO insert a space following a flow control statement keyword. This includes if, for, foreach, do, while, and switch .

if (someExpression)
{
    DoSomething();
}

DO place one space after the opening and before closing braces when placed on a single line.

IList<int> list = new List<int> { 1, 3, 5, 7, 11, 13 };

DO have a space after a comma.

IList<int> list = new List<int> { 1, 3, 5, 7, 11, 13 };

DO NOT have a space in a method call preceding the parenthesis.

DoSomething();

AVOID using a space inside a method call’s parenthesis.

DoSomething(1, 2, 3); // Preferred

DoSomething( 1, 2, 3 ); // Acceptable

Brace Format

DO Use next line brace format. For example:

int MyFunction(string something)
{
...
}

DO always use braces on all constructs that support them. This includes: if, for, foreach, do, while, using, try, and switch. Omitting the braces for single line statements can lead to careless mistakes when a new line of code is added and intended to be included in the inner scope. Better to be explicit.

Brace Examples:

namespace N

{

    internal interface I

    {

        void foo();

    }

    internal class C

    {

    }

}

public class C

{

    private void Method()

    {

    }

    private abstract int prop

    {

        get;

    }

D d = delegate()

      {

          int x = 0;

          return x;

      }

switch (expression)
{

    case 0:

    {

        this.foo();

        break;

    }

}

int[] array = new int[]

{

    1, 2, 3

}

if (condition)

{

    foo();

}
else

{

    foo();

}

public class C

{

    private void EmptyMethod() {}

}

do

{

    foo();

} while (condition);

try

{

    Foo();

}
catch (Exception e)

{

    Foo();

}
finally { }

DO place else, catch, finally statements on a new line.

try
{
    Foo();
}
catch (System.IO.IOException)
{
    // some handling...
}

Single Line Flow Statements

DO NOT use single line flow statements (if, do, for, while switch etc).  Omitting the braces for single line statements can lead to careless mistakes when a new line of code is added and intended to be included in the inner scope. Better to be explicit.

DO always add braces to flow statements.

DO wrap long lines.  A long line is a line longer than 150 characters.  (Resharper can be configured to wrap automatically based on a user defined right margin).

Xml Documentation

DO Always switch on “Generate XML documentation” project parameter.

DO Always use meaningful xml documentation on public and protected members and types.

CONSIDER using Xml comments on internals also.

CONSIDER documenting private fields and members if at first glance their purpose may not be understandable.

This Prefix

DO prefix all field access with the “this.” Prefix. This does not include properties and methods.

CONSIDER prefixing all members with this where this can be used.  This includes non-static properties and non-static methods.

Member Type	Example
Field	private | protected | internal int timeout; ... this.timeout = 100;
Property	public int Timeout { get; set; } ... Timeout = 100;
Method	public int Add(int operand1, int operand2) { ... } ... Int total = Add(1, 2);
Argument / Parameter	private double pi = 3.14; public int Circumference(double radius) {     return radius * 2 * this.pi; }

This provides two benefits, you can see immediately tell the difference between local variables (not prefixed with “this”) and fields (are prefixed with “this”).  The second benefit is you can clearly see the difference between local member access and static member access.  For example:

Client.Save(user);

“Client” could be a type with a static method “Save” or it could be an instance property which has a “Save” method. If prefixed with “this” it can only be an instance call (access to static members using the “this” keyword is not allowed with standard Microsoft Code Analysis rules applied to your project).

this.Client.Save(user);

DO Use the type identifier on static members. This rule is generally enforced using standard Microsoft Code Analysis rules applied to the project. 

Var Usage

CONSIDER using var when types are duplicated. Avoid using if it decreases readability. For example:

Client acmeClient = new Client();

consider using var instead:

var acmeClient = new Client();

AVOID using var when the type is not clear and could lead to misunderstanding. For example:

var result = Foo(x); // Not clear return type.

File Conventions

DO place one public type per file. Each class, enumeration, or structure should be in its own file for clarity to ensure it is easy to find. The exception to this rule is that you are declaring a nested type.

AVOID placing more than one internal type per file for the same reason as above.

DO name the file name the same name as the class name. For example the String class would be String.cs and the List<T> class would be List.cs or ListT.cs.

CONSIDER grouping members into the following sections and order: (the Regionerate tool can do this automatically)

·         Public Static Read-only Fields

·         Constants

·         All Fields

·         All Constructors

·         Events / Delegates

·         Public and protected properties

·         Methods

·         Explicit interface implementations

·         Internal members

·         Private members

·         Nested types.

CONSIDER sorting members within these groups into alphabetical order.

DO place using statements inside the namespace. This better encapsulates the namespace’s dependent using statements and keeps them together if the namespace is moved or copied.

namespace YourCompany.Foo
{

    using System;

    using System.IO;

}

CONSIDER placing overloads in order of fewest arguments to most arguments.

LINQ Conventions

DO follow previously stated line length guidelines when writing lengthy LINQ statements:

double result = this.list.Where(element => element.StartsWith(“abc”))
        .Join(this.list2, (element1, element2) => element1.Id == element2.Id, (e1, e2) => e1)
        .Sum(element.Value);

DO indent one tab when using multiline Linq statements.

AVOID using “var” for the result type of a LINQ expression.  This can lead to unexpected results when changing the query expression and not considering the adjacent code.

IList<int> list = new List<int> {1, 2, 3};
var result = list.Where(element => element.Value > 1); //unclear return type

This can also lead to misunderstanding when using some Linq providers (such as Linq-SQL, Linq-Entities, Linq-XML) that do not return IEnumerable, they return IQueryable.

CONSIDER start each Linq aggregate and expression on a new line to improve readability even for short lines.

double result = this.list.Where(element => element == “1”)
         .Select(element => element.Length)
        .ToList();

Lambda Conventions

DO ensure your lambda expressions are tidy and readable.

AVOID using multiple lambdas in a single method call.
Less Readable:

int result = Foo(x =>
    {
        int a = x.Something();
        return a > x.Value && x.Value > 1;
    },
    y =>
    {
        return y.Value * this.field;
    });

More Readable:

Action<Class1> filter = x =>
{
    int a = x.Something();
    return a > x.Value && x.Value > 1;
};

Action<Class2> selector = y =>
{
    return y.Value * this.field;
};

Int result = Foo(filter, selector);

DO place the opening brace on a new line for multiple line lambdas.

AVOID using anonymous lambdas for event handlers when the event being subscribed to is not an event on the current class. This can lead to exceptions when the event owner attempts to broadcast an event notification to a lambda expression that is no longer valid.  Rather use a named member field or a method reference.

public class Class1
{
    public Class1()
    {
        // Potentially bad:
        GetOtherClass().SomeEventOccured += (s, e) => this.DoSomething();
        // Event subscription to external class never gets deregistered.

        // Better:
        GetOtherClass().SomeEventOccured += this.DoSomething;
        // And ensure event is deregistered at some point
        GetOtherClass().SomeEventOccured -= this.DoSomething;
    }
}

Internal events are Safe to use lambdas:

public class Class1
{
    public event EventHandler Loaded;
    public Class1()
    {
        this.Loaded += (s, e) => Initialise();
    }
}

Reasons to use ‘this’ keyword over ‘m_’

There is a great deal of logic behind the recommendation to use it.  

The overall goal in any style convention is to consistently add value to code by narrowing the number of ways a piece of code can be interpreted. However the code should always be as readable as possible. The more readable it is, the less chance for bugs through misunderstanding.

Real English words are more readable than acronym or symbolic prefixes.  Using provided language constructs makes more sense than inventing a new one.

Using the ‘this’ keyword on properties and methods is not a mandate of this document, but here's an example of how the 'this' keyword can make code more readable, indicate scope, and indicate member or static access:

// Its clear m_Something is a field,  // but what is Something2? // Something2 could be property, or a  // method reference, or a static member.	// Its clear something is a field  //- "this" means instance; lowercase  // first letter 's' means private field. // Something2 is clearly a static member  // or constant by the Class name prefix  // and the captial letter also indicates // constant or readonly static.
m_Something = Something2;	this.something = ClearerUsage.Something2;
// What is Something.Bar? Static class  // with a static property or member // property access? // Something4 same questions as above.	// Something can only be a member property, it  // cannot be a field due to casing of  // the 'S'. And Bar is a member access of  // Something. It cannot be a static of  // Something otherwise it would be Foo.Bar  // (Foo being the type of Something). // Something4 same as above Something2,3,5.
Something.Bar = Something4;	this.Something.Bar = ClearerUsage.Something4;
// Could be a instance or static method.	// This can only be a member method.
SomethingElse();	this.SomethingElse();

The ‘this’ prefix doesn’t pollute the name of a public field as a prefix does.  If there is a need to use public fields including an “m_” will look ugly in a public API.

This is based on recommendations from the Guideline book and from Industry Practice. Logically 'this' definitely adds value and is much clearer than using Hungarian style prefixes. ‘This’ also indicates scope as well as static versus member access whereas prefixes do not. I know the code will compile without it, but increasing readability of the code and reducing ambiguity DOES reduce bugs. Using a built in language construct makes far more sense than inventing a prefixing or suffixing scheme.

Software Code Quality Metrics

There are a few quality metrics easily available for code. I recommend preventative measures rather than retrospectively inspecting code after its checked in. Although a peer-review process is a necessary step I believe, but should not be used to thumb-suck code quality ratings.

Preventative Measures:

• Unit Testing provides a measure of quality assurance to help prevent new code breaking existing code. Builds can be configured to report back (if not reject a check-in) if tests are broken, indicating poor quality.
• TFS 2010 Gated Check-ins (this is a feature probably already in, if not coming soon to other products). This prevents non-building code from being checked in.
• StyleCop can be configured to run during a build and report (if not reject) a check in if style guidelines are not adhered to.
• Code Analysis Warnings should be turned on to an appropriate level showing poor quality and possibly rejecting a check-in.
• Regionerate can be configured to run pre-build to format and sort you code appropriately, and if it breaks the code in doing so, obviously this will break the build.

Post-Check-in Quality Metrics:

• Lines of Code (LOC) - obtained using Visual Studio's (Premium and above) Code Metrics window.  This can be used to see how much code you have, sometimes useful to know to gauge the size of a project versus a proposed new one.  This is not a good measure of developer performance because one line of code is not really equal to another in complexity and difficulty. Although not hard and fast it can be indicative of developer performance versus past record statistics for the same developer.  
• Cyclomatic Complexity - obtained as above. This is a measure of the number of permutations of code paths through a method. This should never exceed 20 for one method within a class. This indicates code that is hard to follow and hard to maintain. It is never impossible to break a method into more smaller simpler strongly named methods.
• Class Coupling - obtained as above. This shows how many class you are referencing from within a class or method. For a class this should never be above 70, ideally 50 or below.  A high number indicates a possible weak point, if something goes wrong in this class it will have big ramifications.  It will also be difficult to maintain with so many dependencies demanding slightly different things of the same class.
• Code Warnings (both compile warnings and Analysis warnings) - obtained by enabling code analysis in project properties.  A high number of warnings means poor quality no matter which way you look at it.
• Missing or Empty Xml Documentation Files - obtained by enabling Xml documentation in project properties.  The produced xml file will need to be examined by some tool, basically you look for empty or short (or possibly template standard text) in text elements.
• Code Coverage - obtained using Code Coverage for MsTest or using a tool like TestDriven.Net.  All code should be 60% or better code coverage.

References:
MSDN Design Guidelines for Developing Class Libraries
SSW Rules to Better Code
SSW Rule to Better .NET Projects
NDepend.com
iDesign (Juval Lowry) C# Guidelines
iDesign (Juval Lowry) WCF Guidelines