DotNetCRM Coding Standards

I won't insult your intelligence with a lecture on the virtues of programming standards and the importance of consistency in code. If you don't already know this, or feel that a bunch of damn rules just cramp your personal style, then this isn't the right project for you. Take luck.

A good place to start is the Development Philosophy page, which outlines some important things like our roadmap, general implementation approach, process, how we define success, and so on. It's also important to understand that, while many of the concepts here are timeless, things may change over time. Visit this page on a regular basis.

As a team, we're pretty particular about most of what's documented here, otherwise we wouldn't have wasted our time writing it. Some of the topics are subjective and must be evaluated on a case-by-case basis, while some are immutable and non-negotiable. That said, if you find something below that doesn't make sense to you or notice that something important is missing, speak up. The sole purpose of this document is to help ensure a high-quality product. Nothing more, nothing less.

The Basics

  • The single most important factor in being successful is to understand what we're building (the requirements) and what's been built already (the existing codebase). Failing to do this will ultimately create a lot of extra work, which will probably upset your fellow teammates, or maybe we end up with a CRM that has 40 different logging mechanisms but no actual CRM functionality. To work on this project you need to be an expert on its every facet. Reviewing code will go a long way to this end, and it has the extra benefit of providing an informal code review process.
  • Plan your work. We're all about Agile, but coding as (or without) design is not a tenet of any respectable Agile methodology I know of.
  • We're doing test-driven development. Code will not be accepted without complete corresponding unit tests. THERE ARE NO EXCEPTIONS TO THIS RULE! If you're not excited to the point of near-full-release to unit test your code, then we don't want you on the team.
  • Use mocking when testing external resources or closed subsystems, like a database or web service. We're not here to unit test MSSQL for Microsoft.
  • Write pretty code. Readability and organization are paramount, and usually speak volumes about quality.
  • Try to push changes up in frequent, small increments. If you're working on something experimental or high-risk, however, you may want to create a separate clone of the repository for this purpose and merge it back up when you're done. Just be aware of the potential impact that ongoing development may have on doing this.
  • Don't check in broken code. It pisses everyone else off and it makes you look stupid.
  • Make refactoring a standard part of your development routine.
  • Don't fight the IDE. Or more specifically, don't fight Visual Studio. If you're using some other tool, try to set it up as close to the Visual Studio C# Developer template as possible.

General Style & Formatting

  • Keep methods short and focused on a single objective. Methods longer than 25-30 lines or that addresses multiple concerns are candidate for refactoring.
  • A class containing more than a few hundred lines is probably trying to do too many things, and is a good candidate for refactoring.
  • Make method names descriptive. The purpose of the method should be obvious by its signature.
  • Use the .NET type aliases (e.g. int versus Int32), as they are more widely recognized and rather than the types defined in System namespace.
  • For non-private methods, test that object parameters are not null and throw an ArgumentNullException or ArgumentException if necessary.
  • For private methods, use Debug.Assert to verify that object parameters are not null before using them.
  • Avoid the use of magic numbers. Where this cannot be avoided, use a constant or read-only configuration member to store the value.
  • Do not use string literals in code. Instead, use a constant, read-only configuration member, or resource file.
  • Be conscious of case and culture any time comparing strings. There are several facilities built into .NET for handling this gracefully.
  • Use String.Empty to represent an empty string. When testing for an empty string, use String.IsNullOrEmpty(“somestring”).
  • Encapsulate all non-private class member variables in property constructs.
  • In general, you should attempt to make domain objects stateless. Do not rely on member variables to store state across instances or threads. The primary exceptions here are a object based on the singleton pattern and type members (e.g. constants, statics).
  • Frequent use of the Singleton pattern is a code smell. Consider refactoring.
  • Make static members thread safe by using the lock construct. Utilize a semaphore instead of instance locking when possible.
  • Develop with thread-safety in mind.
  • Use enumerations when you have a list of discrete values. Ensure that enumeration values are correctly validated when writing to persistent storage, in particular an RDBMS.
  • Do not hard code paths to external resources (e.g. a network share or filesystem directory). Set the values via configuration and/or obtain the information programmatically.
  • Never assume that your code will run from c:\<somepath>. If you need the execution path, get it programmatically. Beware of shadow copied assemblies, particularly when dealing with ASP.NET applications.
  • Perform a sanity check and attempt corrective action on application startup, especially if missing external dependencies (e.g. a file or network share) will cause a faulting application. Such conditions should be logged such that administrators or support personnel are notified of potential problems.
  • One class, one file.
  • Always use the most restrictive access modifier possible. Use the InternalsVisibleTo attribute to create friend assemblies instead of exposing potentially sensitive functionality publicly (e.g. for unit testing).
  • Always explicitly define access modifiers. Do not leave private off of declarations.
  • Limit the number of arguments passed to a method. A method with more than 5 is a good candidate for refactoring.
  • Use interface types for list parameters and return values (e.g. IList<T> versus List<T>).
  • When a method returns a list, return an empty list instead of null when no records are found.
  • Use AssemblyInfo.cs to correctly attach version numbers, descriptions, names, etc. to assemblies.
  • Use the using construct working with objects that implement the IDisposable interface. This helps to ensure that unmanaged resources are garbage collected properly. When it is not possible or unadvisable to utilize a using construct, use a try-catch-finally construct to ensure that unmanaged resources are disposed properly.
  • Keep the using construct as short as possible.
  • Add one Easter Egg to every application and technical document.
  • Declare variables as close as possible to their use. Declare only one variable per line.
  • Strings are immutable in .NET. Use a StringBuilder when performing complex string concatenation operations.
  • When doing a comparison with an non assignable value, put it on the left side of the == operator. This prevents an accidental assignment in the case of a typo. For example:

