Difference between revisions of "Coding guideline - Common rules"

From Catglobe Wiki
Jump to: navigation, search
 
Line 1: Line 1:
<accesscontrol>Administrators,,Cem,,Maysunshine</accesscontrol>
+
<accesscontrol>Main:MyGroup</accesscontrol>
 
== Introduction ==
 
== Introduction ==
  

Latest revision as of 06:39, 18 October 2013

<accesscontrol>Main:MyGroup</accesscontrol>

Introduction

This document aims at giving common rules that developers MUST follow to ensure not only consistency in the whole application but also the ease of code reading and maintaining. Due to the fact that we are working mainly on C# language, all examples will be given in this language only.

Please refer to the .NET Framework Design Guidelines (http://msdn.microsoft.com/en-us/library/czefa0ke.aspx) for a complete set of guidelines suggested by Microsoft.

Terms and Conventions

Naming Terms

There are 3 terms used for naming files, class names, variable names, etc. All are listed in this section.

Pascal case

The first letter in the identifier and the first letter of each subsequent concatenated word are capitalized. You can use Pascal case for identifiers of three or more characters.

For example: CatGlobeImageButton, LanguageEditor

Camel case

The first letter of an identifier is lowercase and the first letter of each subsequent concatenated word is capitalized.

For example: resourceId, resourceTemplateId

Uppercase All letters in the identifier are capitalized. Use this convention only for identifiers that consist of two or fewer letters.

For example: ID, UI, IO

Style Guidelines

File Organization

Location

  • Class file should be put in directory whose path follows class's namespace. This makes finding a class easier.

For example: code file of class CatGlobe.Web.UI.WebControls.CatGlobeImageButton should be found in side {CODE_BASE}\CatGlobe\Web\UI\WebControls

Naming

  • Always use Pascal for naming code file.
  • Source files should be given the name of the first public class in the file plus “.cs”. The purpose of this naming is clear, it increases the consistent of organization and also the maintainability.

For example: code file {CODE_BASE}\CatGlobe\Web\UI\WebControls\CatGlobeImageButton.cs should have the first public class as CatGlobeImageButton

Content

  • There should be only one namespace declaration in one code file only.
  • One source file should contain only one public class/interface/enum, although multiple internal classes are allowed. Delegates and Interfaces are allowed to be put in the same file as class/interface/enum.

Should

public sealed class CatTask : PersistentObject, ICatTaskClientObject, IComparable
{
private sealed class CatTasksAccess : AccessBase
{
}
}

Should not

public sealed class CatTask : PersistentObject, ICatTaskClientObject, IComparable
{
}
[Serializable]
public sealed class StringConstant
{
}

  • Classes member should be grouped into sections (Fields, Constructors, Properties, Events, Methods, Private interface implementations, Nested types). And there should be a region name in #endregion instructive although it is not required. Regions can be shown/hide while viewing with VS thus increasing the readability.

For example:

public sealed class CatTask : PersistentObject, ICatTaskClientObject, IComparable
{
#region AccessClass
private sealed class CatTasksAccess : AccessBase
{
}
#endregion AccessClass
#region Private Instance Fields
private int resourceId;
private int resourceTemplateId;
#endregion Private Instance Fields
}

A propose class file with regions:

public class SampleClass
{
#region Nested Enums, Structs, and Classes
#endregion Nested Enums, Structs, and Classes
#region Constructors & Finalizers
#endregion Constructors & Finalizers
#region Member variables
#endregion Member variables
#region roperties
#endregion Properties
#region Methods
#endregion Methods
}

  • There should not be more than 500 lines of code inside one file. Files containing more than 500 lines have less readability than the ones with less than 500 lines. There is one exception that we should not take into account on this rules, that are auto-generated files since most of the time we do not maintain these files directly.
  • Each line of code should not exceed 120 characters. Lines with more than 120 characters normally require code viewer using the horizontal scrollbar thus decrease readability.

Tabs, Indenting, Spacing and Bracing

Tabs and Indenting

  • Tab characters (\0x09) MUST NOT be used in code. All tabs must be replaced by 3 space characters.
  • All indentation MUST be done with 3 space characters.

Spacing Spaces improve readability by decreasing code density. Here are some guidelines for the use of space characters within code:

  • Do use a single space after a comma between function arguments.
Right:Console.In.Read(myChar,0,1);
Wrong: Console.In.Read(myChar,0,1);
  • Do not use a space after the parenthesis and function arguments.
Right: CreateFoo(myChar, 0, 1);
Wrong: CreateFoo( myChar, 0, 1 )
  • Do notuse spaces between a function name and parenthesis.
Right: CreateFoo();
Wrong: CreateFoo ()
  • Do notuse spaces inside brackets.
Right: x = dataArray[index];
Wrong: x = dataArray[ index ];
  • Do use a single space before flow control statements.
Right: while (x == y)
Wrong: while(x==y)
  • Do use a single space before and after comparison operators.
Right: if (x == y)
Wrong: if (x==y)

Bracing

  • Open braces must always be at the beginning of the line after the statement that begins the block. This increases readability of code.

Must:

if ((CatTaskStatus.Status == CatTaskCommon.CatTaskStatus.Failed)
{
CatTaskCompleted ctc = CatTaskCompleted.GetByLatestCompletedDate(this);
if (ctc != null)
completedDate = ctc.CompletedDate;
return completedDate;
}
else
{
...
}

Must not:

if ((CatTaskStatus.Status == CatTaskCommon.CatTaskStatus.Failed) {=> Wrong brace's location
CatTaskCompleted ctc = CatTaskCompleted.GetByLatestCompletedDate(this);
if (ctc != null) {
completedDate = ctc.CompletedDate;
}
return completedDate;
}

  • For anonymous methods and lambda expressions, the braces must be indent at the same level with its declaration.

Example:

private SampleDelegate _mySampleDelegate = delegate()
   {
   };
private SampleDelegate _mySampleDelegate = (name) =>
   {
   };

  • Must not put block that contains more than 2 statements in the same line:

Must not:

if (ctc != null) {completedDate = ctc.CompletedDate; return completedDate; }

  • With single statement blocks, using of braces is optionnal. It is encourage to not use the braces to save number of line of code.

Should:

if (ctc != null)
completedDate = ctc.CompletedDate;

Should not:

if (ctc != null)
{
completedDate = ctc.CompletedDate; => 3 lines of code do the same as 1 line
}

  • There are exceptions that brace should be placed on the same line as the keyword that requires braces.

One statement code block:

public int CompletedMailTemplateId
{
get { return completedMailTemplateId; } => It is much nicer than having 4 lines of code
set { completedMailTemplateId = value; }
}

.NET 3.0 property syntax:

public int MyProperty { get; set; }

Tips:
All rules in this section can be applied automatically by applying a common Visual Studio settings file. Developers just have to remember to apply these rules by pressing Ctrl + K + D (under default installation) or Ctrl + E + C using Resharper before committing.

Commenting

Comments should be used to describe intention, algorithmic overview, and/or logical flow. It would be ideal, if from reading the comments alone, someone other than the author could understand a function’s intended behavior and general operation. While there are no minimum comment requirements and certainly some very small routines need no commenting at all, it is hoped that most routines will have comments reflecting the programmer’s intent and approach.

  • Use // or /// for commenting but not /* ... */. The last comment style is out of date, it is useful in the old time when coder wanted to comment a large block of code. Using such comment will save a lot of time as writing // on each line. However, with new editor, comment/uncomment block of code is just a matter of selecting the block and calling a short cut key.
  • Do not decorate your comment with special charaters.

Must not:

// *********************************
// Comments block
// **********************************

  • Avoid comments that explain the obvious. Code should be self-explanatory. Good code with readable variable and method names should not require comments.

Should not

/// <summary>
/// Resource Id
/// </summary>
private int resourceId;

=> Comment that makes no sense. Could be something like: <The primary id of the <see cref=”CatGlobe.Domain.Common.Resource” /> that this class is working with.
  • Include Task-List keywords flags to enable comment-filtering. These comment can be tracked by using Visual Studio Task List window or just simply by searching.

Example:

// TODO: Fix this
// HACK: work-around temporarily while waiting for better solution
// UNDONE: Will be done in version xxxx

Tips:
In Visual Studio, more keywords can be defined in Tools Environment → Task List. More detail description can be found here http://msdn.microsoft.com/en-us/library/zce12xx2(VS.80).aspx

  • Always apply C# comment-blocks (///) to public, protected and internal declaration. The comment will be displayed as tooltip in editor and this helps method's user a lot for writing code correctly.

See picture below:

Class comment displayed inside help text


  • Use comment tags whenever possible, this could give very much help on generating help files. The most used tags are: <summary>, <param>, <paramref>, <return>, <exception>, <see>, <seeAlso>.

Class comment:

/// <summary>
/// This is my sample class for illustating comment on coding guide line document
/// </summary>
public class SampleClass
{
}

Method comment:

/// <summary>
/// This is my test method. It is created for demonstrating how to write comment for
/// a method.
/// </summary>
/// <param name="arg1">This is sample argument 1. It will be used for ... </param>
/// <param name="arg2">This is sample argument 2. It will be used for ...</param>
/// <returns>Return the result string.</returns>
protected string MyMethod(string arg1, string arg2)
{
}

Tips:
In Visual Studio, typing three slashes on the line right before a declaration will automatically create a template for comment on the correspondence element.

An explaination on the comment can be found here: http://msdn.microsoft.com/en-us/magazine/cc302121.aspx

A detail explaination of all comment tags can be found here: http://thoughtpad.net/alan-dean/cs-xml-documentation.

Naming

Common rules

  • Always use Camel Case or Pascal Case for naming. With private instance member, prefix variable name with an underscore '_'.

Pascal case for types (classes/interfaces/delegates/enums/structs) and methods:

public class SomeClass
{
public void SomeMethod() { }
}

Camel case for member variables and parameters:

public void MyMethod(int someNumber)
{
int _number;
}

Camel case preceding by an underscore '_' for private/protected fields:

public class SomeClass
{
private int _number;
protected string _name;
}

Pascal case for enum's values:

public enumCodeReviewResult
{
None,
Approval,
PartialDisapproval,
CompleteDisapproval,
NeedRework,
}

  • Must use Upper Case for naming constant and separate words with underscore character '_'

Example:

public const int DATABASE_CONNECTION_TIMEOUT = 500;

  • Should not create namespaces, classes, methods, properties, fields, or parameters that vary only by capitalization.

Should not:

public void MyMethod(string arg1, string Arg1)
{
}

  • Should not use names that begin with numeric characters.
  • Should not use single char variable names, except in For/While loop.
  • Always choose meaningful and specific names.
  • Should not use abbreviations unless the full name is excessive (over 20 characters). With Visual Studio IDE's IntelliSense, it is very easy finding a variable no matter how long the name is.

Should not:

public List<string> UDFList;

=> this stands for User Defined list, the name hide the purpose of the owner variable from code reader

Namespace naming

The general rule for naming namespaces is to use the company name followed by the technology name and optionally the feature and design.

Example:

namespace CatGlobe.DiffStat.Console;
namespace CatGlobe.DiffStat.WinApp;

Type naming

  • Use noun or pronoun for naming classes/interfaces.

Should not:

public class Run
{
}

Should:

public class Runner
{
}

  • MUST prefix an interface with character I.

Example:

public int erface ISomeInterface
{
}

  • MUST suffix custom attribute classes with Attribute.

Example:

public class MyCustomAttribute: System.Attribute
{
}

  • MUST suffix custom exeptions classes with Exeption.

Example:

public class MyException: System.Exception
{
}

  • MUST NOT suffix any enums with Enum.

Must not:

public enum ResourceTypeEnum=> it is better if Enum is removed
{
}

Method and Property naming

  • Use a verb-object pair for naming methods.

Example:

public void TestCatTask();
public void GetName();

  • If returned type is Boolean, try to prefix method name with “Can”, “Is” or “Has”.

Example:

public bool CanSave()
public bool IsEditable()
public bool HasAccess()

  • Never perfix a property name with “Get”, “Set”. It is because when using value of a property it is already understand as calling to a “Get” method.

Must not:

public int GetIntValue { get; set; }

Must:

public int IntValue { get; set; }

Generic types naming

  • Should use single capital letters for generic types.

Should:

public class List<T>where T : string
{
}

Code organization

using Statement

  • All using statements should be put outside any namespaces statements.

Should:

using CatGlobe.Web.UI.WebControls;
using CatGlobe.Web.Common.ClassBase;
using CatGlobe.Web.Common.ServerMethods;
using CatGlobe.Domain.SiteVisitor.ServiceHandler;
namespace CatGlobe.Web.Common.Resources.List;
{
}

Should not:

namespace CatGlobe.Web.Common.Resources.List
{
using CatGlobe.Web.UI.WebControls;
using CatGlobe.Web.Common.ClassBase;
using CatGlobe.Web.Common.ServerMethods;
using CatGlobe.Domain.SiteVisitor.ServiceHandler;
}

  • Related using statements should be group together. This helps reader an overview of which library will be used in the remaining code.

Example:

using System;
using System.Xml;
using System.IO;
using Infragistics.WebUI.UltraWebGrid; => Reference Infragistics
using Infragistics.WebUI.UltraWebNavigator;
using Infragistics.WebUI.UltraWebToolbar;
using CatGlobe.DataAccess;=> Reference CatGlobe's DataAccess
using CatGlobe.Framework.GroupBuilder; => Reference CatGlobe's Framework
using CatGlobe.Framework.Resources;

Internal Types

  • Types that are declared inside a class must be group together. And the group must be put right after class definition.

Example:

public class ResourceList
{
#region Types
/// <summary>
/// Collection of <seecref="CatGlobe.Domain.Common.Resource"/>s,
/// in which we can sort the whole list or
/// fetch resources based on some criteria.
/// </summary>
private class ResourceCollection
{
}
/// <summary>
/// Provide methods for Save/Delete a single <seecref="CatGlobe.Domain.Common.Resource"/>
/// or collection of <see cref="CatGlobe.Domain.Common.Resource"/>s.
/// </summary>
private class ResourceHandler
{
}
#endregion Types
}

Member and Local Variables

  • All class's member variable must be put at top of class (after internal types declaration). This increase readability of code, since at the first look the viewer can tell what are members of class and also theirs types.
  • Each variable of different types must be declared on different lines.

Must:

private string _sampleString;
private int _sampleInt;
private long _sampleLong1, _sampleLong2, _sampleLong3;

Must not:

private string _sampleString;
private int _sampleInt;

  • Local variables must be clared as close as possible to its first use and must be in the same code block that use it.

Methods

  • A method must not exceed 100 lines. Methods with more than 100 lines usually decrease readability and thus maintainability.
  • Methods that relate to each others by any means should be grouped together.

Example:

#region Database Access Methods
#endregion Database Access Methods
#region Private Utility Methods
#endregion Private Utility Methods
#region Serialization Methods
#endregion Serialization Methods

  • Interface implementations must be grouped together.

Example:

public interface ISample
{
void SampleMethod();
}
public class SampleImplementation: ISample
{
#region ISample Members
void ISample.SampleMethod()
{
}
#endregion ISample Members
}

Indentation deep

  • All methods' indentation must not exceed a deep level of 3. Methods, whose indentation exceeds 3 in deep level, decrease readability, maintainability and also increase complexity.

Must not:

if (HasAccess)
{
for (int i = 0; i < COUNT; i++)
{
if (list[i].IsTrue)
{
switch (resourceType)
{
=> Code inside this block are too deep
}
}
}
}

Tips:
There are 2 methods for reducing the deep level of code block:

  • Reorganizing if statement

Original code:

public void SampleMethod(bool someCondition)
{
if (someCondition)
{
for (int i = 0; i < COUNT; i++)
{
}
}
}

Changed code:

public void SampleMethod(bool someCondition)
{
if (!someCondition)
return ;
for (int i = 0; i < COUNT; i++)
{
}
}

=> The indentation's deep level has been decreased by 1.

  • Creating private method

Original code:

public void SampleMethod(bool someCondition)
{
if (HasAccess)
{
for (int i = 0; i < COUNT; i++)
{
if (list[i].IsTrue)
{
foreach (int value in list[i].Values)
{
if (value > 0)
{
for (int j = 0; j < value; j++)
{
if (anotherCondition)
{
}
}
}
}
}
}
}
}

Changed code:

public void SampleMethod(bool someCondition)
{
if (HasAccess)
{
for (int i = 0; i < COUNT; i++)
{
if (list[i].IsTrue)
{
ProcessValues(list[i].Values);
}
}
}
}
private void ProcessValues(List<int> values)
{
foreach (int value in values)
{
if (value > 0)
{
for (int j = 0; j < value; j++)
{
if (anotherCondition)
{
}
}
}
}
}

=> The indentation's deep level is dreased more than 1

Attribute declaration

  • For elements that have more than one Attribute declaration, then each Attribute must be put on one line.

Must not:

[Serializable, STAThread, Obsolete("This class is obsolete. Use class Abc instead.")]
public class SampleClass

Must:

[Serializable]
[STAThread]
[Obsolete("This class is obsolete. Use class Abc instead.")]
public class SampleClass

Coding guidelines

Basic language usage

Access modifier

  • All access modifier must be explicitly given, except in interface's declaration where public is undertood by default.

Must not:

class SampleClass
{
int sampleInt;
void SampleMethod()
{
}
}

Variables and Types

  • If possible, initialize variables at declaration time.

Should:

private int primaryId = 0;
private string sampleString = string.Empty;
private List<int> sampleList = new List<int>();

  • Must use fully qualified name of types, declare the type by using statements. In case there are duplication in type's name, define aliases.

Must not:

ResourceCollectionPager pager = CatGlobe.Framework.Services.SessionManager.Current.DataModule.Get<ResourceCollectionPager>(Request["sessionid"]);

Must:

using CatGlobe.Framework.Services;
ResourceCollectionPager pager = SessionManager.Current.DataModule.Get<ResourceCollectionPager>(Request["sessionid"]);

Tips:
Always choose “using” over fully qualified name from Resolve menu of Visual Studio.
Resolve menu of VS2008

Example of using alias:

using Threading = System.Threading;
using Timers = System.Timers;
using Web = System.Web.UI;
namespaceCatGlobe.DataAccess
{
public class SampleClass
{
private Threading.Timer _threadTimer1;
private Timers.Timer _threadTimer2;
private Web.Timer _threadTimer3;
}
}

Constants and Literals

  • Never hardcoded strings that will be presented to end-users. Use Resoures class instead.

Must not:

<span>Username</span>

Must:

<span><%=Resources.Username %></span>

  • Try to use @ instead escape characters symbol

Should:

string filename = @"D:\test\test.xml";

Direct cast vs. as keyword

Following code block:

CustomClass a = src as CustomClass;

can be considered the same as the code block below:

if (src is CustomClass)
return (CustomClass)src;
else
return null;

Consequently, it is encouraged to use as keyword over directcasting, because it is neat and safer.

Exceptions handling

  • Catch least generic exception first

Must not:

try
{
using (FileStream fs = newFileStream(@"D:\test.xml")) { }
}
catch (Exception) { }
catch (IOException)
{
=>Unreachable code
}

  • Do not use exception for controlling flows, instead try to validate input parameter. Most of the time exceptions can be prevented by proper input validation.

Should not:

string input = "something";
try
{
DoWork(input);
}
catch (Exception e)
{
DoAlternateWork(input);
}

  • All exception must be re-thrown, the outest layer (usually GUI win-form or web-form) must responsible for handling the exception properly.

Must not:

try
{
DoWork();
}
catch (Exception e)
{
// do nothing
}

  • If not creating a wrapper exception, use “throw” instead of “throwe” to reserve the call-stack. “throw” statement's stack trace points to the code place that actually causes the error, while “throwe”'s stack trace points to the location of the “throwe” statement only.

Must not:

catch (IOException ioe)
{
Logger.LogError(ioe);
throw ioe; => Must be throw;
}

Correct:

catch (IOException ioe)
{
Logger.LogError(ioe);
throw new CustomeException("error", ioe);
}
catch (IOException ioe)
{
Logger.LogError(ioe);
throw;
}

Document revisions

Version No. Date Changed By Description
0.1 10.07.2008 Nguyen Trung Chinh Copy from http://blogs.msdn.com/brada/articles/361363.aspx
0.1 22.08.2008 Nguyen Trung Chinh Refine from original document, add more applicable rules
0.1 25.08.2008 Nguyen Trung Chinh Fix for 0.1 after reviews of Dennis and Thuan
0.2 08.10.2008 Nguyen Trung Chinh Add section 3.5 and 4
0.3 13.10.2008 Nguyen Trung Chinh Finish section 4
1.0 14.11.2008 Nguyen Trung Chinh Finalize version 1.0