Best Practices and Standards for Clean Code

Photo by Goran Ivos on Unsplash

Best Practices and Standards for Clean Code

Welcome to our guide on writing clean, maintainable, and consistent code. Adhering to naming conventions, following standardized coding practices, and applying good indentation and spacing are crucial aspects of creating high-quality software. Let's delve into the best practices that will help you produce clean and efficient code.

  1. Naming Conventions and Standards
  • Use Pascal casing for Class names
public class HelloWorld
{
    ...
}
  • Use Pascal casing for Method names
void SayHello(string name)
{
    ...
}
  • Use Camel casing for variables and method parameters
int totalCount = 0;
void SayHello(string name)
{
    string fullMessage = "Hello " + name;
    ...
}
  • Use the prefix “I” with Camel Casing for interfaces ( Example: IEntity )

  • Do not use Hungarian notation to name variables.

  • All variables should use camel casing.

  • Use Meaningful, descriptive words to name variables. Do not use abbreviations.

//Good
string address
int salary
//Not Good:
string nam
string addr
int sal
  • Do not use single-character variable names like i, n, s etc. Use names like index, and temp.

  • One exception in this case would be variables used for iterations in loops:

for ( int i = 0; i < count; i++ )
{
    ...
}
  • If the variable is used only as a counter for iteration and is not used anywhere else in the loop, many people still like to use a single char variable (i) instead of inventing a different suitable name.

Do not use underscores () for local variable names (variable local to functions/ or parameters of function) but keep () for class-level variables.

public class Employee
{
    string _name { get; set; }
    string _address { get; set; }
    public Employee(string name, string address)
    {
        _name = name;
        _address = address;
    }
}
  • Do not use variable names that resemble keywords.

  • Prefix boolean variables, properties and methods with “ is ” or similar prefixes. Use affirmative names where possible, i.e. isValid is preferred to isNotValid. The exception is in LINQ Method Chaining where you cannot apply “!” to a method.

  • Ex: private bool isFinished

  • Namespace names should follow the standard pattern

<company name>.<product name>.<top level module>.<bottom level
module>
  • The file name should match with class name.

  • For example, for the class HelloWorld, the file name should be HelloWorld.cs

  • Use Pascal Case for file names.

  1. Indentation and Spacing
  • Use TAB for indentation. Do not use SPACES. Define the Tab size as 4.

  • Comments should be at the same level as the code (use the same level of indentation).

//Good:
// Format a message and display
string fullMessage = "Hello " + name;
DateTime currentTime = DateTime.Now;
string message = fullMessage + ", the time is : " +
currentTime.ToShortTimeString();
//Not Good:
// Format a message and display
string fullMessage = "Hello " + name;
DateTime currentTime = DateTime.Now;
string message = fullMessage + ", the time is : " +
currentTime.ToShortTimeString();
  • Curly braces ( {} ) should be at the same level as the code outside the braces.

  • Use one blank line to separate logical groups of code.
//Good:
bool SayHello ( string name )
{
    string fullMessage = "Hello " + name;
    DateTime currentTime = DateTime.Now;
    string message = fullMessage + ", the time is : " +
    currentTime.ToShortTimeString();
    MessageBox.Show ( message );
    if ( ... )
    {
        // Do something
        // ...
        return false;
    }
    return true;
}
//Not Good:
bool SayHello (string name)
{
    string fullMessage = "Hello " + name;
    DateTime currentTime = DateTime.Now;
    string message = fullMessage + ", the time is : " + currentTime.ToShortTimeString();
    MessageBox.Show ( message );
    if ( ... )
    {
        // Do something
        // ...
        return false;
    }
    return true;
}
  • There should be one and only one single blank line between each method inside the class.

  • The curly braces should be on a separate line and not in the same line as if, for etc. This applies to C# only, the general convention for JavaScript/JQuery is to put the starting brace on the same line

C#

//Good:
if ( ... )
{
    // Do something
}
//Not Good:
if ( ... )
{
    // Do something
}

JavaScript/ JQuery:

//Good:
var OpenPopUp = function () {
    console.log('Test');
}
//Not Good:
var OpenPopUp = function()
{
    console.log('Test');
}
  • Use a single space before and after each operator and bracket.
//Good:
if (showResult == true)
{
    for ( int i = 0; i < 10; i++ )
    {
        //
    }
}
//Not Good:
if(showResult==true)
{
    for(int
    i= 0;i<10;i++)
    {
        //
    }
}
  • Use #region to group related pieces of code together. If you use proper grouping using #region, the page should be like this when all definitions are collapsed.

