logo


Coding Guidelines

for C# 5.0, 6.0 and 7.0

Dennis Doomen
Version 5.0.0
18 October 2016

1. Introduction

1.1. What is this?

This document attempts to provide guidelines (or coding standards if you like) for coding in C# 5.0, 6.0 or 7.0 that are both useful and pragmatic. Of course, if you create such a document you should practice what you preach. So rest assured, these guidelines are representative to what we at Aviva Solutions do in our day-to-day work. Notice that not all guidelines have a clear rationale. Some of them are simply choices we made at Aviva Solutions. In the end, it doesn’t matter what choice you made, as long as you make one and apply it consistently.

Visual Studio’s Static Code Analysis (which is also known as FxCop) and StyleCop can already automatically enforce a lot of coding and design rules by analyzing the compiled assemblies. You can configure it to do that at compile time or as part of a continuous or daily build. The companion site www.csharpcodingguidelines.com provides a list of code analysis rules depending on the type of code base you’re dealing with. This document just provides an additional set of rules and recommendations that should help you achieve a more maintainable code base.

1.2. Why would you use this document?

Although some might see coding guidelines as undesired overhead or something that limits creativity, this approach has already proven its value for many years. This is because not every developer:

1.3. Basic principles

There are many unexpected things I run into during my work as a consultant, each deserving at least one guideline. Unfortunately, I still need to keep this document within a reasonable size. But unlike what some junior developers believe, that doesn’t mean that something is okay just because it is not mentioned in this document.

In general, if I have a discussion with a colleague about a smell that this document does not cover, I’ll refer back to a set of basic principles that apply to all situations, regardless of context. These include:

Regardless of the elegance of someone’s solution, if it’s too complex for the ordinary developer, exposes unusual behavior, or tries to solve many possible future issues, it is very likely the wrong solution and needs redesign. The worst response a developer can give you to these principles is: “But it works?”.

1.4. How do you get started?

1.5. Why did we create it?

The idea started in 2002 when Vic Hartog (Philips Medical Systems) and I were assigned the task of writing up a coding standard for C# 1.0. Since then, I’ve regularly added, removed and changed rules based on experiences, feedback from the community and new tooling support offered by a continuous stream of new Visual Studio releases.

Additionally, after reading Robert C. Martin’s book Clean Code: A Handbook of Agile Software Craftsmanship, I became a big fan of his ideas and decided to include some of his smells and heuristics as guidelines. Notice though, that this document is in no way a replacement for his book. I sincerely recommend that you read his book to gain a solid understanding of the rationale behind his recommendations.

I’ve also decided to include some design guidelines in addition to simple coding guidelines. They are too important to ignore and have a big influence in reaching high quality code.

1.6. Is this a coding standard?

The document does not state that projects must comply with these guidelines, neither does it say which guidelines are more important than others. However, we encourage projects to decide themselves which guidelines are important, what deviations a project will use, who is the consultant in case doubts arise, and what kind of layout must be used for source code. Obviously, you should make these decisions before starting the real coding work.

To help you in this decision, I’ve assigned a level of importance to each guideline:

Guidelines that you should never skip and should be applicable to all situations

Strongly recommended guidelines

May not be applicable in all situations

1.7. Feedback and disclaimer

This document has been compiled using many contributions from community members, blog posts, on-line discussions and many years of developing in C#. If you have questions, comments or suggestions, just let me know by sending me an email at dennis.doomen@avivasolutions.nl, creating an issue or Pull Request on GitHub, or ping me at http://twitter.com/ddoomen. I will try to revise and republish this document with new insights, experiences and remarks on a regular basis.

Notice though that it merely reflects my view on proper C# code so Aviva Solutions will not be liable for any direct or indirect damages caused by applying the guidelines of this document. This document is published under a Creative Commons license, specifically the Creative Commons Attribution-ShareAlike 4.0 license.

2. Class Design Guidelines

A class or interface should have a single purpose (AV1000)

A class or interface should have a single purpose within the system it functions in. In general, a class either represents a primitive type like an email or ISBN number, an abstraction of some business concept, a plain data structure, or is responsible for orchestrating the interaction between other classes. It is never a combination of those. This rule is widely known as the Single Responsibility Principle, one of the S.O.L.I.D. principles.

Tip: A class with the word And in it is an obvious violation of this rule.

Tip: Use Design Patterns to communicate the intent of a class. If you can’t assign a single design pattern to a class, chances are that it is doing more than one thing.

Note If you create a class representing a primitive type you can greatly simplify its use by making it immutable.

Only create a constructor that returns a useful object (AV1001)

There should be no need to set additional properties before the object can be used for whatever purpose it was designed. However, if your constructor needs more than three parameters (which violates AV1561), your class might have too much responsibility (and violates AV1000).

An interface should be small and focused (AV1003)

Interfaces should have a name that clearly explains their purpose or role in the system. Do not combine many vaguely related members on the same interface just because they were all on the same class. Separate the members based on the responsibility of those members, so that callers only need to call or implement the interface related to a particular task. This rule is more commonly known as the Interface Segregation Principle.

Use an interface rather than a base class to support multiple implementations (AV1004)

If you want to expose an extension point from your class, expose it as an interface rather than as a base class. You don’t want to force users of that extension point to derive their implementations from a base class that might have an undesired behavior. However, for their convenience you may implement a(n abstract) default implementation that can serve as a starting point.

