# C# Practices and Style Guide Some parts of this guide are inspired by [Microsoft Framework Design Guidelines](https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/), please read it if you can. - ## Naming Guidelines - Use `camelCasing` for `variable` and `parameter` names - Use `PascalCasing` for `Namespace, Type, Interface, Method, Property, Event, Field, Enum value`. - A special case is made for two-letter acronyms in which both letters are capitalized. eg. `IOStream` - Do not capatilize each-word in compound word eg. write `Gridline` and not `GridLine` - Choose readable identifie name eg. a property named `HorizontalAlignment` is more English-readable than `AlignmentHorizontal`. - Favor readability over brevity eg. the property name `CanScrollHorizontally` is better than `ScrollableX` - Do not use underscores, hyphens, or any other nonalphanumeric characters. - Do Not use abbreviations or contractions as part of identifier names. eg. use `GetWindow` rather than `GetWin`. - Use following template for *naming namespaces*: `<Company>.(<Product>|<Technology>)[.<Feature>][.<Subnamespace>]` - Do not use hungarian notation. eg. `string m_sName;` or `int nAge;` - Prefix `boolean` variables, properties and methods with `is` or similar prefixes. - ##### Names of classes, Structs and Interfaces - Name classes and structs with nouns or noun phrases - Name interfaces with adjective (quality of nouns) phrases, or occasionally with nouns or noun phrases - Use verbs for method names - Consider ending the name of derived classes with the name of the base class. eg. `ArgumentOutOfRangeException`, which is a kind of `Exception`. But with reasonable judgment eg. `Button` class is a kind of `Control`, but`Control` doesn't appear in name. - Prefix interface names with the letter I. eg `IComponent` eg. ``` c# namespace i2v.vms.Queues { public class MessageQueue : IEnqueuable<string> { // variable private int messageCount; // property public int QueueLength { get; } // Event public event EventHandler MessageEnqueued; public void Enqueue(string message) { // store message } public bool CanEnqueueMessage() { // check cases return true; } } } ``` - Use appropriate *prefix* for each of the *ui element*. A brief list is given below. | Control | Prefix | | ------------ | --------| | Label | lbl | | TextBox | txt | | DataGrid | dtg | | Button | btn | | ImageButton | imb | | Hyperlink | hlk | | DropDownList | ddl | | ListBox | lst | | DataList | dtl | | Repeater | rep | | Checkbox | chk | | CheckBoxList | cbl | | RadioButton | rdo | | RadioButtonList | rbl | | Image | img | | Panel | pnl | | PlaceHolder | phd | | Table | tbl | | Validators | val | - ## Identation and Spacing - Use TAB for indentation. Do not use SPACES. Define the Tab size as 4. - Comments should be in the same level as the code (use the same level of indentation). eg. ``` c# // Format a message string fullMessage = "Hello" + name; ``` - Curly braces ( { } ) should be in the same level as the code outside the braces. eg. ``` c# if (...) { // Do something // ... return false; } ``` - Use one blank line to separate logical groups of code. eg. ``` c# bool SayHello ( string name ) { string fullMessage = "Hello " + name; DateTime currentTime = DateTime.Now; string message = string.format("{0}, the time is : {1}",fullMessage, currentTime.ToShortTimeString(); MessageBox.Show ( message ); if ( ... ) { // Do something // ... return false; } return true; } ``` - Use a single space before and after each operator and brackets. eg. ```c# if ( showResult == true ) { for ( int i = 0; i < 10; i++ ) { // } } ``` - Avoid writing horizontally long lines of code. ```C# // BAD var studentNames = studentList.Where(s => s.Age > 18).Select(s => s).Where(st => st.StandardID > 0).Select(s => s.StudentName); // GOOD var studentNames = studentList.Where(s => s.Age > 18) .Select(s => s) .Where(st => st.StandardID > 0) .Select(s => s.StudentName); ``` - ## Control Flow - Avoid Deep Nesting - Do not change a loop variable inside a *for* loop block. - All flow control primitives (if, else, while, for, do, switch) shall be followed by a block, even if it is empty. - Try turning your conditions to functions. ```C# if( hero.health > 10 && !villain.IsBoss() ) { // do something } // REFACTOR IT TO if( CanHeroKillVillain(hero, villain) ) { // do something } bool CanHeroKillVillain(Hero hero, Villain villain) { return hero.health > 10 && !villain.IsBoss(); } ``` - Avoid *multiple* or *conditional return* statements. However, in some cases, such as when preconditions are checked, it may be good practice to exit a method immediately when a certain precondition is not met. - Avoid conditions with double negatives. eg. ```C# // DONT DO THIS bool hasOrders = !customer.HasNoOrders; ``` - Don't use method parameters as temporary variables. - ## Type Design - **Allocations** and **deallocations** of **value types** are in general **cheaper** than allocations and deallocations of reference types. - Reference type assignments copy the reference, whereas value type assignments copy the entire value. Therefore, **assignments of large reference** types are **cheaper** than assignments of large value types. - Reference types are passed by reference, whereas value types are passed by value. - Changes to an instance of a reference type affect all references pointing to the instance. - Value type instances are copied when they are passed by value. When an instance of a value type is changed, it of course does not affect any of its copies. - Because the copies are not created explicitly by the user but are implicitly created when arguments are passed or return values are returned, value types that can be changed can be confusing to many users. Therefore, **value types should be immutable**. - **CONSIDER** defining a *struct* instead of a class if instances of the *type are small* and *commonly short-lived* or are *commonly embedded in other objects*. - **AVOID** defining a *struct* unless the type has all of the following characteristics: - It logically represents a single value, similar to primitive types (`int`, `double`, etc.). - It has an instance size under 16 bytes. - It is immutable. - It will not have to be boxed frequently. - ### Abstract class - **DO NOT** define public or protected internal constructors in abstract types. - **DO** define a protected or an internal constructor in abstract classes. - **DO** provide at least one concrete type that inherits from each abstract class. - ### Static class - Declare static classes as sealed, abstract. - Use static classes only as holders of extension methods, or functionality for which a full object-oriented wrapper is unwarranted. - **DO NOT** treat static classes as a miscellaneous bucket. - **DO NOT** declare or override instance members in static classes. - **DO NOT** provide a default constructor for a struct. - Favor using an enum instead of static constants. - ## Object Orientation - Declare all fields (data members) *private*. Exception: - static readonly fields and const fields, which may have any accessibility deemed appropriate. - Classes that only contain fields and no methods/properties. - Prevent instantiation of a class if it contains *only static* members. - Explicitly define a *protected* constructor on an *abstract* base class. - Make all types internal by default. eg. ```C# internal class BaseClass { public static int intM = 0; } ``` - Limit the contents of a source code file to one type (class). - All variants of an overloaded method shall be used for the same purpose and have similar behavior. - Override the GetHashCode method whenever you override the Equals method. - Do NOT use the Equals method to compare diffferent value types, but use the equality operators instead. - ## Object Lifecycle - In methods *declare and initialize* variables *close to where they are used*. - If possible, *initialize variables at the point of declaration*. - Use a *public static readonly* field to define *predefined object instances*. eg. ```C# // consider a Color class/struct that expresses a certain color // internally as red, green, and blue public struct Color { public static readonly Color Red = new Color(0xFF0000); public static readonly Color Black = new Color(0x000000); public static readonly Color White = new Color(0xFFFFFF); public Color(int rgb) { // implementation } } ``` - Do not 'shadow' a name in an outer scope. ```C# // DON'T DO THIS int foo = 123; if ( whatever ) { double foo = 0.14; // ... } ``` - Implement IDisposable if a class uses unmanaged/expensive resources such as objects returned by C/C++ DLLs, owns disposable objects or subscribes to other objects. - Properties, methods and arguments representing *strings* or *collections* should never be `null`. - C# members (methods, properties) and *arguments that return a collection* (such as a list or a dictionary) or string *should never return null*. Instead of returning null when there is nothing more useful to return, they should always return the empty list, the empty dictionary, the empty string, etc. ``` C# List<int> GetUserIds() { List<int> usersList; // get user ids from somewhere if( !found ) { usersList = List<int>(); } else { // fill userlist } return usersList; } ``` - Return an `IEnumerable<T>` or `ICollection<T>` instead of a concrete collection class. - Don't use "using" variables outside the scope of the "using" statement ```C# // WRONG NEVER DO THIS private SsisPackageTester CreateSsisPackageTester(string workbookFile) { using (SsisPackageTester tester = CreateSsisPackageTester(workbookFile)) { tester.SetAssertionVisitors(DefaultAssertionVisitors); return tester; // WRONG } } ``` - ## Error / Exception handling - For conditions that are likely to occur but might trigger an exception, consider handling them in a way that will avoid the exception. ```c# // DONT DO THIS // Might throw InvalidOperationException depending on connection state try { conn.Close(); } catch (InvalidOperationException ex) { Console.WriteLine(ex.GetType().FullName); Console.WriteLine(ex.Message); } // DO THIS // instead we can write if (conn.State != ConnectionState.Closed) { conn.Close(); } ``` - **DO NOT** throw or catch generic `System.Exception` instead be specific while throwing or catching exceptions. - Use Exception Filters ```c# WebClient wc = null; try { wc = new WebClient(); //download a web page var resultData = wc.DownloadString("http://google.com"); } catch (WebException ex) when (ex.Status == WebExceptionStatus.ProtocolError) { //code specifically for a WebException ProtocolError } catch (WebException ex) when ((ex.Response as HttpWebResponse)?.StatusCode == HttpStatusCode.NotFound) { //code specifically for a WebException NotFound } catch (WebException ex) when ((ex.Response as HttpWebResponse)?.StatusCode == HttpStatusCode.InternalServerError) { //code specifically for a WebException InternalServerError } finally { //call this if exception occurs or not wc?.Dispose(); } ``` - **DO NOT** explicitly throw `StackOverflowException` or `OutOfMemoryException` or any other exceptions that are meant to be thrown only by the CLR. - **DO NOT** allow publicly callable APIs to explicitly or implicitly throw `NullReferenceException`, `AccessViolationException`, or `IndexOutOfRangeException`. These exceptions are reserved and thrown by the execution engine and in most cases indicate a bug. - **DO** report execution failures by throwing exceptions. - **CONSIDER** the performance implications of throwing exceptions. Throw rates above 100 per second are likely to noticeably impact the performance of most applications. - **DO NOT** have public members that return exceptions as the return value or an `out` parameter - **DO NOT** return error codes usually but read below. - ### Return magic value, throw exception or return false on failure? - Throw exception when the semantics of the function is that the caller is sure that it will work. eg. `Dictionary<,>.Item` throws `KeyNotFoundException`, `Double.Parse` throws `FormatException`; - Use an `out` parameter and return a `bool` if the semantics is to probe if the operation is possible or not. eg. `Dictionary<,>.TryGetValue`, `Double.TryParse`. - Magic value is a option when there is an "until" condition such as `StreamReader.Read` or when there is a simple to use value that will never be a valid answer (-1 for `IndexOf`). eg. `Collection<T>.IndexOf` returns -1, `StreamReader.Read` returns -1. **BUT** first preference should be the `Try-Parse`/`Tester-Doer` pattern. - Article Links - [msdn best practices for exceptions](https://docs.microsoft.com/en-us/dotnet/standard/exceptions/best-practices-for-exceptions) - ## File Access - When applicable, always use using statements to open files. - To open the file beforehand is not only unnecessary but also wrong. ```c# // open file var stream = File.Open(path, FileOpen.Read); // do something else var image = callToSomeApi("/cat.png"); ... ``` - If possible serialize access to file. - use `FileShare` enumeration to instruct OS to allow other processes (or other parts of your own process) to access same file concurrently. ```c# // opens a file for writing and share for reading using (var stream = File.Open(path, FileMode.Open, FileAccess.Write, FileShare.Read)) { // do something } ``` - ## Performance - **Avoid** boxing and unboxing of value types. [msdn article on boxing and unboxing](https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/types/boxing-and-unboxing) - Value types get boxed when cast to a reference type or one of the interfaces they implement. They get unboxed when cast back to the value type. Because boxes are objects that are allocated on the heap and are garbage-collected, too much boxing and unboxing can have a negative impact on the heap, the garbage collector, and ultimately the performance of the application. In contrast, no such boxing occurs as reference types are cast. ```c# // BAD for Performance List<object> numberList = new List<object>(); // Add some integers to the list. for (int j = 1; j < 5; j++) { // Each element j is boxed when you add j to numberList. mixedList.Add(j); } ``` - **Do not** use ToLower/ToUpper for case insensitive string comparison. - Consider using `Any()` to determine whether an `IEnumerable` is empty. - Test for empty strings using `String.IsNullOrEmpty`. - Always use *Stringbuilder* for String concatenation operations. - Use the *evaluation order* of && (and operator) and || (or operator) to increase performance. - **Use** *List* instead of *ArrayList* especially for value types. - Throw Fewer Exceptions. - Data marshalling for text such as conversion from ASCII to Unicode is Expensive. - P/Invoke, interop and remoting calls all carry overhead, so use them sparingly and make such function calls chunky. - Use `AddRange` to Add Groups. - Precompile Windows Forms Applications during install. - Keep IO Buffer Size Between 4KB and 8KB - ## General - ## Links - [few simple rules for good coding](https://hackernoon.com/few-simple-rules-for-good-coding-my-15-years-experience-96cb29d4acd9)