Use this only when the number of methods in a file is too much.

  • The ordering of methods in a class file should follow the sequence – public, internal, protected internal, protected, private.

  • There should not be unnecessary “using” statements and all using statements at the top of the file should be arranged alphabetically. Do - Organise using “Remove and sort using” – that will remove all unused namespaces as well as sort them.

  1. Good Programming practices
  • Avoid writing very long methods. A method should typically have 1 to 25 lines of code. If a method has more than 25 lines of code, you must consider re-factoring into separate methods. The Code Maid Spade complexity rating for all methods should ideally be kept to 5 or under, although a rating of up to 10 is acceptable where multiple related conditional statements are required (e.g. a switch statement with many possible outcomes). This requires the CodeMaid Visual Studio Extension to be installed on all developer machines

  • Avoid explicit properties that do nothing except access a member variable. Use automatic properties instead:

//Avoid
public class Employee
{
    string _name;
    public string Name
    {
        get
        {
            return _name;
        }
        set
        {
            _name = value;
        }
    }
}
//Correct
public class Employee
{
    public string Name { get; set; }
}
  • The method name should tell what it does. Do not mislead names. If the method name is obvious, there is no need for documentation explaining what the method does.
//Good:
void SavePhoneNumber ( string phoneNumber )
{
    // Save the phone number.
}
//Not Good:
// This method will save the phone number.
void SaveDetails ( string phoneNumber )
{
    // Save the phone number.
}
  • Avoid providing explicit values for enums unless they are integer powers of 2:
//Correct
public enum Color
{
    Red,
    Green,
    Blue
}
//Avoid
public enum Color
{
    Red = 1,
    Green = 2,
    Blue = 3
}
  • Always use a curly brace scope in an if/for/conditional construct statement, even if it conditions a single statement.

  • Avoid function calls in Boolean conditional statements if the output of that function is to be reused again in the same scope. Assign into local variables and check on them.

bool IsEverythingOK()
{....}
//Avoid:
if(IsEverythingOK())
{....}
//Correct:
bool ok = IsEverythingOK();
if(ok)
{....}
  • But if the output of the function is not needed again in the same scope –we can we do not need to store the output in a variable. In this case, the calling function is the same as is ok.
if (IsEverythingOK())
{
}
  • A method should do only 'one job'. Do not combine more than one job in a single method, even if those jobs are very small.
//Good:
// Save the address.
SaveAddress ( address );
// Send an email to the supervisor to inform that the address is updated.
SendEmail ( address, email );
void SaveAddress ( string address )
{
    // Save the address.
    // ...
}
void SendEmail ( string address, string email )
{
    // Send an email to inform the supervisor that the address is changed.
    // ...
}
//Not Good:
// Save address and send an email to the supervisor to inform that
// the address is updated.
SaveAddress ( address, email );
void SaveAddress ( string address, string email )
{
    // Job 1.
    // Save the address.
    // ...
    // Job 2.
    // Send an email to inform the supervisor that the address is changed.
    // ...
}
  • Use the c# specific types (aliases), rather than the types defined in System namespace.
int age;
(not Int16)
string name; (not String)
object contactInfo; (not Object)
  • Always watch for unexpected values. For example, if you are using a parameter with 2 possible values, never assume that if one is not matching then the only possibility is the other value.
//Good:
If ( memberType == eMemberTypes.Registered )
{
    // Registered user… do something…
}
else if ( memberType == eMemberTypes.Guest )
{
    // Guest user... do something…
}
else
{
    // Un expected user type. Throw an exception
    //throw new Exception (“Un expected value “ + memberType.ToString()
+ “’.”)
    // If we introduce a new user type in future, we can easily find
    // the problem here.
}
//Not Good:
If ( memberType == eMemberTypes.Registered )
{
    // Registered user… do something…
}
else
{
    // Guest user... do something…
    // If we introduce another user type in future, this code will
    // fail and will not be noticed.
}
  • Do not hardcode numbers. Use constants instead. Declare a constant at the top of the file or in a separate file (if it needs to be used in many places) and use it in your code. The constant variable should be declared in Pascal's Case.
static class Constants
{
    public const double Pi = 3.14159;
    public const int SpeedOfLight = 300000; // km per sec.
}
  • However, using constants is also not recommended. You should use the constants in the config file or database so that you can change them later. Declare them as constants only if you are sure this value will never need to be changed.

  • Do not hardcode strings. Use resource files.

  • Convert strings to lowercase before comparing unless the comparison should be case-sensitive (like comparing passwords). This will ensure the string will match even if the string being compared has a different case.