Use an interface to decouple classes from each other (AV1005)

Interfaces are a very effective mechanism for decoupling classes from each other:

Avoid static classes (AV1008)

With the exception of extension method containers, static classes very often lead to badly designed code. They are also very difficult, if not impossible, to test in isolation, unless you’re willing to use some very hacky tools.

Note: If you really need that static class, mark it as static so that the compiler can prevent instance members and instantiating your class. This relieves you of creating an explicit private constructor.

Don’t suppress compiler warnings using the new keyword (AV1010)

Compiler warning CS0114 is issued when breaking Polymorphism, one of the most essential object-orientation principles. The warning goes away when you add the new keyword, but it keeps sub-classes difficult to understand. Consider the following two classes:

public class Book  
{
	public virtual void Print()  
	{
		Console.WriteLine("Printing Book");
	}  
}

public class PocketBook : Book  
{
	public new void Print()
	{
		Console.WriteLine("Printing PocketBook");
	}  
}

This will cause behavior that you would not normally expect from class hierarchies:

PocketBook pocketBook = new PocketBook();

pocketBook.Print(); // Outputs "Printing PocketBook "

((Book)pocketBook).Print(); // Outputs "Printing Book"

It should not make a difference whether you call Print() through a reference to the base class or through the derived class.

It should be possible to treat a derived object as if it were a base class object (AV1011)

In other words, you should be able to use a reference to an object of a derived class wherever a reference to its base class object is used without knowing the specific derived class. A very notorious example of a violation of this rule is throwing a NotImplementedException when overriding some of the base-class methods. A less subtle example is not honoring the behavior expected by the base class.

Note: This rule is also known as the Liskov Substitution Principle, one of the S.O.L.I.D. principles.

Don’t refer to derived classes from the base class (AV1013)

Having dependencies from a base class to its sub-classes goes against proper object-oriented design and might prevent other developers from adding new derived classes.

Avoid exposing the other objects an object depends on (AV1014)

If you find yourself writing code like this then you might be violating the Law of Demeter.

someObject.SomeProperty.GetChild().Foo()

An object should not expose any other classes it depends on because callers may misuse that exposed property or method to access the object behind it. By doing so, you allow calling code to become coupled to the class you are using, and thereby limiting the chance that you can easily replace it in a future stage.

Note: Using a class that is designed using the Fluent Interface pattern seems to violate this rule, but it is simply returning itself so that method chaining is allowed.

Exception: Inversion of Control or Dependency Injection frameworks often require you to expose a dependency as a public property. As long as this property is not used for anything other than dependency injection I would not consider it a violation.

Avoid bidirectional dependencies (AV1020)

This means that two classes know about each other’s public members or rely on each other’s internal behavior. Refactoring or replacing one of those classes requires changes on both parties and may involve a lot of unexpected work. The most obvious way of breaking that dependency is to introduce an interface for one of the classes and using Dependency Injection.

Exception: Domain models such as defined in Domain-Driven Design tend to occasionally involve bidirectional associations that model real-life associations. In those cases, make sure they are really necessary, and if they are, keep them in.

Classes should have state and behavior (AV1025)

In general, if you find a lot of data-only classes in your code base, you probably also have a few (static) classes with a lot of behavior (see AV1008). Use the principles of object-orientation explained in this section and move the logic close to the data it applies to.

Exception: The only exceptions to this rule are classes that are used to transfer data over a communication channel, also called Data Transfer Objects, or a class that wraps several parameters of a method.

Classes should protect the consistency of their internal state (AV1026)

Validate incoming arguments from public members. For example:

public void SetAge(int years)
{
	AssertValueIsInRange(years, 0, 200, nameof(years));
	
	this.age = years;
}

Protect invariants on internal state. For example:

public void Render()
{
	AssertNotDisposed();
	
	// ...
}

3. Member Design Guidelines

Allow properties to be set in any order (AV1100)

Properties should be stateless with respect to other properties, i.e. there should not be a difference between first setting property DataSource and then DataMember or vice-versa.

Use a method instead of a property (AV1105)

Exception: Populating an internal cache or implementing lazy-loading is a good exception.

Don’t use mutual exclusive properties (AV1110)

Having properties that cannot be used at the same time typically signals a type that represents two conflicting concepts. Even though those concepts may share some of their behavior and states, they obviously have different rules that do not cooperate.

This violation is often seen in domain models and introduces all kinds of conditional logic related to those conflicting rules, causing a ripple effect that significantly increases the maintenance burden.

A method or property should do only one thing (AV1115)

Similarly to rule AV1000, a method should have a single responsibility.

Don’t expose stateful objects through static members (AV1125)

A stateful object is an object that contains many properties and lots of behavior behind it. If you expose such an object through a static property or method of some other object, it will be very difficult to refactor or unit test a class that relies on such a stateful object. In general, introducing a construction like that is a great example of violating many of the guidelines of this chapter.

A classic example of this is the HttpContext.Current property, part of ASP.NET. Many see the HttpContext class as a source of a lot of ugly code. In fact, the testing guideline Isolate the Ugly Stuff often refers to this class.

Return an IEnumerable<T> or ICollection<T> instead of a concrete collection class (AV1130)