if  (null == someVariable) 
{
	DoSometing();
	...
}
  • In general, you should use whitespace and indentation to enhance the readability of your code.
  • Group logical code blocks using whitespace. For example:

if (0 < numberOfSomething)
{
	DecreaseInventory();
	DoSometing();

	SendNotifications();
	AlertSysop();
}
  • Use a single tab character for indentation. Use the default tab size (4) in Visual Studio.
  • Comments should use the same level of indentation as other code.
  • Curly braces should be placed on their own lines, and at the same level as the parent construct’s definition. For example:

if (0 < numberOfSomething)
{
	DecreaseInventory();
	DoSometing();

	SendNotifications();
	AlertSysop();
}
  • Separate method definitions using one blank line.
  • Use the #region directive to group related pieces of code together, particularly in long classes.
  • Code will not be considered complete until it:
    • Is well organized and has been refactored where necessary.
    • Is well documented.
    • Is checked in.
    • Unit tests successfully with sufficient coverage.
    • Passes all other required quality tests (e.g. static code analysis, integration tests, etc.)
    • Addresses applicable functional and technical requirements.
    • Has been satisfactorily QA tested.
  • Do your best to keep documentation and the discussion area updated and informative.

Naming Conventions

  • Namespace names should follow the pattern: DotNetCrm.<AssemblyName>.<NamespacePathToModule>.<ModuleName>.
  • Use Pascal casing for class names.
  • Use Pascal casing for method names.
  • Use Pascal casing for file names.
  • Use Camel casing for variables and method parameters.
  • Prefix interface names with I.
  • Do not use Hungarian notation (e.g. iSomeNumber, mVariable, etc.) The exception here is for user interface controls; use Microsoft’s standard guidance in this case (e.g. lblMyLabel, etc.)
  • Use meaningful and descriptive variable names. Do not use abbreviations.
  • Only use single character variable names as counter variables in loop constructs.
  • Prefix member variable names with the underscore character. Do not use this prefix for local variable declarations.
  • Don’t use variable names that resemble keywords.
  • Prefix Boolean variables, properties, and methods with is, has, or another similar prefix.
  • When possible, file names should match the name of the class they contain.

Data Access

  • We're using LINQ-to-SQL for most of the DotNetCRM application.
  • Avoid accessing a database directly from an application front-end (e.g. a web form or web service.) Use the BLL and DAL for this purpose.
  • For stateless or multi-threaded applications (e.g. a website), use an offline locking pattern that guarantees consistency of data.
  • All database writes MUST be atomic. Use transactions when necessary.
  • You should use transactions only when they are required. Transactions are expensive and are generally not needed when writing a single row to the database.
  • If possible, use Integrated (Windows) authentication to connect to the database server.
  • If you must use a non-Windows authentication scheme, you MUST encrypt your connection string(s).
  • Use Unicode data types (e.g. nvarchar versus varchar) whenever there is a possibility of localization in the application.
  • Avoid the use of database cursors.
  • Avoid the use of esoteric database features in application code unless they are absolutely required. This promotes portability of the database.