if (name.ToLower() == “john” )
{
    //…
}
  • Use string.IsNullOrWhiteSpace() instead of “”
//Good:
if (string.IsNullOrWhiteSpace(name))
{
    // do something
}
//Not Good:
If ( name == “” )
{
    // do something
}
  • Avoid using member variables (except in the case of dependency injection). Declare local variables wherever necessary and pass them to other methods instead of sharing a member variable between methods. If you share a member variable between methods, it will be difficult to track which method changed the value and when. In the case of dependency injection, we can use the same variable.

  • Use an enum wherever required. Do not use numbers or strings to indicate discrete values.

//Good:
enum MailType
{
    Html,
    PlainText,
    Attachment
}
void SendMail (string message, MailType mailType)
{
    switch ( mailType )
    {
        case MailType.Html:
            // Do something
            break;
        case MailType.PlainText:
            // Do something
            break;
        case MailType.Attachment:
            // Do something
            break;
        default:
            // Do something
            break;
    }
}
//Not Good:
void SendMail (string message, string mailType)
{
    switch ( mailType )
    {
        case "Html":
            // Do something
            break;
        case "PlainText":
            // Do something
            break;
        case "Attachment":
            // Do something
            break;
        default:
            // Do something
            break;
    }
}
  • In general, try to keep the access modifier of properties and methods at a minimum level and increase the level of access modifier based on need. We should use “public” if it has to be used everywhere and use “protected” when only used to be at the derived class level.

  • The event handler should not contain the code to perform the required action. Rather call another method from the event handler.

  • Do not programmatically click a button to execute the same action you have written in the button click event. Rather, call the same method which is called by the button click event handler.

  • Never hardcode a path or drive name in code. Get the application path programmatically and use a relative path.

  • Error messages should help the user to solve the problem. Never give error messages like "Error in Application", "There is an error" etc. Instead, give specific messages like "Failed to update the database. Please make sure the login ID and password are correct."

  • When displaying error messages, in addition to telling what is wrong, the message should also tell what the user should do to solve the problem. Instead of a message like "Failed to update the database.” suggest what should the user do: "Failed to update the database. Please make sure the login ID and password are correct."

  • Show a short and friendly message to the user. But log the actual error with all possible information. This will help a lot in diagnosing problems.

  • Do not have more than one class in a single file.

  • Avoid having very large files. If a single file has more than 1000 lines of code, it is a good candidate for refactoring. Split them logically into two or more classes.

  • Avoid public methods and properties, unless they really need to be accessed from outside the class. Use “internal” if they are accessed only within the same assembly.

  • Avoid passing too many parameters to a method. If you have more than 4~5 parameters, it is a good candidate to define a class or structure.

  • If you have a method returning a collection, return an empty collection instead of null, if you have no data to return. For example, if you have a method returning an ArrayList, always return a valid ArrayList. If you have no items to return, then return a valid ArrayList with 0 items. This will make it easy for the calling application to just check for the “count” rather than doing an additional check for “null”.

  • Use the AssemblyInfo file to fill in information like version number, description, company name, copyright notice etc.

  • Make sure you have a good logging class which can be configured to log errors, warnings or traces. If you configure it to log errors, it should only log errors. But if you configure it to log traces, it should record all (errors, warnings and traces). Your log class should be written in such a way that in future you can change it easily to log to Windows Event Log, SQL Server, or Email to administrator or to a File etc without any change in any other part of the application. Use the log class extensively throughout the code to record errors, and warnings and even trace messages that can help you troubleshoot a problem.

  • If you are opening database connections, sockets, file streams etc, always close them in the final block. This will ensure that even if an exception occurs after opening the connection, it will be safely closed in the final block.

  • Declare variables as close as possible to where it is first used. Use one variable declaration per line.

  • Use StringBuilder class instead of String when you have to manipulate string objects in a loop. The String object works weirdly. NET. Each time you append a string, it is discarding the old string object and recreating a new object, which is a relatively expensive operation. Consider the following example:

  •   public string ComposeMessage (string[] lines)
      {
          string message = String.Empty;
          for (int i = 0; i < lines.Length; i++)
          {
              message += lines [i];
          }
          return message;
      }
    

    In the above example, it may look like we are just appending to the string object ‘message’. But what is happening in reality is, the string object is discarded in each iteration and recreated and appending the line to it.

  • If your loop has several iterations, then it is a good idea to use the StringBuilder class instead of the String object.

  • See the example where the String object is replaced with StringBuilder.

  •   public string ComposeMessage (string[] lines)
      {
          StringBuilder message = new StringBuilder();
          for (int i = 0; i < lines.Length; i++)
          {
              message.Append( lines[i] );
          }
          return message.ToString();
      }
    

    Never access the database from the UI pages. Always have a data layer class which performs all the database-related tasks. This will help you support or migrate to another database back end easily.

  • Separate your application into multiple assemblies. Group all independent utility classes into a separate class library. All your database-related files can be in another class library.

  • Use the below string interpolation to create a string with dynamic values.

  •   var errorMessage = $"You cannot set {contact.FirstName} {contact.LastName } to deleted
      because they have active business."
    
  1. Comments