You generally don’t want callers to be able to change an internal collection, so don’t return arrays, lists or other collection classes directly. Instead, return an IEnumerable<T>, or, if the caller must be able to determine the count, an ICollection<T>.

Note: If you’re using .NET 4.5, you can also use IReadOnlyCollection<T>, IReadOnlyList<T> or IReadOnlyDictionary<TKey, TValue>.

Properties, methods and arguments representing strings or collections should never be null (AV1135)

Returning null can be unexpected by the caller. Always return an empty collection or an empty string instead of a null reference. This also prevents cluttering your code base with additional checks for null, or even worse, string.IsNullOrEmpty().

Define parameters as specific as possible (AV1137)

If your member needs a specific piece of data, define parameters as specific as that and don’t take a container object instead. For instance, consider a method that needs a connection string that is exposed through a central IConfiguration interface. Rather than taking a dependency on the entire configuration, just define a parameter for the connection string. This not only prevents unnecessary coupling, it also improves maintainability in the long run.

Note: An easy trick to remember this guideline is the Don’t ship the truck if you only need a package.

Consider using domain-specific value types rather than primitives (AV1140)

Instead of using strings, integers and decimals for representing domain-specific types such as an ISBN number, an email address or amount of money, consider creating dedicated value objects that wrap both the data and the validation rules that apply to it. By doing this, you prevent ending up having multiple implementations of the same business rules, which both improves maintainability and prevents bugs.

4. Miscellaneous Design Guidelines

Throw exceptions rather than returning some kind of status value (AV1200)

A code base that uses return values to report success or failure tends to have nested if-statements sprinkled all over the code. Quite often, a caller forgets to check the return value anyway. Structured exception handling has been introduced to allow you to throw exceptions and catch or replace them at a higher layer. In most systems it is quite common to throw exceptions whenever an unexpected situation occurs.

Provide a rich and meaningful exception message text (AV1202)

The message should explain the cause of the exception, and clearly describe what needs to be done to avoid the exception.

Throw the most specific exception that is appropriate (AV1205)

For example, if a method receives a null argument, it should throw ArgumentNullException instead of its base type ArgumentException.

Don’t swallow errors by catching generic exceptions (AV1210)

Avoid swallowing errors by catching non-specific exceptions, such as Exception, SystemException, and so on, in application code. Only top-level code, such as a last-chance exception handler, should catch a non-specific exception for logging purposes and a graceful shutdown of the application.

Properly handle exceptions in asynchronous code (AV1215)

When throwing or handling exceptions in code that uses async/await or a Task remember the following two rules:

Always check an event handler delegate for null (AV1220)

An event that has no subscribers is null. So before invoking, always make sure that the delegate list represented by the event variable is not null. Invoke using the null conditional operator, because it additionally prevents conflicting changes to the delegate list from concurrent threads.

event EventHandler Notify;

protected virtual void OnNotify(NotifyEventArgs args)  
{
	Notify?.Invoke(this, args);
}

Use a protected virtual method to raise each event (AV1225)

Complying with this guideline allows derived classes to handle a base class event by overriding the protected method. The name of the protected virtual method should be the same as the event name prefixed with On. For example, the protected virtual method for an event named TimeChanged is named OnTimeChanged.

Note: Derived classes that override the protected virtual method are not required to call the base class implementation. The base class must continue to work correctly even if its implementation is not called.

Consider providing property-changed events (AV1230)

Consider providing events that are raised when certain properties are changed. Such an event should be named PropertyChanged, where Property should be replaced with the name of the property with which this event is associated

Note: If your class has many properties that require corresponding events, consider implementing the INotifyPropertyChanged interface instead. It is often used in the Presentation Model and Model-View-ViewModel patterns.

Don’t pass null as the sender argument when raising an event (AV1235)

Often an event handler is used to handle similar events from multiple senders. The sender argument is then used to get to the source of the event. Always pass a reference to the source (typically this) when raising the event. Furthermore don’t pass null as the event data parameter when raising an event. If there is no event data, pass EventArgs.Empty instead of null.

Exception: On static events, the sender argument should be null.

Use generic constraints if applicable (AV1240)

Instead of casting to and from the object type in generic types or methods, use where constraints or the as operator to specify the exact characteristics of the generic parameter. For example:

class SomeClass  
{}

// Don't  
class MyClass  
{
	void SomeMethod(T t)  
	{  
		object temp = t;  
		SomeClass obj = (SomeClass) temp;  
	}  
}

// Do  
class MyClass where T : SomeClass  
{
	void SomeMethod(T t)  
	{  
		SomeClass obj = t;  
	}  
}

Evaluate the result of a LINQ expression before returning it (AV1250)

Consider the following code snippet

public IEnumerable GetGoldMemberCustomers()
{
	const decimal GoldMemberThresholdInEuro = 1_000_000;
	
	var query = 
		from customer in db.Customers
		where customer.Balance > GoldMemberThresholdInEuro
		select new GoldMember(customer.Name, customer.Balance);
	
	return query;  
}

Since LINQ queries use deferred execution, returning query will actually return the expression tree representing the above query. Each time the caller evaluates this result using a foreach cycle or similar, the entire query is re-executed resulting in new instances of GoldMember every time. Consequently, you cannot use the == operator to compare multiple GoldMember instances. Instead, always explicitly evaluate the result of a LINQ query using ToList(), ToArray() or similar methods.

