Code Review Checklist

Coding Style
	Use tabs to indent code, spaces to align within code
	Use the vertical pair-matching style of bracing
	System variables declared using system keyword aliases (e.g. int, float, string ) instead of base types ( e.g System.Int32, System.Single, System.String)
	Declare variables as closely as possible to first use and within the deepest possible scope
	Do not use public constants unless the value is truly a constant (e.g. Avogadro's Number but not our company name)
	Use static readonly for public "constants"
	Avoid using fully-qualified type names in your code
	Do not use using keywords inside a namespace declaration
	If an enumeration represents combinable settings (e.g. bitmasks), mark the enum with the [Flags] attribute
	All switch statements must have a default case [ See Standards doc]
	Avoid compiling out methods using #if/#endif constructs (see standards doc for alternative)
	Place all assembly-level attributes (e.g. [assembly: FileIOPermission(…)] ) in AssemblyInfo.cs
Threading
	The lock keyword is used for any updates/modifications to objects shared across threads
	Do not use lock(typeof(x)). Locking on typeof will cause an appDomain lock on all instances of that type of object
	Is the right locking mechanism being used? [lock, Monitor, etc]
Arithmetic
	Use checked region on arithmetical operations that may overflow
Naming Conventions
	Assemblies' names must reflect their root hierarchy (e.g.  "Monster.Seeker.Users.dll")
	Each “dot level” of a namespace should be organized into a separate directory
	Classes should be Pascal cased and named using nouns
	Underscores should not be used in any class / variable names (except as specified below for private fields and within constant names)
	A private field of a class should be named with a leading underscore (“_”) character
	Classes should be organized into separate files
	Do not name entities of the same level (e.g. namespace, method, property) that differ only in case
	Classes serving as collections should have names ending with “Collection”.  The general form when aggregating an entity class is <entityname>Collection
	Custom attributes must be named ending with “Attribute”.
	Interfaces should be named with a prefix of “I” followed by nouns, noun phrases or adjectives describing behavior
	A variable name should use camel casing
	Method names should be camel cased, as long as necessary to describe action and composed of verb/verb phrases
	Abbreviations should not be used
	Constants should be all upper case with underscores between words
	Do not use an Enum suffix on enumeration
	Do not use hungarian notation except for WebForm elements
Exception Handling
	Only catch exception when necessary to add value/log/recover
	Exceptions must be caught/logged with one of the following: MS ExceptionManagementBlock/MS EntLib1.0/Log4net
	Do not write code in which exceptions are common
	Do not use exceptions to control program flow
	Within a catch region, avoid "throw exp", use "throw" instead
	Do not use Application Exception (ever)
	Use standard framework-defined exceptions whenever possible
	Catch exceptions as specifically as possible with more general exception types in lower catch regions
	Custom exceptions must be have a name ending in Exception and be derived from System.Exception unless a more specific exception exists. Do not derive from ApplicationException
	Custom exceptions should have the [Serializable] attribute and implement an additional (protected) constructor accepting SerializationInfo and StreamingConext
	Ensure Try-Finally blocks are used correctly
	Ensure Try-Catch_Finally blocks are used correctly
Code Documentation
	All classes and methods should have Xml documentation
	Inline comments should only be used to convey the non obvious or clarify complex logic
Class Design
	All classes should have a default parameterless constructor
	Static variable defaults should be assigned in static constructor
	Constructor should not cause a potentially long running process
	Classes that should not be instantiated should have a private constructor
	All properties should be non deterministic [e.g. do not rely/require on property a to be set before property b]
	All classes that are not base classes should be marked Sealed
	Consider marking any protected/public method in a non sealed class as virtual (performance hit in doing so)
	Use structure instead of a class for very small classes [ see standards doc ]. Total size s/b less than 16 bytes
	ToString() is overridden to return meaningful information about the class
	Object comparison (IComparable) is implemented correctly
	GetHashCode is overridden correctly [return value is immutable over the lifetime of the object]
	Reference Types are exposed as ReadOnly or via helper properties
	If ICloneable is used the class documents whether a deep or shallow copy of the object is returned
	Static only classes should be marked Sealed
	Are method arguments validated and rejected with an exception if they are invalid?
	Singleton objects should be created without the double-check locking pattern [See DevWiki/MSDN PAG for proper C# singleton pattern]
	Are all classes that will be marshaled or remoted marked with the Serializable attribute?
	All classes that explicitly implement ISerializable must provide both the required GetObjectData and the implied constructor that takes a SerializationInfo and a StreamingContext?
	Delegates have a void return type and do not use output and ref params
	All members of derived EventArg classes should be read-only. This prevents one subscriber from modifying the EventArgs, which would affect the other subscribers
	Delegates should be published as events?  This prevents the subscribers from firing the event, see Lowy, p. 102 for details.
GC
	All classes that use scarce resources/unmanaged resources should implement IDisposable
	All classes that implement IDisposable are wrapped in using statements
Overloading
	Overloaded methods should be functionally equivalent. A specific overload should not result in different behavior [use a new method instead]
ADO.NET
	Global.Database.Connections is used to get all SqlConnection objects
	All stored procedures are accessed via Command Layer Generator (CLG) generated classes
	System.Data.SqlClient is used for access to SQL Server. OLEDB should not be used unless no other option exists
	All connections are wrapped in a using statement to ensure they are closed upon exit
	Do not pass open datareader between methods/layers
	Do not pass open database connection between methods/layers
	Open and close a database connection rather than hold connection open for multiple requests (assuming other processing is done between requests)
	Do not use a DataSet for only one DataTable. Use a DataTable or a DataReader
	Do not use a DataSet/DataTable when only returning a single value or row.
	Supply the CommandBehavior properties (e.g. CloseConnection) to the command object ExecuteReader method
	Do not cache connection strings returned from Global.Database.Connections
ASP.NET
	No server-side code in the aspx page [ e.g. Binding.Eval]
	Disable viewstate for all controls that do not require it
	Do not use a Datagrid when a Repeater control will suffice
	Do not use the built in DataGrid paging
Web.Config
	Configure unhandled exceptions to go to default error page
	Ensure that debug = false
	Ensure that no environmental specific values exist (e.g. QA, Production, Dev)
Caching
	Pages should be designed with output caching in mind. Enable output caching everywhere feasible
	Expensive, static data should be placed in Cache object
	All items added to cache should have priority and expiration values set
	Items retrieved from cache use the correct design pattern ( no race conditions ) [ See DevWiki/Standards doc]
	Do not store updatable data in cache [ See Standards doc]
	Do not store user specific data in cache
Interop
	Avoid Interop - consider all alternatives first
	Any pages calling Apartment threaded COM objects must set ASPCompat = true page directive
	Do not construct STA objects in page constructor
Casting/Boxing
	Avoid excessive boxing operations - reference to value types and value types to reference types
	Arraylist should not be used to store value types (due to boxing)
	The as keyword should be used to cast reference types
	All reference types should be checked for NULL prior to use
NUnit
	Unit tests are in a separate assembly from tested code
	Unit test project exists as <project>.Test [e.g. a separate project under the tested projects folder]
	Unit tests provide for at least 60% code coverage [ see CoverageEye for code coverage]
	Negative nUnit tests use the ExpectedException attribute to indicate that an exception must be thrown
	All public methods have at least one corresponding unit test
Globalization
	Never hard-code text that will be displayed to users.  Create a message and use Global.Translations to retrieve it.