Good and meaningful comments make code more maintainable. However,

  • Do not write comments for every line of code and every variable declared.

  • Use // or /// for comments. Avoid using /* … */

  • Write comments wherever required. But good readable code will require very less comments. If all variables and method names are meaningful, that would make the code very readable and will not need many comments.

  • Do not write comments if the code is easily understandable without comment. The drawback of having a lot of comments is, that if you change the code and forget to change the comment, it will lead to more confusion.

  • Fewer lines of comments will make the code more elegant. But if the code is not clean/readable and there are fewer comments, that is worse.

  • If you have to use some complex or weird logic for any reason, document it very well with sufficient comments.

  • If you initialize a numeric variable to a special number other than 0, -1 etc., document the reason for choosing that value.

  • The bottom line is, to write clean, readable code in such a way that it doesn't need any comments to understand.

  • Perform spelling check on comments and also make sure proper grammar and punctuation is used.

  1. Exception Handling
  • Never do a 'catch exception and do nothing'. If you hide an exception, you will never know if the exception happened or not. A lot of developers use this handy method to ignore non-significant errors. You should always try to avoid exceptions by checking all the error conditions programmatically. In any case, catching an exception and doing nothing is not allowed. In the worst case, you should log the exception and proceed.

  • In case of exceptions, give a friendly message to the user, but log the actual error with all possible details about the error, including the time it occurred, method and class name etc.

  • Always catch only the specific exception, not a generic exception.

//Good:
void ReadFromFile ( string fileName )
{
    try
    {
        // read from file.
    }
    catch (FileIOException ex)
    {
        // log error.
        // re-throw exception depending on your case.
        throw;
    }
}
//Not Good:
void ReadFromFile ( string fileName )
{
    try
    {
        // read from file.
    }
    catch (Exception ex)
    {
        // Catching general exception is bad... we will never know whether
        // it was a file error or some other error.
        // Here you are hiding an exception.
        // In this case no one will ever know that an exception happened.
    }
    return "";
}
  • No need to catch the general exception in all your methods. Leave it open and let the application crash. This will help you find most of the errors during the development cycle. You can have an application-level (thread-level) error handler where you can handle all general exceptions. In case of an 'unexpected general error', this error handler should catch the exception and should log the error in addition to giving a friendly message to the user before closing the application, or allowing the user to 'ignore and proceed'.

  • When you re-throw an exception, use the throw statement without specifying the original exception. This way, the original call stack is preserved.