Do not use this and base prefixes unless it is required (AV1251)

In a class hierarchy, it is not necessary to know at which level a member is declared to use it. Refactoring derived classes is harder if that level is fixed in the code.

5. Maintainability Guidelines

Methods should not exceed 7 statements (AV1500)

A method that requires more than 7 statements is simply doing too much or has too many responsibilities. It also requires the human mind to analyze the exact statements to understand what the code is doing. Break it down into multiple small and focused methods with self-explaining names, but make sure the high-level algorithm is still clear.

Make all members private and types internal sealed by default (AV1501)

To make a more conscious decision on which members to make available to other classes, first restrict the scope as much as possible. Then carefully decide what to expose as a public member or type.

Avoid conditions with double negatives (AV1502)

Although a property like customer.HasNoOrders makes sense, avoid using it in a negative condition like this:

bool hasOrders = !customer.HasNoOrders;

Double negatives are more difficult to grasp than simple expressions, and people tend to read over the double negative easily.

Name assemblies after their contained namespace (AV1505)

All DLLs should be named according to the pattern Company.Component.dll where Company refers to your company’s name and Component contains one or more dot-separated clauses. For example AvivaSolutions.Web.Controls.dll.

As an example, consider a group of classes organized under the namespace AvivaSolutions.Web.Binding exposed by a certain assembly. According to this guideline, that assembly should be called AvivaSolutions.Web.Binding.dll.

Exception: If you decide to combine classes from multiple unrelated namespaces into one assembly, consider suffixing the assembly name with Core, but do not use that suffix in the namespaces. For instance, AvivaSolutions.Consulting.Core.dll.

Name a source file to the type it contains (AV1506)

Use Pascal casing to name the file and don’t use underscores. Don’t include (the number of) generic type parameters in the file name.

Limit the contents of a source code file to one type (AV1507)

Exception: Nested types should be part of the same file.

Exception: Types that only differ by their number of generic type parameters should be part of the same file.

Name a source file to the logical function of the partial type (AV1508)

When using partial types and allocating a part per file, name each file after the logical part that part plays. For example:

// In MyClass.cs
public partial class MyClass
{...}

// In MyClass.Designer.cs	
public partial class MyClass
{...}

Use using statements instead of fully qualified type names (AV1510)

Limit usage of fully qualified type names to prevent name clashing. For example, don’t do this:

var list = new System.Collections.Generic.List<string>();

Instead, do this:

using System.Collections.Generic;

var list = new List<string>();

If you do need to prevent name clashing, use a using directive to assign an alias:

using Label = System.Web.UI.WebControls.Label;

Don’t use “magic” numbers (AV1515)

Don’t use literal values, either numeric or strings, in your code, other than to define symbolic constants. For example:

public class Whatever  
{
	public static readonly Color PapayaWhip = new Color(0xFFEFD5);
	public const int MaxNumberOfWheels = 18;
	public const byte ReadCreateOverwriteMask = 0b0010_1100;
}

Strings intended for logging or tracing are exempt from this rule. Literals are allowed when their meaning is clear from the context, and not subject to future changes, For example:

mean = (a + b) / 2; // okay  
WaitMilliseconds(waitTimeInSeconds * 1000); // clear enough

If the value of one constant depends on the value of another, attempt to make this explicit in the code.

public class SomeSpecialContainer  
{  
	public const int MaxItems = 32;  
	public const int HighWaterMark = 3 * MaxItems / 4; // at 75%  
}

Note: An enumeration can often be used for certain types of symbolic constants.

Only use var when the type is very obvious (AV1520)

Only use var as the result of a LINQ query, or if the type is very obvious from the same statement and using it would improve readability. So don’t

var item = 3;                              // what type? int? uint? float?
var myfoo = MyFactoryMethod.Create("arg"); // Not obvious what base-class or			
                                           // interface to expect. Also
                                           // difficult to refactor if you can't
                                           // search for the class

Instead, use var like this:

var query = from order in orders where order.Items > 10 and order.TotalValue > 1000;
var repository = new RepositoryFactory.Get();	
var list = new ReadOnlyCollection();

In all of three above examples it is clear what type to expect. For a more detailed rationale about the advantages and disadvantages of using var, read Eric Lippert’s Uses and misuses of implicit typing.

Declare and initialize variables as late as possible (AV1521)

Avoid the C and Visual Basic styles where all variables have to be defined at the beginning of a block, but rather define and initialize each variable at the point where it is needed.

Assign each variable in a separate statement (AV1522)

Don’t use confusing constructs like the one below:

var result = someField = GetSomeMethod();

Exception: Multiple assignments per statement are allowed by using out variables, is-patterns or deconstruction into tuples. Examples:

bool success = int.TryParse(text, out int result);

if ((items[0] is string text) || (items[1] is Action action))
{
}

(int a, int b) = M();

Favor Object and Collection Initializers over separate statements (AV1523)

Instead of:

var startInfo = new ProcessStartInfo("myapp.exe");	
startInfo.StandardOutput = Console.Output;
startInfo.UseShellExecute = true;

var countries = new List();
countries.Add("Netherlands");
countries.Add("United States");