Web Services

  • As a general rule, follow Microsoft’s prescriptive guidance when building web services, particularly those that relate to versioning. Visit the WCF best practices section on MSDN for more information.
  • When possible, use the Microsoft Patterns and Practices Web Service Software Factory to model web services. This tool will help architect services using guidance provided by Microsoft, including the generation of projects and code stubs.
  • Design web services to be stateless.
  • Web services should represent a lightweight application entry point only; do not implement business logic in the web service layer.
  • Use only primitive datatypes in web service signatures. Avoid exposing .NET-specific types, as these may not be interoperable with other languages, platforms, or toolkits.
  • Try to use arrays or objects decorated with the CollectionDataAttribute to express collections of objects in web service signatures. When this is not possible, you should use a concrete list type of List<T>, Dictionary<K, V>, or Hashtable, as these are the only collection types that can be serialized by WCF with HTTP bindings.
  • Avoid creating an extra set of data transfer objects for communication between your service and business logic layers. This adds unnecessary complexity and maintenance headaches for what usually amounts to an insignificant performance gain.
  • Data contracts should always implement the IExtensibleDataObject interface for forward compatilbility.
  • Use XML namespaces to correctly version your services. Use care not to leave the default namespace of tempuri.org on any service definition.
  • Provide clear separation amongst service contracts from their implementation and from data objects. In most cases, this means that a complete web service implementation will be spread over three separate projects.

Web Applications

  • Implement as much of a web application’s complexity as possible outside of the web project.
  • Web forms should be coded to the XHTML 1.0 Strict schema.
  • Style information should be stored in external stylesheets. Do not use inline CSS.
  • JavaScript should be stored in external files. Do not use inline client-side scripting.
  • We're using jQuery for rich client-side programming. Don't introduce another library into the project without good reason (because you like it is not a good reason) and consensus from the rest of the team.
  • Implement client-side scripts such that they degrade gracefully, in-line with browser support requirements for the project.
  • Avoid storing large objects in the ASP.NET Session collection. In general, you should limit the use of the Session object, and be conscious of implications related to Session backing and farmed web servers.
  • Do not pass objects from System.Web (e.g. the Session object) into lower layers of an application. You should (almost) never have a need for a dependency on System.Web in a non-web project.
  • Avoid storing large objects in ViewState, and NEVER store sensitive information there.
  • Take advantage of partial output caching whenever possible.
  • Never access a database directly from a web application.
  • Connection strings and other sensitive information stored in configuration files should be encrypted in RTM releases.

Security

  • Never assume that your code will run with administrative privileges. You should code for least-rights scenario.
  • More information forthcoming.

Logging & Instrumentation

  • When in doubt, err on the side of providing too much logging information.
  • Were using the entlib Logging Application Block for logging.
  • At a minimum, your application should emit trace output sufficient for a developer to debug, support, and tune a running application.
  • Avoid using project-specific jargon or abbreviations in log messages, since these may be meaningless to the person who, probably under duress, is trying to make sense of them.
  • Logging should not be implemented as an afterthought. Do it as you go and make sure it’s well tested.
  • Ensure that logging is well integrated with exception handling.
  • Plan carefully when using a database as a backing store for log output, since a failure in the database would make logging the problem impossible.
  • Allow verbose log output to be toggled via application configuration.
  • Log files should be rolling, such that a single file does not grow to an unusable size.
  • Use care when choosing what to send to a log sink. Standard trace output emitted from an application should not have a negative performance impact on the application. A noticeable performance on an application for verbose and/or debug level output is acceptable, but this MUST NOT be the default logging state and must be manually enabled by technical personnel.

Error Handling

  • We're using the entlib Exception Handling Application Block to handle and manage exceptions.
  • Exception handling should not be implemented as an afterthought. Where exception handling is required, your code will not be considered complete without it. Do it as you go and make sure it’s well tested.
  • Because error handling is a crosscutting-concern, a dependency-injection strategy is recommended for all but the smallest of applications. This allows ultimate flexibility in configuration and testing, and reduces dependencies on specific tooling. Using such a pattern, for example, makes variance in notification targets through different server environments a simple configuration exercise that can be easily scripted.
  • Custom exceptions should always be derived from System.ApplicationException or one of its descendants. System.Exception is reserved for framework-level errors and should not be subclassed.
  • Custom exceptions should always use Exception as a naming suffix, and should exist in the same or a less-specific namespace as the classes that use them. For example, MyApplication.MyCustomException, not MyApplication.Exceptions.Custom or MyApplication.Exceptions.MyCustomException.
  • Make try-catch blocks as short as possible.
  • Do not swallow exceptions. If you’re catching an exception, you should be doing something meaningful with it.
  • NEVER use a try-catch block as a logic construct. Statements like the following should be avoided:

int i = 0;
try
{
	i = int.Parse(string.Empty);
}
catch (Exception ex)
{
	i = -1;
}
  • Exceptions should be caught in order from most specific to least specific, and use this hierarchy to recover from the error whenever possible. Handling System.Exception as a catch-all should be avoided. For example:

try
{
	SomeDangerousOperation();
}
catch (SqlException sex)
{
	DoSometingElse(sex);
	...
}
catch (Exception ex)
{
	DoSometingElse(ex);
	...
}
  • NEVER catch exceptions in the lower layers of an application. Let exceptions bubble up to the top tier and deal with them there.
  • Catch exceptions in the top-tier of your application using a try-catch-finally construct. Ultimately we want DotNetCRM to be as durable and failure-proof as possible.
  • When an exception is catastrophic in nature try to fail gracefully, avoiding the display of operating system or application server error dialogs (e.g. the dreaded Service Unavailable page in IIS). Communicate to the user what happened and what steps might be taken to avoid the error in the future or retry the operation.
  • When an exception message may contain sensitive information (e.g. database name or network path), replace the exception with a generic exception object that contains useful support information, and throw that object. Do not allow exceptions expose a potential security vulnerability or attack vector for your code.
  • Use care when rethrowing exceptions. For example:

try
{
	DoSomeDangerousOperation();
}
catch (BigBadException ex)
{
	DoSomethingImportantWithAnException(ex);
	throw ex; // destroys call stack (bad)
}

// versus...

try
{
	DoSomeDangerousOperation();
}
catch (BigBadException ex)
{
	DoSomethingImportantWithAnException(ex);
	throw; // preserves call stack (good)
}
  • Store string literals used in exception messages in resource files and/or the application configuration. This will vary throughout DotNetCRM, but in general resource files will contain fallback and system messages, while configuration will contain those that are content-centric.
  • We have several notification sinks planned for exceptions. These will include the WEV, email, and possibly SNMP. Plan accordingly.

Documentation

  • The primary goal for documentation is—to the extent possible—to create a self-documenting application. In other words, a developer should be able to understand the purpose and functionality of a module simply by looking at the source code and without in-depth study of tomes of technical documentation. This does not mean that there is or will be no documentation for this project, and consequently is not the same as documentation that will be used by end-users, administrators, or other consumers of the system. Those documents are covered elsewhere on this site. See the Documentation page for more information.
  • Write self-documenting code. If you're following the standards as outlined above, there's a good chance that you're already doing this.
  • Commenting should not be done as an afterthought. Do it as you go.
  • XmlDoc tags drive Intellisense features in Visual Studio, and are critical in helping other developers understand application functionality.
  • XmlDoc documentation is critical for application components that may be reused. For our purposes, this means the entire application.
  • XmlDoc documentation will be used by an external build tools to generate API documentation. Plan accordingly.
  • Use lexical and semantic conventions similar to those used in the .NET BCL, as these will be most familiar to developers.
  • ALL classes, structs, methods, properties, events, delegates, constants, members, etc. should be documented regardless of their visibility. The obvious exceptions are codegen classes and other automatically generated code (e.g. a standard event handler in ASP.NET code-behind).
  • Provide samples in a <remarks> tag for highly-complex classes or for classes where usage might not be readily apparent.
  • DO NOT create empty XmlDoc tags for the purpose of suppressing compiler warnings. Likewise, do not disable documentation related compiler warnings in a project, a shameful transgression for which you will eventually be caught.
  • Avoid documenting code where functionality is obvious. For example, the following is unnecessary:

int i = 0; // initialize i to 0
  • Use the /* … */ convention only when a comment is abnormally long.
  • Document unexpected or complex code blocks.
  • Comments should be written in clear, understandable, and grammatically correct English.
  • GhostDoc is a fantastic, free tool that simplifies the process of documenting code substantially.
  • Always feel free to assist in the development of other documentation. For example: test cases, deployment and/or configuration documentation, support documentation, etc.

Last edited Jul 10, 2010 at 1:35 AM by kcargile, version 17

Comments

No comments yet.