//Good:
catch
{
    // do whatever you want to handle the exception
    throw;
}
//Not Good:
catch (Exception ex)
{
    // do whatever you want to handle the exception
    throw ex;
}
  • Do not write try-catch in all your methods. Use it only if there is a possibility that a specific exception may occur and it cannot be prevented by any other means. For example, if you want to insert a record that does not already exist in the database, you should try to select the record using the key. Some developers try to insert a record without checking if it already exists. If an exception occurs, they will assume that the record already exists. This is strictly not allowed. You should always explicitly check for errors rather than waiting for exceptions to occur. On the other hand, you should always use exception handlers while you communicate with external systems like networks, hardware devices etc. Such systems are subject to failure anytime and error checking is not usually reliable. In those cases, you should use exception handlers and try to recover from the error.

  • Do not write very large try-catch blocks. If required, write a separate try-catch for each task you perform and enclose only the specific piece of code inside the try-catch. This will help you find which piece of code generated the exception and you can give a specific error message to the user.

  • Write your custom exception classes if required in your application and inherit your custom exceptions from the base class System. Exception. Do not inherit from ApplicationException (this is outdated now).

  1. Asynchronous Programming
  • Asynchrony is useful for processing activities that are potentially blocking, such as making web requests, file I/O or database access. If such an activity is blocked within a synchronous process, the entire application must wait. In an asynchronous process, the application can continue with other work that doesn't depend on the web resource until the potentially blocking task finishes. Asynchrony improves the scalability of ASP.NET and other server-based applications by reducing the need for threads. If the application uses a dedicated thread per response and a thousand requests are being handled simultaneously, a thousand threads are needed. Asynchronous operations often don’t need to use a thread during the wait.

  • As traditional techniques for writing asynchronous applications can be complicated, making them difficult to write, debug, and maintain, BOS will leverage the asynchronous support in the .NET Framework 4.5 to implement asynchronous code. The async and await keywords in C# are the heart of async programming. By using those two keywords, resources can be used in the .NET Framework or the Windows Runtime to create an asynchronous method almost as easily as you create a synchronous method. Asynchronous methods that are defined by using async and await are referred to as async methods.

  • BOS will use async methods wherever there exists an async method in the .NET Framework that can be utilised, in particular, the following situations will be implemented using async calls:

  • Reading and writing to the database

  • Reading and writing files

  • Accessing web resources (either using internal APIs or external web services)

  • Accessing other external resources such as email services which provide support for async calls.

  • To fully utilise async methods, the entire call stack needs to support async methods from the Controller methods down through all the individual Service methods. This will ensure that whilst a particular request is being processed, control will always return up to the web server, thus allowing it to process a lot more requests with fewer threads. The following code shows an example of how an async methods in BOS should be written:

public async Task<Contact> GenerateWebLoginAsync(Contact apiContact)
{
    Contact updatedContact = null;
    using (var scope = new TransactionScope(TransactionScopeOption.Required,
        new TransactionOptions() { IsolationLevel = IsolationLevel.Serializable },
        TransactionScopeAsyncFlowOption.Enabled))
    {
        var originalContact = await ReadContactAsync(apiContact);
        var updatedContact = await AddWebLoginAsync(originalContact);
        await SaveContactAsync(originalContact, updatedContact);
        var t1 = SendWebAccountWelcomeEmailAsync(updatedContact);
        var t2 = AddWebLoginToWSIContact(updatedContact);
        await Task.WhenAll(t1, t2);
        scope.Complete();
    }
    return updatedContact
}

Whilst this has the familiar feel of a synchronous method, async methods have the following important differences:

  • The method signature has an async modifier; this allows the method to designate suspension points and allows the method to be awaited by methods that call it.

  • The return type is Task or Task; the returned task represents ongoing work. A task encapsulates information about the state of the asynchronous process and, eventually, either the final result from the process or the exception that the process raises if it doesn't succeed. Note: Async Event Handlers will have a void return type but this is an exception to the rule.

  • The method name ends in “Async”; this is a convention but should always be followed

  • The method usually includes at least one await expression; this marks a point where the method can't continue until the awaited asynchronous operation is complete. In the meantime, the method is suspended, and control returns to the method's caller.

The use of TransactionScopeAsyncFlowOption.Enabled in the TransactionScope constructor in the above example specifies that transaction flow across thread continuations is enabled. This is essential for async programming as failure to specify this will ensure the transaction fails if any operations happen on an awaited thread.

The calls to ReadContactAsync(), AddWebLoginAsync(), and SaveContactAsync() all specify the await expression, which means that the method is suspended until the async method being called completes. However, the calls SendWebAccountWelcomeEmailAsync() and AddWebLoginToWSIContact() do not specify the await expression but instead are used to set Task variables that are then awaited together by the await Task.WhenAll(t1, t2) statement. This is because the result from each of the first 3 methods is needed to perform the next call, using an await expression ensures that this is available before continuing. The last 2 method calls however are independent and can be carried out at the same time, the await Task. WhenAll(t1, t2) statement allows the code to return control to the calling method until both tasks are complete, at which point this method will continue and the transaction scope will be completed.

The parallel approach should be used wherever independent work needs to be carried out, in the above case the BOS database has already been updated, which means the calls to email the contact and synchronise the update to WSI can both safely be carried out at the same time as they will not affect each other. Other examples of where tasks may be run in parallel are when performing File I/O on multiple files within a given request and performing multiple API calls to read data from an MVC Controller (for example to get the data for all the drop-downs in a given view).

Note: It is not possible to send multiple read requests in parallel using the same context through EF, this is because the underlying database connections are not thread-safe so each database read should have the await expression specified for it.