var countryLookupTable = new Dictionary<string, string>();
countryLookupTable.Add("NL", "Netherlands");
countryLookupTable.Add("US", "United States");

Use Object and Collection Initializers:

var startInfo = new ProcessStartInfo("myapp.exe")  
{
	StandardOutput = Console.Output,
	UseShellExecute = true  
};

var countries = new List { "Netherlands", "United States" };

var countryLookupTable = new Dictionary<string, string>
{
	["NL"] = "Netherlands",
	["US"] = "United States"
};

Don’t make explicit comparisons to true or false (AV1525)

It is usually bad style to compare a bool-type expression to true or false. For example:

while (condition == false) // wrong; bad style  
while (condition != true) // also wrong  
while (((condition == true) == true) == true) // where do you stop?  
while (condition) // OK

Don’t change a loop variable inside a for loop (AV1530)

Updating the loop variable within the loop body is generally considered confusing, even more so if the loop variable is modified in more than one place.

for (int index = 0; index < 10; ++index)  
{  
	if (someCondition)
	{
		index = 11; // Wrong! Use 'break' or 'continue' instead.  
	}
}

Avoid nested loops (AV1532)

A method that nests loops is more difficult to understand than one with only a single loop. In fact, in most cases nested loops can be replaced with a much simpler LINQ query that uses the from keyword twice or more to join the data.

Always add a block after the keywords if, else, do, while, for, foreach and case (AV1535)

Please note that this also avoids possible confusion in statements of the form:

if (isActive) if (isVisible) Foo(); else Bar(); // which 'if' goes with the 'else'?

// The right way:  
if (isActive)  
{  
	if (isVisible)  
	{  
		Foo();  
	}  
	else  
	{  
		Bar();  
	}  
}

Always add a default block after the last case in a switch statement (AV1536)

Add a descriptive comment if the default block is supposed to be empty. Moreover, if that block is not supposed to be reached throw an InvalidOperationException to detect future changes that may fall through the existing cases. This ensures better code, because all paths the code can travel have been thought about.

void Foo(string answer)  
{  
	switch (answer)  
	{  
		case "no":  
		{
		  Console.WriteLine("You answered with No");  
		  break;
		}  
		  
		case "yes":
		{  
		  Console.WriteLine("You answered with Yes");  
		  break;
		}
		
		default:  
		{
		  // Not supposed to end up here.  
		  throw new InvalidOperationException("Unexpected answer " + answer);
		}  
	}  
}

Finish every if-else-if statement with an else-part (AV1537)

For example:

void Foo(string answer)  
{  
	if (answer == "no")  
	{  
		Console.WriteLine("You answered with No");  
	}  
	else if (answer == "yes")  
	{  
		Console.WriteLine("You answered with Yes");  
	}  
	else  
	{  
		// What should happen when this point is reached? Ignored? If not,
		// throw an InvalidOperationException.  
	}  
}

Be reluctant with multiple return statements (AV1540)

One entry, one exit is a sound principle and keeps control flow readable. However, if the method is very small and complies with guideline AV1500 then multiple return statements may actually improve readability over some central boolean flag that is updated at various points.

Don’t use if-else statements instead of a simple (conditional) assignment (AV1545)

Express your intentions directly. For example, rather than:

bool isPositive;

if (value > 0)
{
	isPositive = true;
}
else
{
	isPositive = false;
}

write:

bool isPositive = (value > 0);

Or instead of:

string classification;

if (value > 0)
{
	classification = "positive";
}
else
{
	classification = "negative";
}

return classification;

write:

return (value > 0) ? "positive" : "negative";

Or instead of:

int result;

if (offset == null)
{
	result = -1;
}
else
{
	result = offset.Value;
}

return result;

write:

return offset ?? -1;

Or instead of:

if (employee.Manager != null)
{
	return employee.Manager.Name;
}
else
{
	return null;
}

write:

return employee.Manager?.Name;

Encapsulate complex expressions in a method or property (AV1547)

Consider the following example:

if (member.HidesBaseClassMember && (member.NodeType != NodeType.InstanceInitializer))
{
	// do something
}

In order to understand what this expression is about, you need to analyze its exact details and all of its possible outcomes. Obviously, you can add an explanatory comment on top of it, but it is much better to replace this complex expression with a clearly named method:

if (NonConstructorMemberUsesNewKeyword(member))  
{  
	// do something
}  
  
  
private bool NonConstructorMemberUsesNewKeyword(Member member)  
{  
	return
		(member.HidesBaseClassMember &&
		(member.NodeType != NodeType.InstanceInitializer)  
}

You still need to understand the expression if you are modifying it, but the calling code is now much easier to grasp.

Call the more overloaded method from other overloads (AV1551)

This guideline only applies to overloads that are intended to provide optional arguments. Consider, for example, the following code snippet:

public class MyString  
{
	private string someText;
	
	public int IndexOf(string phrase)  
	{  
		return IndexOf(phrase, 0); 
	}
	
	public int IndexOf(string phrase, int startIndex)  
	{  
		return IndexOf(phrase, startIndex, someText.Length - startIndex);
	}
	
	public virtual int IndexOf(string phrase, int startIndex, int count)  
	{  
		return someText.IndexOf(phrase, startIndex, count);
	}  
}

The class MyString provides three overloads for the IndexOf method, but two of them simply call the one with one more parameter. Notice that the same rule applies to class constructors; implement the most complete overload and call that one from the other overloads using the this() operator. Also notice that the parameters with the same name should appear in the same position in all overloads.

Important: If you also want to allow derived classes to override these methods, define the most complete overload as a non-private virtual method that is called by all overloads.

Only use optional arguments to replace overloads (AV1553)

The only valid reason for using C# 4.0’s optional arguments is to replace the example from rule AV1551 with a single method like:

public virtual int IndexOf(string phrase, int startIndex = 0, int count = -1)
{
    int length = (count == -1) ? (someText.Length - startIndex) : count;
    return someText.IndexOf(phrase, startIndex, length);
}

If the optional parameter is a reference type then it can only have a default value of null. But since strings, lists and collections should never be null according to rule AV1135, you must use overloaded methods instead.

Note: The default values of the optional parameters are stored at the caller side. As such, changing the default value without recompiling the calling code will not apply the new default value properly.

Note: When an interface method defines an optional parameter, its default value is not considered during overload resolution unless you call the concrete class through the interface reference. See this post by Eric Lippert for more details.

Avoid using named arguments (AV1555)

C# 4.0’s named arguments have been introduced to make it easier to call COM components that are known for offering many optional parameters. If you need named arguments to improve the readability of the call to a method, that method is probably doing too much and should be refactored.

Exception: The only exception where named arguments improve readability is when calling a method of some code base you don’t control that has a bool parameter, like this:

object[] myAttributes = type.GetCustomAttributes(typeof(MyAttribute), inherit: false);

Don’t allow methods and constructors with more than three parameters (AV1561)

If you create a method with more than three parameters, use a structure or class to pass multiple arguments, as explained in the Specification design pattern. In general, the fewer the parameters, the easier it is to understand the method. Additionally, unit testing a method with many parameters requires many scenarios to test.

Don’t use ref or out parameters (AV1562)

They make code less understandable and might cause people to introduce bugs. Instead, return compound objects or tuples.

Exception: Calling and declaring members that implement the TryParse pattern is allowed. For example:

bool success = int.TryParse(text, out int number);

Avoid methods that take a bool flag (AV1564)

Consider the following method signature:

public Customer CreateCustomer(bool platinumLevel) {}

On first sight this signature seems perfectly fine, but when calling this method you will lose this purpose completely:

Customer customer = CreateCustomer(true);

Often, a method taking such a flag is doing more than one thing and needs to be refactored into two or more methods. An alternative solution is to replace the flag with an enumeration.

Don’t use parameters as temporary variables (AV1568)

Never use a parameter as a convenient variable for storing temporary state. Even though the type of your temporary variable may be the same, the name usually does not reflect the purpose of the temporary variable.

Prefer is patterns over as operations (AV1570)

If you use ‘as’ to safely upcast an interface reference to a certain type, always verify that the operation does not return null. Failure to do so may cause a NullReferenceException at a later stage if the object did not implement that interface. Pattern matching syntax prevents this and improves readability. For example, instead of:

var remoteUser = user as RemoteUser;
if (remoteUser != null)
{
}

write:

if (user is RemoteUser remoteUser)
{
}

Don’t comment out code (AV1575)

Never check in code that is commented out. Instead, use a work item tracking system to keep track of some work to be done. Nobody knows what to do when they encounter a block of commented-out code. Was it temporarily disabled for testing purposes? Was it copied as an example? Should I delete it?

6. Naming Guidelines

Use US-English (AV1701)

All type members, parameters and variables should be named using words from the American English language.

Exception: In most projects, you will use words and phrases from your domain and names specific to your company. Visual Studio’s Static Code Analysis performs a spelling check on all code, so you may need to add those terms to a Custom Code Analysis Dictionary.

Use proper casing for language elements (AV1702)

Language element      |Casing    |Example ——————–|———-|:———– Class, Struct|Pascal|AppDomain Interface | Pascal | IBusinessService Enumeration type | Pascal | ErrorLevel | Enumeration values | Pascal | FatalError | Event | Pascal | Click | Private field | Camel | listItem | Protected field | Pascal | MainPanel | Constant field | Pascal | MaximumItems | Constant Local variable | Camel | maximumItems | Read-only static field | Pascal | RedValue | Local Variable | Camel | listOfValues | Method | Pascal | ToString | Local function | Pascal | FormatText | Namespace | Pascal | System.Drawing | Parameter | Camel | typeName | Type Parameter | Pascal | TView | Property | Pascal | BackColor | Tuple element | Camel | firstName |

Don’t include numbers in variables, parameters and type members (AV1704)

In most cases they are a lazy excuse for not defining a clear and intention-revealing name.

Don’t prefix fields (AV1705)

For example, don’t use g_ or s_ to distinguish static from non-static fields. A method in which it is difficult to distinguish local variables from member fields is generally too big. Examples of incorrect identifier names are: _currentUser, mUserName, m_loginTime.

Don’t use abbreviations (AV1706)

For example, use OnButtonClick rather than OnBtnClick. Avoid single character variable names, such as i or q. Use index or query instead.

Exceptions: Use well-known acronyms and abbreviations that are widely accepted or well-known in your work domain. For instance, use acronym UI instead of UserInterface and abbreviation Id instead of Identity.

Name a member, parameter or variable according to its meaning and not its type (AV1707)

Name types using nouns, noun phrases or adjective phrases (AV1708)

For example, the name IComponent uses a descriptive noun, ICustomAttributeProvider uses a noun phrase and IPersistable uses an adjective. Bad examples include SearchExamination (a page to search for examinations), Common (does not end with a noun, and does not explain its purpose) and SiteSecurity (although the name is technically okay, it does not say anything about its purpose).

Don’t include terms like Utility or Helper in classes. Classes with names like that are usually static classes and are introduced without considering object-oriented principles (see also AV1008).

Name generic type parameters with descriptive names (AV1709)

Don’t repeat the name of a class or enumeration in its members (AV1710)

class Employee
{
	// Wrong!
	static GetEmployee() {}
	DeleteEmployee() {}
	
	// Right
	static Get() {...}
	Delete() {...}
	
	// Also correct.
	AddNewJob() {...}
	RegisterForMeeting() {...}
}

.NET developers are already accustomed to the naming patterns the framework uses, so following this same pattern helps them find their way in your classes as well. For instance, if you define a class that behaves like a collection, provide members like Add, Remove and Count instead of AddItem, Delete or NumberOfItems.

Avoid short names or names that can be mistaken for other names (AV1712)

Although technically correct, statements like the following can be confusing:

bool b001 = (lo == l0) ? (I1 == 11) : (lOl != 101);

Properly name properties (AV1715)

Name methods and local functions using verbs or verb-object pairs (AV1720)

Name a method or local function using a verb like Show or a verb-object pair such as ShowDialog. A good name should give a hint on the what of a member, and if possible, the why.

Also, don’t include And in the name of a method or local function. That implies that it is doing more than one thing, which violates the Single Responsibility Principle explained in AV1115.

Name namespaces using names, layers, verbs and features (AV1725)

For instance, the following namespaces are good examples of that guideline.

AvivaSolutions.Commerce.Web
NHibernate.Extensibility
Microsoft.ServiceModel.WebApi
Microsoft.VisualStudio.Debugging
FluentAssertion.Primitives
CaliburnMicro.Extensions

Note: Never allow namespaces to contain the name of a type, but a noun in its plural form (e.g. Collections) is usually OK.

Use a verb or verb phrase to name an event (AV1735)

Name events with a verb or a verb phrase. For example: Click, Deleted, Closing, Minimizing, and Arriving. For example, the declaration of the Search event may look like this:

public event EventHandler<SearchArgs> Search;

Use -ing and -ed to express pre-events and post-events (AV1737)

For example, a close event that is raised before a window is closed would be called Closing, and one that is raised after the window is closed would be called Closed. Don’t use Before or After prefixes or suffixes to indicate pre and post events.

Suppose you want to define events related to the deletion of an object. Avoid defining the Deleting and Deleted events as BeginDelete and EndDelete. Define those events as follows:

Prefix an event handler with On (AV1738)

It is good practice to prefix the method that handles an event with On. For example, a method that handles the Closing event can be named OnClosing.

Use an underscore for irrelevant lambda parameters (AV1739)

If you use a lambda expression (for instance, to subscribe to an event) and the actual arguments of the event are irrelevant, use discards to make that explicit:

button.Click += (_, _) => HandleClick();

Group extension methods in a class suffixed with Extensions (AV1745)

If the name of an extension method conflicts with another member or extension method, you must prefix the call with the class name. Having them in a dedicated class with the Extensions suffix improves readability.

Post-fix asynchronous methods with Async or TaskAsync (AV1755)

The general convention for methods that return Task or Task<TResult> is to post-fix them with Async, but if such a method already exists, use TaskAsync instead.

7. Performance Guidelines

Consider using Any() to determine whether an IEnumerable<T> is empty (AV1800)

When a method or other member returns an IEnumerable<T> or other collection class that does not expose a Count property, use the Any() extension method rather than Count() to determine whether the collection contains items. If you do use Count(), you risk that iterating over the entire collection might have a significant impact (such as when it really is an IQueryable<T> to a persistent store).

Note: If you return an IEnumerable<T> to prevent editing from outside the owner as explained in AV1130, and you’re developing in .NET 4.5 or higher, consider the new read-only classes.

Only use async for low-intensive long-running activities (AV1820)

The usage of async won’t automagically run something on a worker thread like Task.Run does. It just adds the necessary logic to allow releasing the current thread, and marshal the result back on that same thread if a long-running asynchronous operation has completed. In other words, use async only for I/O bound operations.

Prefer Task.Run for CPU-intensive activities (AV1825)

If you do need to execute a CPU bound operation, use Task.Run to offload the work to a thread from the Thread Pool. Remember that you have to marshal the result back to your main thread manually.

Beware of mixing up await/async with Task.Wait (AV1830)

await does not block the current thread but simply instructs the compiler to generate a state-machine. However, Task.Wait blocks the thread and may even cause deadlocks (see AV1835).

Beware of async/await deadlocks in single-threaded environments (AV1835)

Consider the following asynchronous method:

private async Task GetDataAsync()
{
	var result = await MyWebService.GetDataAsync();
	return result.ToString();
}

Now when an ASP.NET MVC controller action does this:

public ActionResult ActionAsync()
{
	var data = GetDataAsync().Result;
	
	return View(data);  
}

You end up with a deadlock. Why? Because the Result property getter will block until the async operation has completed, but since an async method will automatically marshal the result back to the original thread and ASP.NET uses a single-threaded synchronization context, they’ll be waiting on each other. A similar problem can also happen on WPF, Silverlight or a Windows Store C#/XAML app. Read more about this here.

8. Framework Guidelines

Use C# type aliases instead of the types from the System namespace (AV2201)

For instance, use object instead of Object, string instead of String, and int instead of Int32. These aliases have been introduced to make the primitive types first class citizens of the C# language, so use them accordingly.

Exception: When referring to static members of those types, it is custom to use the full CLS name, e.g. Int32.Parse() instead of int.Parse(). The same applies to members that need to specify the type they return, e.g. ReadInt32, GetUInt16.

Properly name properties, variables or fields referring to localized resources (AV2205)

The guidelines in this topic apply to localizable resources such as error messages and menu text.

Don’t hard-code strings that change based on the deployment (AV2207)

Examples include connection strings, server addresses, etc. Use Resources, the ConnectionStrings property of the ConfigurationManager class, or the Settings class generated by Visual Studio. Maintain the actual values into the app.config or web.config (and most definitely not in a custom configuration store).

Build with the highest warning level (AV2210)

Configure the development environment to use Warning Level 4 for the C# compiler, and enable the option Treat warnings as errors . This allows the compiler to enforce the highest possible code quality.

Properly fill the attributes of the AssemblyInfo.cs file (AV2215)

Ensure that the attributes for the company name, description, copyright statement, version, etc. are filled. One way to ensure that version and other fields that are common to all assemblies have the same values, is to move the corresponding attributes out of the AssemblyInfo.cs into a SolutionInfo.cs file that is shared by all projects within a solution.

Avoid LINQ for simple expressions (AV2220)

Rather than:

var query = from item in items where item.Length > 0 select item;

prefer the use of extension methods from the System.Linq namespace:

var query = items.Where(item => item.Length > 0);

Since LINQ queries should be written out over multiple lines for readability, the second example is a bit more compact.

Use Lambda expressions instead of anonymous functions (AV2221)

Lambda expressions provide a much more elegant alternative for anonymous functions. So instead of:

Customer customer = Array.Find(customers, delegate(Customer customer)
{
	return customer.Name == "Tom";
});

use a Lambda expression:

Customer customer = Array.Find(customers, customer => customer.Name == "Tom");

Or even better:

var customer = customers.Where(customer => customer.Name == "Tom").FirstOrDefault();

Only use the dynamic keyword when talking to a dynamic object (AV2230)

The dynamic keyword has been introduced for working with dynamic languages. Using it introduces a serious performance bottleneck because the compiler has to generate some complex Reflection code.

Use it only for calling methods or members of a dynamically created instance class (using the Activator) as an alternative to Type.GetProperty() and Type.GetMethod(), or for working with COM Interop types.

Favor async/await over the Task (AV2235)

Using the new C# 5.0 keywords results in code that can still be read sequentially and also improves maintainability a lot, even if you need to chain multiple asynchronous operations. For example, rather than defining your method like this:

public Task GetDataAsync()
{
  return MyWebService.FetchDataAsync()
    .ContinueWith(t => new Data(t.Result));
}

define it like this:

public async Task<Data> GetDataAsync()
{
  string result = await MyWebService.FetchDataAsync();
  return new Data(result);
}

Tip: Even if you need to target .NET Framework 4.0 you can use the async and await keywords. Simply install the Async Targeting Pack.

10. Layout Guidelines

Use a common layout (AV2400)

Order and group namespaces according to the company (AV2402)

// Microsoft namespaces are first
using System;
using System.Collections.Generic;
using System.XML;

// Then any other namespaces in alphabetic order
using AvivaSolutions.Business;
using AvivaSolutions.Standard;
using Telerik.WebControls;
using Telerik.Ajax;

Using static directives and using alias directives should be written below regular using directives. Always place these directives at the top of the file, before any namespace declarations (not inside them).

Place members in a well-defined order (AV2406)

Maintaining a common order allows other team members to find their way in your code more easily. In general, a source file should be readable from top to bottom, as if reading a book, to prevent readers from having to browse up and down through the code file.

  1. Private fields and constants (in a region)
  2. Public constants
  3. Public read-only static fields
  4. Factory Methods
  5. Constructors and the Finalizer
  6. Events
  7. Public Properties
  8. Other methods and private properties in calling order

Declare local functions at the bottom of their containing method bodies (after all executable code).

Be reluctant with #regions (AV2407)

Regions can be helpful, but can also hide the main purpose of a class. Therefore, use #regions only for:

A1. Important Resources

The companion website

This document is part of an effort to increase the consciousness with which C# developers do their daily job on a professional level. Therefore I’ve started a dedicated CodePlex site that can be found at www.csharpcodingguidelines.com.

In addition to the most up to date version of this document, you’ll find:

In addition to the many links provided throughout this document, I’d like to recommend the following books, articles and sites for everyone interested in software quality:


Hosted on GitHub Pages. Built from cfc77de73f88d197058053be624bba30e2cad1ee