Rules

Secret ingredients to quality software

Edit
Info

Rules to Better Code

64 Rules

What makes code "cleaner"? What makes the difference between readable code and very readable code?

It can be very painful when needing to modify a piece of code in an application that you never spec'd out or wrote. But it doesn't have to be this way. By following some of these better programming tips your code can be easily read and easily modified by any developer at any time.

  1. Do you avoid clear text email addresses in web pages?

    Clear text email addresses in web pages are very dangerous because it gives spam sender a chance to pick up your email address, which produces a lot of spam/traffic to your mail server, this will cost you money and time to fix.

    Never put clear text email addresses on web pages.

    <!--SSW Code Auditor - Ignore next line(HTML)--> 
    <a href="mailto:test@ssw.com.au">Contact Us</a>

    Bad - Using a plain email address that it will be crawled and made use of easily

    <a href="javascript:sendEmail('74657374407373772e636f6d2e6175')" onmouseover="javascript:displayStatus('74657374407373772e636f6d2e6175');return true;" onmouseout="javascript:clearStatus(); return true;">Contact Us</a>

    Good - Using an encoded email address

    Tip: To decode and encode a string you can use this page. If you use Wordpress, use the Email Encoder Bundle plugin to help you encode email addresses easily.

    We have a program called SSW CodeAuditor to check for this rule. We have a program called SSW LinkAuditor to check for this rule.

  2. Do you refactor your code and keep methods short?

    Refactoring is all about making code easier to understand and cheaper to modify without changing its behavior.

    As a rule of thumb, no method should be greater than 50 lines of code. Long-winded methods are the bane of any developer and should be avoided at all costs. Instead, a method of 50 lines or more should be broken down into smaller functions.

  3. Do you maintain separation of concerns?

    One of the major issues people had back in the day with ASP (before ASP.NET) was the prevalence of "Spaghetti Code". This mixed Reponse.Write() with actual code.

    Ideally, you should keep design and code separate - otherwise, it will be difficult to maintain your application. Try to move all data access and business logic code into separate modules.

    Bob Martin explains this best:

  4. Do you follow naming conventions?

    It's the most obvious - but naming conventions are so crucial to simpler code, it's crazy that people are so loose with them...

    For Javascript / Typescript 

    Google publishes a JavaScript style guide. For more guides, please refer to this link: Google JavaScript Style Guide

    Here are some key points:

    • Use const or let – Not var
    • Use semicolons
    • Use arrow functions
    • Use template strings
    • Use uppercase constants
    • Use single quotes

    See 13 Noteworthy Points from Google’s JavaScript Style Guide

    For C# Java

    See chapter 2: Meaningful Names Clean Code: A Handbook of Agile Software Craftsmanship

    For SQL (see Rules to Better SQL Databases)

  5. Do you use the testing stage, in the file name?

    When moving through the different stages of testing i.e. from internal testing, through to UAT, you should suffix the application name with the appropriate stage:

    StageTesting DescriptionNaming Convention
    AlphaDeveloper testing with project teamNorthwind_v2-3_alpha.exe
    BetaInternal “Test Please" testing with non-project working colleaguesNorthwind_v2-3_beta.exe
    Production e.g.When moving onto production, this naming convention is droppedNorthwind_v2-3.exe
  6. Do you remove spaces from folders and files names?

    It is not a good idea to have spaces in a folder or file name as they don't translate to URLs very well and can even cause technical problems.

    Instead of using spaces, we recommend:

    • kebab-case - using dashes between words

    Other not recommended options include:

    • CamelCase - using the first letter of each word in uppercase and the rest of the word in lowercase
    • Snake_Case - using underscores between words

    For further information, read Do you know how to name documents?

    This rule should apply to any file or folder that is on the web. This includes Azure DevOps Team Project names and SharePoint Pages.

    • extremeemailsversion1.2.doc
    • Extreme Emails version 1.2.doc

    Figure: Bad Examples - File names have spaces or dots

    • extreme-emails-v1-2.doc
    • Extreme-Emails-v1-2.doc

    Figure: Good Examples - File names have dashes instead of spaces

    • sharepoint.ssw.com.au/Training/UTSNET/Pages/UTS%20NET%20Short%20Course.aspx
    • fileserver/Shared%20Documents/Ignite%20Brisbane%20Talk.docx

    Figure: Bad Examples - File names have been published to the web with spaces so the URLs look ugly and are hard to read

    • sharepoint.ssw.com.au/Training/UTS-NET/Pages/UTS-NET-Short-Course.aspx
    • fileserver/Shared-Documents/Ignite-Brisbane-Talk.docx"

    Figure: Good Examples - File names have no spaces so are much easier to read

  7. Do you start versioning at 0.1 and change to 1.0 once approved by a client or tester?

    Software and document version numbers should be consistent and meaningful to both the developer and the user.

    Generally, version numbering should begin at 0.1. Once the project has been approved for release by the client or tester, the version number will be incremented to 1.0. The numbering after the decimal point needs to be decided on and uniform. For example, 1.1 might have many bug fixes and a minor new feature, while 1.11 might only include one minor bug fix.

  8. Tools – Do you make sure StyleCop is installed and turned on?

    You should avoid adding exclusions to StyleCop. But, if they are necessary, ensure they are agreed to by all team members and stored in a shared exclusions file that is included in the repository, eg. stylecop.json

    https://github.com/StyleCop/StyleCop

  9. Tools - Do you make sure you have Visual Studio Code Analysis turned on?

    Then avoid adding exclusions… and if you *have to* make sure any exclusions…then have each one agreed to by all the team members.

    https://marketplace.visualstudio.com/items?itemName=VisualStudioPlatformTeam.MicrosoftCodeAnalysis2017

  10. Do you know how to avoid problems in if-statements?

    Try to avoid problems in if-statements without curly brackets and just one statement which is written one line below the if-statement. Use just one line for such if-statements. If you want to add more statements later on and you could forget to add the curly brackets which may cause problems later on.

    if (ProductName == null) ProductName = string.Empty; if (ProductVersion == null)
     ProductVersion = string.Empty; if (StackTrace == null) StackTrace = string.Empty;

    Figure: Bad Example

    if (ProductName == null) 
    { 
     ProductName = string.Empty; 
    } 
    if (ProductVersion == null)
    { 
     ProductVersion = string.Empty; 
    } 
    if (StackTrace == null) 
    { 
     StackTrace = string.Empty;
    }

    Figure: Good Example

  11. Do you avoid Double-Negative Conditionals in if-statements?

    Try to avoid Double-Negative Conditionals in if-statements. Double negative conditionals are difficult to read because developers have to evaluate which is the positive state of two negatives. So always try to make a single positive when you write if-statement.

    if (!IsValid)
    {
            // handle no error
    }
    else
    {
           // handle error
    }

    Figure: Bad example

    if (IsValid)
    {
           // handle error
    }
    else
    {
           // handle no error
    }

    Figure: Good example

    if (!IsValid)
    {
           // handle error
    }

    Figure: Another good example

  12. C# Code - Do you use string literals?

    Do you know String should be @-quoted instead of using escape character for "\"?The @ symbol specifies that escape characters and line breaks should be ignored when the string is created.

    As per:  Strings

    string p2 = "\\My Documents\\My Files\\";

    Figure: Bad example - Using "\"

    string p2 = @"\My Documents\My Files\";

    Figure: Good example - Using @

  13. Do you add the Application Name in the SQL Server connection string?

    You should always add the application name to the connection string so that SQL Server will know which application is connecting, and which database is used by that application. This will also allow SQL Profiler to trace individual applications which helps you monitor performance or resolve conflicts.

    <add key="Connection" value="Integrated Security=SSPI;Persist Security Info=False;Initial Catalog=Biotrack01;Data Source=sheep;"/>

    Bad example - The connection string without Application Name

    <add key="Connection" value="Integrated Security=SSPI;Persist Security 
     Info=False;Initial Catalog=Biotrack01;Data Source=sheep; 
     Application Name=Biotracker"/> <!-- Good Code - Application Name is added in the connection string. -->

    Good example - The connection string with Application Name

  14. Do you know how to use Connection Strings?

    Accessing your application configuration and secret values is easily done via the IConfiguration interface (from Microsoft.Extensions.Configuration.Abstractions).

    However, this convenience can lead you down the path of loosely typed secret handling (everything is a string) and can cause maintenance overhead. This is bad!

    lockedBox
    Figure: Keep your secrets safe

    Luckily there is a better way to avoid these issues and we are able to consume our configuration and secrets with strongly typed classes.

    Option #1 - Using connection strings directly in code

    public class MyDataService
    {
        private async Task<string> GetCustomerDetails(CustomerDetailsQuery request)
        {
            var sql = @"SELECT * FROM CustomerDetails WHERE CustomerId = @auctionId";
            await using var db = new SqlConnection("Server=localhost;Database=Northwind;Trusted_connection=false;user id=sa;pwd=admin");
            return (await db.QueryAsync<string>(sql,
                new
                {
                    customerId = request.CustomerId
                })
                ).SingleOrDefault();
        }
    }

    Figure: Bad Example - Option #1 Connection strings do not belong in your code, anyone seeing this could access your database

    Option #2 - Using connection strings in appsettings.json

    // In appsettings.json
    {
      "ApplicationSecrets": {
        "LicenceKey": "ABCD-1234-HIJK",
        "ConnectionString": "Server=localhost;Database=Northwind;Trusted_connection=false;user id=sa;pwd=admin"
      }
    }

    Figure: Bad Example - Option #2 Connection strings do not belong in your appsettings.json either, once committed to version control they are hard to remove

    Option #3 - Referencing loosely typed connection strings

    public class MyDataService
    {
        public readonly string _connectionString;
        
        public MyDataService(IConfiguration config)
        {
            // In Production, your connection string will be read from Key Vault instead of appsettings.json
    	// Note: we are grabbing the connection string setting directly from config as a string using another magic string - this is bad!
            _connectionString = config["ApplicationSecrets:SqlConnectionString"];
        }
        
        private async Task<string> GetCustomerDetails(CustomerDetailsQuery request)
        {
            var sql = @"SELECT * FROM CustomerDetails WHERE CustomerId = @auctionId";
            await using var db = new SqlConnection(_connectionString);
            return (await db.QueryAsync<string>(sql,
                new
                {
                    customerId = request.CustomerId
                })
                ).SingleOrDefault();
        }
    }

    Figure: Bad Example - Option #3 Referencing a loosely typed connection string defined in application settings

    Option #4 - User Secrets

    An alternative to putting secrets into appsettings.json is via User Secrets

    User secrets provide you with a secret file (called secrets.json) that is stored on your local machine away from the current code repository and cannot be committed by accident.

    To access your User Secrets:

    1. Right-click your project in Visual Studio
    2. Select Manage User Secrets
    3. Edit the content of your secrets.json file
    4. Save the file

    manage user secrets
    Figure: OK Example - Option #4 User secrets (secrets.json)

    // In ApplicationSecrets.cs
    public class ApplicationSecrets
    {
        public string SqlConnectionString { get; set; }
        public string LicenseKey { get; set; }
    }

    Figure: Good Example - The strongly typed class containing application secrets

    // In Startup.cs
    // This method gets called by the runtime. Use this method to add services to the container.
    public void ConfigureServices(IServiceCollection services)
    {
        // Bind the "ApplicationSecrets" config section to an instance of the `ApplicationSecrets` class
        services.Configure<ApplicationSecrets>(Configuration.GetSection("ApplicationSecrets"));
        ...
    }

    Figure: Good Example - Binding the ApplicationSecrets section (from appsettings.json or secrets.json) to an instance of the ApplicationSecrets class

    // In MyDataService.cs
    public class MyDataService
    {
        // Note how we use a strongly typed class here to contain all our settings - this is good!
        public readonly ApplicationSecrets _settings;
        
        public MyDataService(IOptions<ApplicationSecrets> settings)
        {
            // In Production, your connection string will be read from Key Vault
            _settings = settings.Value;
        }
        
        private async Task<string> GetCustomerDetails(CustomerDetailsQuery request)
        {
            var sql = @"SELECT * FROM CustomerDetails WHERE CustomerId = @auctionId";
            await using var db = new SqlConnection(_settings.SqlConnectionString);
            return (await db.QueryAsync<string>(sql,
                new
                {
                    customerId = request.CustomerId
                })
                ).SingleOrDefault();
        }
    }

    Figure: Good Example - Consuming strongly typed application secrets

    Option #5 - Integrating Azure Key Vault into your ASP.NET Core application

    In .NET 5 we can use Azure Key Vault to securely store our connection strings away from prying eyes.

    Azure Key Vault is great for keeping your secrets secret because you can control access to the vault via Access Policies. The access policies allows you to add Users and Applications with customized permissions. Make sure you enable the System assigned identity for your App Service, this is required for adding it to Key Vault via Access Policies.

    You can integrate Key Vault directly into your ASP.NET Core application configuration. This allows you to access Key Vault secrets via IConfiguration.

    public static IHostBuilder CreateHostBuilder(string[] args) =>
    	Host.CreateDefaultBuilder(args)
    		.ConfigureWebHostDefaults(webBuilder =>
    		{
    			webBuilder
    				.UseStartup<Startup>()
    				.ConfigureAppConfiguration((context, config) =>
    				{
    					// To run the "Production" app locally, modify your launchSettings.json file
    					// -> set ASPNETCORE_ENVIRONMENT value as "Production"
    					if (context.HostingEnvironment.IsProduction())
    					{
    						IConfigurationRoot builtConfig = config.Build();
    
    						// ATTENTION:
    						//
    						// If running the app from your local dev machine (not in Azure AppService),
    						// -> use the AzureCliCredential provider.
    						// -> This means you have to log in locally via `az login` before running the app on your local machine.
    						//
    						// If running the app from Azure AppService
    						// -> use the DefaultAzureCredential provider
    						//
    						TokenCredential cred = context.HostingEnvironment.IsAzureAppService() ?
    							new DefaultAzureCredential(false) : new AzureCliCredential();
    
    						var keyvaultUri = new Uri($"https://{builtConfig["KeyVaultName"]}.vault.azure.net/");
    						var secretClient = new SecretClient(keyvaultUri, cred);
    						config.AddAzureKeyVault(secretClient, new KeyVaultSecretManager());
    					}
    				});
    		});

    Good Example - Option #5 For a complete example, refer to this sample application

    Tip: You can detect if your application is running on your local machine or on an Azure AppService by looking for the WEBSITE_SITE_NAME environment variable. If null or empty, then you are NOT running on an Azure AppService.

    public static class IWebHostEnvironmentExtensions
    {
    	public static bool IsAzureAppService(this IWebHostEnvironment env)
    	{
    		var websiteName = Environment.GetEnvironmentVariable("WEBSITE_SITE_NAME");
    		return string.IsNullOrEmpty(websiteName) is not true;
    	}
    }

    Setting up your Key Vault correctly

    In order to access the secrets in Key Vault, you (as User) or an Application must have been granted permission via a Key Vault Access Policy.

    Applications require at least the LIST and GET permissions, otherwise the Key Vault integration will fail to retrieve secrets.

    access policies
    Figure: Key Vault Access Policies - Setting permissions for Applications and/or Users

    Azure Key Vault and App Services can easily trust each other by making use of System assigned identities. Azure takes care of all the complicated logic behind the scenes for these two services to communicate with each other - reducing the complexity for application developers.

    So, make sure that your Azure App Service has the System assigned identity enabled.

    Once enabled, you can create a Key Vault Access policy to give your App Service permission to retrieve secrets from the Key Vault.

    identity
    Figure: Enabling the System assigned identity for your App Service - this is required for adding it to Key Vault via Access Policies

    Adding secrets into Key Vault is easy.

    1. Create a new secret by clicking on the Generate/Import button
    2. Provide the name
    3. Provide the secret value
    4. Click Create

    add a secret
    Figure: Creating the SqlConnectionString secret in Key Vault.

    secrets
    Figure: SqlConnectionString stored in Key Vault. Note the ApplicationSecrets section is indicated by ApplicationSecrets-- instead of ApplicationSecrets:

    As a result of storing secrets in Key Vault, your Azure App Service configuration (app settings) will be nice and clean. You should not see any fields that contain passwords or keys. Only basic configuration values.

    configuration
    Figure: Your WebApp Configuration - no passwords or secrets, just a name of the Key vault that it needs to access

    Watch SSW's William Liebenberg explain Connection Strings and Key Vault in more detail

    History of Connection Strings

    In .NET 1.1 we used to store our connection string in a configuration file like this:

    <configuration>
         <appSettings>
              <add key="ConnectionString" value ="integrated security=true;
               data source=(local);initial catalog=Northwind"/>
         </appSettings>
    </configuration>

    ...and access this connection string in code like this:

    SqlConnection sqlConn = 
    new SqlConnection(System.Configuration.ConfigurationSettings.
    AppSettings["ConnectionString"]);

    Historical example - old ASP.NET 1.1 way, untyped and prone to error

    In .NET 2.0 we used strongly typed settings classes:

    Step 1: Setup your settings in your common project. E.g. Northwind.Common

    ConnStringNET2 Settings
    Figure: Settings in Project Properties

    Step 2: Open up the generated App.config under your common project. E.g. Northwind.Common/App.config

    Step 3: Copy the content into your entry applications app.config. E.g. Northwind.WindowsUI/App.config The new setting has been updated to app.config automatically in .NET 2.0

    <configuration>
          <connectionStrings>
             <add name="Common.Properties.Settings.NorthwindConnectionString"
                  connectionString="Data Source=(local);Initial Catalog=Northwind;
                  Integrated Security=True"
                  providerName="System.Data.SqlClient" />
            </connectionStrings>
     </configuration>

    ...then you can access the connection string like this in C#:

    SqlConnection sqlConn =
     new SqlConnection(Common.Properties.Settings.Default.NorthwindConnectionString);

    Historical example - access our connection string by strongly typed generated settings class...this is no longer the best way to do it

  15. Do you store your secrets securely?

    Most systems will have variables that need to be stored securely; OpenId shared secret keys, connection strings, and API tokens to name a few.

    These secrets must not be stored in source control in plain text – it is insecure by nature, and basically means that it is sitting.

    There are many options for managing secrets in a secure way:

    Bad Practices

    Store production passwords in source control protected with the ASP.NET IIS Registration Tool

    Pros:

    • Minimal change to existing process – no need for DPAPI or a dedicated Release Management (RM) tool
    • Simple and easy to understand

    Cons:

    • Need to manually give the app pool identity ability to read the default RSA key container
    • Difficult to manage production and non-production config settings
    • Developers can easily decrypt and access the production password
    • Manual transmission of the password from the key store to the encrypted config file

    Figure: Bad practice - Overall rating: 2/10

    Use Windows Identity instead of username/ password

    Pros:

    • Minimal change to existing process – no need for DPAPI or a dedicated RM tool
    • Simple and easy to understand

    Cons:

    • Difficult to manage production and non-production config settings
    • Not generally applicable to all secured resources
    • Can hit firewall snags with Kerberos and AD ports
    • Vulnerable to DOS attacks related to password lockout policies
    • Has key-person reliance on network admin

    Figure: Bad practice - Overall rating: 4/10

    Use External Configuration Files

    Pros:

    • Simple to understand and implement

    Cons:

    • Makes setting up projects the first time very hard
    • Easy to accidentally check the external config file into source control
    • Still need DPAPI to protect the external config file
    • No clear way to manage the DevOps process for external config files

    Figure: Bad practice - Overall rating: 1/10

    Good Practices

    Use Octopus/ VSTS RM secret management, with passwords sourced from KeePass

    Pros:

    • Scalable and secure
    • General industry best practice - great for organizations of most sizes below large corporate

    Cons:

    • Password reset process is still manual
    • DPAPI still needed

    Figure: Good practice - Overall rating: 8/10

    Use Enterprise Secret Management Tool – LastPass/ Hashicorp Vault/ etc..

    Pros:

    • Enterprise grade – supports cryptographically strong passwords, auditing of secret access and dynamic secrets
    • Supports hierarchy of secrets
    • API interface for interfacing with other tools
    • Password transmission can be done without a human in the chain

    Cons:

    • More complex to install and administer
    • DPAPI still needed for config files at rest

    Figure: Good practice -  Overall rating: 8/10

    Use .NET User Secrets

    Pros:

    • Simple secret management for development environments
    • Keeps secrets out of version control

    Cons:

    • Not suitable for production environments

    Figure: Good Practice - Overall rating 8/10

    Use Azure KeyVault

    See the SSW Rewards mobile app repository for how SSW is using this in a production application: https://github.com/SSWConsulting/SSW.Rewards

    Pros:

    • Best solution for cloud (Azure) solutions
    • Enterprise grade
    • Uses industry standard best encryption
    • Dynamically cycles secrets
    • Access granted based on Azure AD permissions - no need to 'securely' share passwords with colleagues
    • Can be used to inject secrets in your CI/CD pipelines for non-cloud solutions

    Cons:

    Figure: Good Practice - Overall rating 9/10

  16. Do you always create suggestions when something is hard to do?

    One of our goals is to make the job of the developer as easy as possible. If you have to write a lot of code for something that you think you should not have to do, you should make a suggestion and add it to the relevant page.

    If you have to add a suggestion, make sure that you put the link to that suggestion into the comments of your code.

    /// <summary>
    /// base class for command implementations
    /// This is a work around as standard MVVM commands
    /// are not provided by default. 
    /// </summary>
    public class Command : ICommand
    {
     // code
    }

    Figure: Bad example - The link to the suggestion should be in the comments

    /// <summary>
    /// base class for command implementations
    /// This is a work around as standard MVVM commands
    /// are not provided by default. 
    /// </summary>
    ///
    /// <remarks>
    ///  Issue Logged here: https://github.com/SSWConsulting/SSW.Rules/issues/3
    ///</remarks>
    public class Command : ICommand
    {
     // code
    }

    Figure: Good example - When you link to a suggestion everyone can find it and vote it up

  17. Do you avoid casts and use the "as operator" instead?

    Use casts only if:a. You know 100% that you get that type backb. You want to perform a user-defined conversion

    private void AMControlMouseLeftButtonUp(object sender, MouseButtonEventArgs e)
    {
     var auc = (AMUserControl)sender; 
       
     var aucSessionId = auc.myUserControl.Tag;
     // snip snip snip
    }

    Bad example

    private void AMControlMouseLeftButtonUp(object sender, MouseButtonEventArgs e)
    {
     var auc = sender as AMUserControl; 
       
     if (auc != null)
     {
     var aucSessionId = auc.myUserControl.Tag;
     // snip snip snip
     } 
       
    }

    Good example

    More info here: http://blog.gfader.com/2010/08/avoid-type-casts-use-operator-and-check.html

  18. Do you avoid Empty code blocks?

    Empty Visual C# .NET methods consume program resources unnecessarily. Put a comment in code block if its stub for future application.Don’t add empty C# methods to your code. If you are adding one as a placeholder for future development, add a comment with a TODO.

    Also, to avoid unnecessary resource consumption, you should keep the entire method commented until it has been implemented.

    If the class implements an inherited interface method, ensure the method throws NotImplementedException.

    public class Example
     {
           public double salary()
           { 
       
           }
     }

    Figure: Bad Example - Method is empty

    public class Sample
     {
           public double salary()
           {
                   return 2500.00;
            }
     }

    Figure: Good Example - Method implements some code

    public interface IDemo
     {
           void DoSomethingUseful();
           void SomethingThatCanBeIgnored();
     }
    public class Demo : IDemo
     {
           public void DoSomethingUseful()
           {
                  // no audit issues
                 Console.WriteLine("Useful");
           }
           // audit issues 
          public void SomethingThatCanBeIgnored()
          { 
          } 
     }

    Figure: Bad Example - No Comment within empty code block

    public interface IDemo
     {
           void DoSomethingUseful();
           void SomethingThatCanBeIgnored();
     }
    public class Demo : IDemo
     {
           public void DoSomethingUseful()
           {
                  // no audit issues
                  Console.WriteLine("Useful");
           }
           // No audit issues 
           public void SomethingThatCanBeIgnored() 
           {
                  // stub for IDemo interface
           } 
     }

    Figure: Good Example - Added comment within Empty Code block method of interface class

  19. Do you avoid logic errors by using Else If?

    We see a lot of programmers doing this, they have two conditions - true and false - and they do not consider other possibilities - e.g. an empty string. Take a look at this example. We have an If statement that checks what backend database is being used.

    In the example the only expected values are "Development" and "Production".

    void Load(string environment)
    {
      if (environment == "Development")
      {
        // set Dev environment variables
      }
      else
      {
        // set Production environment variables	
      }
    }

    Figure: Bad example with If statement

    Consider later that extra environments may be added: e.g. "Staging"

    By using the above code, the wrong code will run because the above code assumes two possible situations. To avoid this problem, change the code to be defensive .g. Use an Else If statement (like below).

    Now the code will throw an exception if an unexpected value is provided.

    void Load(string environment)
    {
      if (environment == "Development")
      {
        // set Dev environment variables
      }
      else if (environment == "Production")
      {
        // set Production environment variables	
      }
      else
      {
        throw new InvalidArgumentException(environment); 
      }
    }

    Figure: Good example with If statement

  20. Do you avoid putting business logic into the presentation layer?

    Be sure you are aware of what is business logic and what isn't. Typically, looping code will be placed in the business layer. This ensures that no redundant code is written and other projects can reference this logic as well.

    private void btnOK_Click(object sender, EventArgs e)
    {
      rtbParaText.Clear();
      var query =
        from p in dc.GetTable()
        select p.ParaID;
      foreach (var result in query)
      {
        var query2 =
          from t in dc.GetTable()
          where t.ParaID == result
          select t.ParaText;
        rtbParaText.AppendText(query2.First() + "\r\n");
      }
    }

    Bad Example: A UI method mixed with business logics

    private void btnOK_Click(object sender, EventArgs e)
    {
      string paraText = Business.GetParaText();
      rtbParaText.Clear();
      rtbParaText.Add(paraText);
    }

    Good Example : Putting business logics into the business project, just call the relevant method when needed

  21. Do you avoid "UI" in event names?

    No "UI" in event names, the event raiser should be unaware of the UI in MVVM and user controlsThe handler of the event should then do something on the UI.

    private void RaiseUIUpdateBidButtonsRed()
    {
    if (UIUpdateBidButtonsRed != null)
    {
    UIUpdateBidButtonsRed();
    }
    }

    Bad Code: Avoid "UI" in event names, an event is UI un-aware

    private void RaiseSelectedLotUpdated()
    {
    if (SelectedLotUpdated != null)
    {
    SelectedLotUpdated();
    }
    }

    Good Code: We received an update on the currently selected item, change the UI correspondingly (or even better: use MVVM and data binding)

  22. Do you avoid using if-else instead of switch block?

    The .NET framework and the C# language provide two methods for conditional handling where multiple distinct values can be selected from. The switch statement is less flexible than the if-else-if tree but is generally considered to be more efficient.

    The .NET compiler generates a jump list for switch blocks, resulting in far better performance than if/else for evaluating conditions. The performance gains are negligible when the number of conditions is trivial (i.e. fewer than 5), so if the code is clearer and more maintainable using if/else blocks, then you can use your discretion. But be prepared to refactor to a switch block if the number of conditions exceeds 5.

    int DepartmentId = GetDepartmentId()
    if(DepartmentId == 1)
    {
    // do something
    }
    else if(DepartmentId == 2)
    {
    // do something #2
    }
    else if(DepartmentId == 3)
    {
    // do something #3
    }
    else if(DepartmentId == 4)
    {
    // do something #4
    }
    else if(DepartmentId == 5)
    {
    // do something #5
    }
    else 
    {
    // do something #6
    }

    Figure: Bad example of coding practice

    int DepartmentId = GetDepartmentId()
    switch(DepartmentId)
    {
    case 1:
    // do something
    break;
    case 2:
    // do something # 2
    break;
    case 3:
    // do something # 3
    break;
    case 4:
    // do something # 4
    break;
    case 1:
    // do something # 5
    break;
    case 1:
    // do something # 6
    break;
    default:
    //Do something here
    break;
    }

    Figure: Good example of coding practice which will result better performance

    Further Reading: Speed Test: Switch vs If-Else-If

  23. Do you avoid validating XML documents unnecessarily?

    Validating an XML document against a schema is expensive, and should not be done where it is not absolutely necessary. Combined with weight the XML document object, validation can cause a significant performance hit:

    • Read with XmlValidatingReader: 172203 nodes - 812 ms
    • Read with XmlTextReader: 172203 nodes - 320 ms
    • Parse using XmlDocument no validation - length 1619608 - 1052 ms
    • Parse using XmlDocument with XmlValidatingReader: length 1619608 - 1862 ms

    You can disable validation when using the XmlDocument object by passing an XmlTextReader instead of the XmlValidatingTextReader:

    XmlDocument report = new XmlDocument();
     XmlTextReader tr = new XmlTextReader(Configuration.LastReportPath);
     report.Load(tr);

    To perform validation:

    XmlDocument report = new XmlDocument();
     XmlTextReader tr = new XmlTextReader(Configuration.LastReportPath);
     XmlValidatingReader reader = new XmlValidatingReader(tr);
     report.Load(reader);

    The XSD should be distributed in the same directory as the XML file and a relative path should be used:

    <Report> <Report xmlns="LinkAuditorReport.xsd">
     ... </Report>
  24. Do you change the connection timeout to 5 seconds?

    By default, the connection timeout is 15 seconds. When it comes to testing if a connection is valid or not, 15 seconds is a long time for the user to wait. You should change the connection timeout inside your connection strings to 5 seconds.

    "Integrated Security=SSPI;Initial Catalog=SallyKnoxMedical;Data 
    Source=TUNA"

    Figure: Bad Connection String

    "Integrated Security=SSPI;Initial Catalog=SallyKnoxMedical;Data Source=TUNA;
    Connect Timeout=5"

    Figure: Good Connection String with a 5-second connection timeout

  25. Do you declare member accessibility for all classes?

    Not explicitly specifying the access type for members of a structure or class can be misleading for other developers. The default member accessibility level for classes and structs in Visual C# .NET is always private. In Visual Basic .NET, the default for classes is private, but for structs is public.

    Match MatchExpression(string input, string pattern)

    Figure: Bad - Method without member accessibility declared

    private Match MatchExpression(string input, string pattern)

    Figure: Good - Method with member accessibility declared

    Figure: Compiler warning given for not explicitly defining member access level

    We have a program called SSW Code Auditor to check for this rule.

  26. Do you do your validation with Return?

    The return statement can be very useful when used for validation filtering.Instead of a deep nested If, use Return to provide a short execution path for conditions which are invalid.

    private void AssignRightToLeft()
    {
      // Validate Right 
      if (paraRight.SelectedIndex >= 0)
      { 
        // Validate Left 
        if (paraLeft.SelectedIndex >= 0)
        {
           string paraId = paraRight.SelectedValue.ToString();
           Paragraph para = new Paragraph();
           para.MoveRight(paraId);
           RefreshData();
        }
      }
    }

    Figure: Bad example - using nested if for validation

    private void AssignRightToLeft()
    {
      // Validate Right 
      if (paraRight.SelectedIndex >= 0)
      {
        return; 
      }
      
      // Validate Left 
      if (paraLeft.SelectedIndex >= 0)
      {
        return;
      }
    
      string paraId = paraRight.SelectedValue.ToString();
      Paragraph para = new Paragraph();
      para.MoveRight(paraId);
      RefreshData();
    }

    Figure: Good example - using Return to exit early if invalid

  27. Do you expose events as events?

    You should expose events as events.

    public Action
    < connectioninformation > ConnectionProblem;

    Bad code

    public event Action
    < connectioninformation > ConnectionProblem;

    Good code

  28. Do you follow boy scout rule?

    The idea is simple - each time leave code a little cleaner than you found it.

    Common examples are:

    1. In VS Code, open Code Analysis, and enable one rule
    2. In VS Code, open StyleCop and enable one rule
    3. Get Code Auditor to zero

    For more info, read The Boy Scout Rule ~Robert C. Martin (Uncle Bob)

  29. Do you follow naming conventions for your Boolean Property?

    Boolean Properties must be prefixed by a verb. Verbs like "Supports", "Allow", "Accept", "Use" should be valid. Also properties like "Visible", "Available" should be accepted (maybe not). Here is how we name Boolean columns in SQL databases.

    public bool Enable { get; set; }
    public bool Invoice { get; set; }

    Bad Example

    public bool Enabled { get; set; }
    public bool IsInvoiceSent { get; set; }

    Good Example - Naming Convention for Boolean Property

    We have a program called SSW Code Auditor to check for this rule.

  30. Do you format "Environment.NewLine" at the end of a line?

    You should format "Environment.NewLine" at the end of a line.

    string message = "The database is not valid." + Environment.NewLine + "Do you want to upgrade it? ";

    Bad Example: "Environment.NewLine" isn't at the end of the line

    string message = "The database is not valid." + Environment.NewLine;
    message += "Do you want to upgrade it? ";

    Good Example:  "Environment.NewLine" is at the end of the line

    return string.Join(Environment.NewLine, paragraphs);

    Good Example: "Environment.NewLine" is an exception for String.Join

  31. Do you have the time taken in the status bar?

    This feature is Particularly important if the user runs a semi-long task (e.g.30 seconds) once a day. Only at the end of the long process can they know the particular amount of time, if the time taken dialog is shown after the finish. If the status bar contains the time taken and the progress bar contains the progress percentage, they can evaluate how long it will take according to the time taken and percentage. Then they can switch to other work or go get a cup of coffee.

    Also a developer, you can use it to know if a piece of code you have modified has increased the performance of the task or hindered it.

    TimeTaken Bad
    Figure: Bad example - popup dialog at the end of a long process

    TimeTaken Good
    Figure: Good example - show time taken in the status bar

  32. Do you import namespaces and shorten the references?

    You should import namespaces and shorten the references.

    System.Text.StringBuilder myStringBuilder = new System.Text.StringBuilder();

    Figure: Bad code - Long reference to object name

    using System.Text;
    ...
    ...
    StringBuilder myStringBuilder = new StringBuilder();

    Figure: Good code - Import the namespace and remove the repeated System.Text reference

    If you have ReSharper installed, you can let ReSharper take care of this for you:

    Figure: Right click and select "Reformat Code..."

    Figure: Make sure "Shorten references" is checked and click "Reformat"

  33. Do you initialize variables outside of the try block?

    You should initialize variables outside of the try block.

    Cursor cur;
    try
    {
    ...
    cur = Cursor.Current; //Bad Code - initializing the variable inside the try block
    Cursor.Current = Cursors.WaitCursor;
    ...
    }
    finally
    {
    Cursor.Current = cur;
    }

    Bad Example: Because of the initializing code inside the try block. If it failed on this line then you will get a NullReferenceException in Finally

    Cursor cur = Cursor.Current; //Good Code - initializing the variable outside the try block
    try
    {
    ...
    Cursor.Current = Cursors.WaitCursor;
    ...
    }
    finally
    {
    Cursor.Current = cur;
    }

    Good Example : Because the initializing code is outside the try block

  34. Do you know that Enum types should not be suffixed with the word "Enum"?

    This is against the .NET Object Naming Conventions and inconsistent with the framework.

    Public Enum ProjectLanguageEnum CSharp VisualBasic End Enum

    Bad - Enum type is suffixed with the word "Enum"

    Public Enum ProjectLanguage CSharp VisualBasic End Enum

    Good - Enum type is not suffixed with the word "Enum"

    We have a program called SSW Code Auditor to check for this rule.

  35. Do you know the right way to define a connection string?

    The bad practice below because the application can now do anything it wants to the SQL server (e.g. DROP other databases).

    Server=DRAGON;Database=SSWData2005;Uid=sa;Pwd=password;

    Bad example - The connection string use 'sa' in Uid

    If using SQL Authentication
    Server=DRAGON;Database=SSWData2005;Uid=SSWWebsite;Pwd=password;Application Name=SSWWebsite 
    If using Windows Authentication (Recommended)
    Server=DRAGON;Database=SSWData2005;Integrated Security=True;Application Name=SSWWebsite

     Good example - The connection string with Application Name

    • Application Name (e.g. SSWWebsite)

      • This makes profiling the database easier as you can filter by Application Name
    • Application Specific Login/Windows Integrated security with a Domain Account for the application (e.g. SSWWebsite)

      • These logins should only have access to the databases they use (e.g. SSWData2005)
  36. Do you know what to do with a work around?

    If you have to use a workaround you should always comment your code.

    In your code add comments with:

    1. The pain - In the code add a URL to the existing resource your are following e.g. a blog post
    2. The potential solution - Search for a suggestion on the product websie. If there isn't one, create a suggestion to the product team that points to the resource. e.g. on https://uservoice.com/ or https://bettersoftwaresuggestions.com/

    "This is a workaround as per the suggestion [URL]"

    Figure: Always add a URL to the suggestion that you are compensating for

    Exercise: Understand commenting

    You have just added a grid that auto updates, but you need to disable all the timers when you click the edit button. You have found an article on Code Project (http://www.codeproject.com/Articles/39194/Disable-a-timer-at-every-level-of-your-ASP-NET-con.aspx) and you have added the work around.

    Now what do you do?

    protected override void OnPreLoad(EventArgs e)
    {
         //Fix for pages that allow edit in grids
         this.Controls.ForEach(c =>
         {   
              if (c is System.Web.UI.Timer)
              {
                  c.Enabled = false;
              }
         });
         base.OnPreLoad(e);
    }

    Figure: Work around code in the Page Render looks good. The code is done, something is missing

  37. Do you know when to use named parameters?

    Named parameters have always been there for VB developers and in C# 4.0, C# developers finally get this feature.

    You should use named parameters under these scenarios:

    • When there are 4 or more parameters
    • When you make use of optional parameters
    • If it makes more sense to order the parameters a certain way
  38. Do you know where to store your application's files?

    Although many have differing opinions on this matter, Windows applications have standard storage locations for their files, whether they're settings or user data. Some will disagree with those standards, but it's safe to say that following it regardless will give users a more consistent and straightforward computing experience.

    The following grid shows where application files should be placed:

    store files

    Further Information

    • The System.Environment class provides the most general way of retrieving those paths
    • The Application class lives in the System.Windows.Form namespace, which indicates it should only be used for WinForm applications. Other types of applications such as Console and WebForm applications use their corresponding utility classes

    Microsoft's write-up on this subject can be found at Microsoft API and reference catalog.

  39. Do you name your events properly?

    Events should end in "ing" or "ed".

    public event Action
    < connectioninformation > ConnectionProblem;

    Bad code

    public event Action
    < connectioninformation > ConnectionProblemDetected;

    Good code

  40. Do you pre-format your time strings before using TimeSpan.Parse()?

    TimeSpan.Parse() constructs a Timespan from a time indicated by a specified string. The acceptable parameters for this function are in the format "d.hh:mm" where "d" is the number of days (it is optional), "hh" is hours and is between 0 and 23 and "mm" is minutes and is between 0 and 59. If you try to pass, as a parameter, as a string such as "45:30" (meaning 45 hours and 30 minutes), TimeSpan.Parse() function will crash. (The exact exception received is: "System.OverflowException: TimeSpan overflowed because duration is too long".) Therefore it is recommended that you should always pre-parse the time string before passing it to the "TimeSpan.Parse()" function. This pre-parsing is done by the FormatTimeSpanString( ) function. This function will format the input string correctly. Therefore, a time string of value "45:30" will be converted to "1.21:30" (meaning 1 day, 21 hours and 30 minutes). This format is perfectly acceptable for TimeSpan.Parse() function and it will not crash.

    ts = TimeSpan.Parse(cboMyComboBox.Text)

    Figure: Bad code because a value greater than 24hours will crash eg. 45:30

    ts = TimeSpan.Parse(FormatTimeSpanString(cboMyComboBox.Text))

    Figure: Good code because we are using a wrapper method to pre-parse the string containing the  TimeSpan value. (Look it up in CodeBase)

    We have a program called SSW Code Auditor to check for this rule.

  41. Do you know not to put Exit Sub before End Sub? (VB)

    Do not put "Exit Sub" statements before the "End Sub". The function will end on "End Sub". "Exit Sub" is serving no real purpose here.

    Private Sub SomeSubroutine()
    'Your code here....
    Exit Sub ' Bad code - Writing Exit Sub before End Sub.
    End Sub

    Bad example

    Private Sub SomeOtherSubroutine()
    'Your code here....
    End Sub

    Good example

    We have a program called SSW Code Auditor to check for this rule.

  42. Do you put optional parameters at the end?

    Optional parameters should be placed at the end of the method signature as optional ones tend to be less important. You should put the important parameters first.

    public void SaveUserProfile(
    [Optional] string username,
    [Optional] string password,
    string firstName,
    string lastName, 
    [Optional] DateTime? birthDate)

    Figure: Bad Example - Username and Password are optional and first - they are less important than firstName and lastName and should be put at the end

    public void SaveUserProfile(
    string firstName,
    string lastName, 
    [Optional] string username,
    [Optional] string password,
    [Optional] DateTime? birthDate)

    Figure: Good Example - All the optional parameters are the end

    Note: When using optional parameters, please be sure to use named para meters

  43. Do you refer to form controls directly?

    When programming in form based environments one thing to remember is not to refer to form controls directly. The correct way is to pass the controls values that you need through parameters. There are a number of benefits for doing this:

    1. Debugging is simpler because all your parameters are in one place
    2. If for some reason you need to change the control's name then you only have to change it in one place
    3. The fact that nothing in your function is dependant on outside controls means you could very easily reuse your code in other areas without too many problems re-connecting the parameters being passed in

    It's a correct method of programming.

    Private Sub Command0_Click()
     CreateSchedule
    End Sub
    Sub CreateSchedule()
     Dim dteDateStart As Date
     Dim dteDateEnd As Date
     dteDateStart = Format(Me.ctlDateStart,"dd/mm/yyyy") 'Bad Code - refering the form control directly
     dteDateEnd = Format(Me.ctlDateEnd, "dd/mm/yyyy")
     .....processing code
    End Sub

    Bad Example

    Private Sub Command0_Click()
     CreateSchedule(ctlDateStart, ctlDateEnd)
    End Sub
    Sub CreateSchedule(pdteDateStart As Date, pdteDateEnd As Date)
     Dim dteDateStart As Date
     Dim dteDateEnd As Date
     dteDateStart = Format(pdteDateStart, "dd/mm/yyyy") 'Good Code - refering the parameter directly
     dteDateEnd = Format(pdteDateEnd, "dd/mm/yyyy")
     &....processing code
    End Sub

    Good Example

  44. Do you know how to format your MessageBox code?

    You should always write each parameter of MessageBox in a separate line. So it will be more clear to read in the code. Format your message text in code as you want to see on the screen.

    Private Sub ShowMyMessage()
     MessageBox.Show("Are
     you sure you want to delete the team project """ + strProjectName
     + """?" + Environment.NewLine + Environment.NewLine + "Warning:
     Deleting a team project cannot be undone.", strProductName + "
     " + strVersion(), MessageBoxButtons.YesNo, MessageBoxIcon.Warning, MessageBoxDefaultButton.Button2)

    Figure: Bad example of MessageBox code format

    Private Sub ShowMyMessage()
     MessageBox.Show( _ 
     "Are you sure you want to delete the team project """ + strProjectName + """?"
     _ + Environment.NewLine _ +
     Environment.NewLine _ +
     "Warning: Deleting a team project cannot be undone.", _
     strProductName + " " + strVersion(), _
     MessageBoxButtons.YesNo, _
     MessageBoxIcon.Warning, _
     MessageBoxDefaultButton.Button2)
    End Sub

    Figure: Good example of MessageBox code format

  45. Do you reference websites when you implement something you found on Google?

    If you end up using someone else's code, or even idea, that you found online, make sure you add a reference to this in your source code. There is a good chance that you or your team will revisit the website. And of course, it never hurts to tip your hat, to thank other coders.

    private void HideToSystemTray()
    {
           // Hide the windows form in the system tray
           if (FormWindowState.Minimized == WindowState)
           { 
                  Hide();
           } 
    }

    Bad Example: The website where the solution was found IS NOT referenced in the comments

    private void HideToSystemTray()
    {
            // Hide the windows form in the system tray
           // I found this solution at http://www.developer.com/net/csharp/article.php/3336751
           if (FormWindowState.Minimized == WindowState)
           { 
                  Hide();
           } 
    }

    Good Example: The website where the solution was found is referenced in the comments

  46. Do you store Application-Level Settings in your database rather than configuration files when possible?

    For database applications, it is best to keep application-level values (apart from connection strings) from this in the database rather than in the web.config.  There are some merits as following:

    1. It can be updated easily with normal SQL e.g. Rolling over the current setting to a new value.
    2. It can be used in joins and in other queries easily without the need to pass in parameters.
    3. It can be used to update settings and affect the other applications based on the same database.
  47. Do you suffix unit test classes with "Tests"?

    Unit test classes should be suffixed with the word "Tests" for better coding readability.

    [TestFixture] public class SqlValidatorReportTest { }

    Bad - Unit test class is not suffixed with "Tests"

    [TestFixture] public class HtmlDocumentTests { }

    Good - Unit test class is suffixed with "Tests"

    We have a program called SSW Code Auditor to check for this rule.

  48. Do you use a helper extension method to raise events?

    Enter Intro Text

    Instead of:

    private void RaiseUpdateOnExistingLotReceived()
    {
    if (ExistingLotUpdated != null)
    {
    ExistingLotUpdated();
    }
    }

    ...use this event extension method:

    public static void Raise<t>(this EventHandler<t> @event,
    object sender, T args) where T : EventArgs
    {
    var temp = @event;
    if (temp != null)
    {
    temp(sender, args);
    }
    }
    public static void Raise(this Action @event)
    {
    var temp = @event;
    if (temp != null)
    {
    temp();
    }
    }

    That means that instead of calling:

    RaiseExistingLotUpdated();

    ...you can do:

    ExistingLotUpdated.Raise();

    Less code = less code to maintain = less code to be blamed for ;)

  49. Do you use a regular expression to validate an email address?

    A regex is the best way to verify an email address.

    public bool IsValidEmail(string email)
    {
     // Return true if it is in valid email format.
     if (email.IndexOf("@") <= 0) return false; 
     if (email.EndWith("@")) return false; 
     if (email.IndexOf(".") <= 0) return false; 
     if ( ... 
    }

    Figure: Bad example of verify email address

    public bool IsValidEmail(string email) 
    { 
     // Return true if it is in valid email format.
     return System.Text.RegularExpressions.Regex.IsMatch( email, 
     @"^([\w-\.]+)@(([[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.)|(([\w-]+\.)+))([a-zA-Z]{2,4}|[0-9]{1,3})(\]?)$";
    }

    Figure: Good example of verify email address

  50. Do you use a regular expression to validate an URL?

    A regex is the best way to verify an URI.

    public bool IsValidUri(string uri)
    {
    try 
    
    Uri testUri = new Uri(uri); 
    return true; 
    
    catch (UriFormatException ex)
    
    return false; 
    
    }

    Figure: Bad example of verifying URI

    public bool IsValidUri(string uri) 
    
    // Return true if it is in valid Uri format.
    return System.Text.RegularExpressions.Regex.IsMatch( uri,@"^(http|ftp|https)://([^\/][\w-/:]+\.?)+([\w- ./?/:/;/\%&=]+)?(/[\w- ./?/:/;/\%&=]*)?"); 
    }

    Figure: Good example of verifying URI 

    You should have unit tests for it, see our Rules to Better Unit Tests for more information.

  51. Do you use Enums instead of hard coded strings?

    Use Enums instead of hard-coded strings, it makes your code lot cleaner and is really easy to manage .

    EnumBadExample
    Figure: Bad example - "Hard- coded string" works, but is a bad idea

    EnumGoodExample
    Figure: Good example - Used Enums, looks good and is easy to manage

  52. Do you use Environment.NewLine to make a new line in your string?

    When you need to create a new line in your string, make sure you use Environment.NewLine, and then literally begin typing your code on a new line for readability purposes.

    string strExample = "This is a very long string that is \r\n not properly implementing a new line.";

    Bad Example: The string has implemented a manual carriage return line feed pair (\r\n)

    string strExample = "This is a very long string that is " + Environment.NewLine +
    		 " properly implementing a new line.";

    OK Example: The new line is created with Enviroment.NewLine (but strings are immutable)

    var example = new StringBuilder();
    
    example.AppendLine("This is a very long string that is ");
    
    example.Append(" properly implementing a new line.");

    Good Example: The new line is created by the StringBuilder and has better memory utilisation

  53. Do you use good code over backward compatibility?

    Supporting old operating systems and old versions means you have more (and often messy) code, with lots of if or switch statements. This might be OK for you because you wrote the code, but down the track when someone else is maintaining it, then there is more time/expense needed.

    When you realize there is a better way to do something, then you will change it, clean code should be the goal, however, because this affects old users, and changing interfaces at every whim also means an expense for all the apps that break, the decision isn't so easy to make.

    Our views on backward compatibility start with asking these questions:

    • Question 1: How many apps are we going to break externally?
    • Question 2: How many apps are we going to break internally?
    • Question 3: What is the cost of providing backward compatibility and repairing (and test) all the broken apps?

    Let's look at an example:

    If we change the URL of this public Web Service, we'd have to answer the questions as follows:

    • Answer 1: Externally - Don't know, we have some leads: We can look at web stats and get an idea.  If an IP address enters our website at this point, it tells us that possibly an application is using it and the user isn't just following the links.
    • Answer 2: Website samples + Adams code demo
    • Answer 3: Can add a redirect or change the page to output a warning Old URL. Please see www.ssw.com.au/ PostCodeWebService for new URL

    Because we know that not many external clients use this example, we decide to remove the old web service after some time.

    Just to be friendly, we would send an email for the first month, and then another email in the second month.  After that, just emit "This is deprecated (old)."  We'll also need to update the UDDI so people don't keep coming to our old address.

    We probably all prefer working on new features, rather than supporting old code, but it’s still a core part of the job. If your answer to question 3 scares you, it might be time to consider a backward compatibility warning.

    Figure: Good Example - Email as a backward compatibility warning

  54. Do you use Public/Protected Properties instead of Public/Protected Fields?

    Public/Protected properties have a number of advantages over public/protected fields:

    • Data validation Data validation can be performed in the get/set accessors of a public property. This is especially important when working with the Visual Studio .NET Designer.
    • Increased flexibility Properties conceal the data storage mechanism from the user, resulting in less broken code when the class is upgraded. Properties are a recommended object-oriented practice for this reason.
    • Compatibility with data binding You can only bind to a public property, not a field.
    • Minimal performance overhead The performance overhead for public properties is trivial. In some situations, public fields can actually have inferior performance to public properties.
    public int Count;

    Figure: Bad code. Variable declared as a Field

    public int Count
    {
     get
     {
     return _count;
     }
     set
     {
     _count = value; 
     }
    }

    Figure: Good code. Variable declared as a Property

    We agree that the syntax is tedious and think Microsoft should improve this.

  55. Do you use resource file to store all the messages and globlal strings?

    Storing all the messages and global strings in one place will make it easy to manage them and to keep the applications in the same style.

    Code StoreMessage
    Store messages in the Message.resx

    Catch(SqlNullValueException sqlex)
    {
    Response.Write("The value cannot be null.");
    }

    Bad Example - if you want to change the message, it will cost you lots of time to investigate every try-catch block

    Catch(SqlNullValueException sqlex)
    {
    Response.Write(GetGlobalResourceObject("Messages", "SqlValueNotNull"));
    }

    Better Example - better than the hard code, but still wordy

    Catch(SqlNullValueException sqlex)
    {
    Response.Write(Resources.Messages.SqlValueNotNull); 'Good Code - storing message in resource file. 
    }

    Good Example

  56. Do you use resource file to store messages?

    All messages are stored in one central place so it's easy to reuse. Furthermore, it is strongly typed - easy to type with IntelliSense in Visual Studio.

    Module Startup Dim HelloWorld As String = "Hello World!" Sub Main() Console.Write(HelloWorld)Console.Read() End Sub End Module

    Bad example of a constant message

    Figure: Saving constant message in Resource

    Module Startup Sub Main() Console.Write(My.Resources.Messages.Constant_HelloWorld) Console.Read() End Sub End Module

    Good example of a constant message

  57. Do you use String.Empty instead of ""?

    Considering the memory management of .NET Framework String.Empty will get higher performance then using "".

    public string myString 
       
    {
     get
     {
     return ;
     } 
       
    }

    Figure: Bad code. Low performance

    public string myString
    { 
       
     get 
       
     { 
       
     return string.Empty; 
       
     } 
       
    }

    Figure: Good code. Higher performance

    We have a program called SSW Code Auditor to check for this rule.

  58. Do you use "using" declaration instead of use explicitly "dispose"?

    Don't explicitly use "dispose" to close objects and dispose of them, the "using" statement will do all of them for you. It is another awesome tool that helps reduce coding effort and possible issues.

    static int WriteLinesToFile(IEnumerable<string> lines)
    {
      // We must declare the variable outside of the using block
      // so that it is in scope to be returned.
      int skippedLines = 0;
      var file = new System.IO.StreamWriter("WriteLines2.txt")
      foreach (string line in lines)
      {
        if (!line.Contains("Second"))
        {
          file.WriteLine(line);
        }
        else
        {
          skippedLines++;
        }
      }
      file.Dispose();
      return skippedLines;
    }

    Figure: Bad example of dispose of resources

    static int WriteLinesToFile(IEnumerable<string> lines)
    {
      // We must declare the variable outside of the using block
      // so that it is in scope to be returned.
      int skippedLines = 0;
      using (var file = new System.IO.StreamWriter("WriteLines2.txt"))
      {
        foreach (string line in lines)
        {
          if (!line.Contains("Second"))
          {
            file.WriteLine(line);
          }
          else
          {
            skippedLines++;
          }
        }
      } // file is disposed here
       return skippedLines;
    }

    Figure: Bad example of dispose of resources

    static int WriteLinesToFile(IEnumerable<string> lines)
    {
      using var file = new System.IO.StreamWriter("WriteLines2.txt");
      // Notice how we declare skippedLines after the using statement.
      int skippedLines = 0;
      foreach (string line in lines)
      {
        if (!line.Contains("Second"))
        {
          file.WriteLine(line);
        }
        else
        {
          skippedLines++;
         }
        }
        // Notice how skippedLines is in scope here.
        return skippedLines;
       // file is disposed here
    }

    Figure: Good example of dispose of resources, using c# 8.0 using declaration

    Tip: Did you know it is not recommended to dispose HttpClient?

    One last note is regarding disposing of HttpClient.  Yes, HTTPClient does implement IDisposable, however, I do not recommend creating a HttpClient inside a Using block to make a single request.  When HttpClient is disposed it causes the underlying connection to be closed also.  This means the next request has to re-open that connection.  You should try and re-use your HttpClient instances.  If the server really does not want you holding open it’s connection then it will send a header to request the connection be closed.http://www.bizcoder.com/httpclient-it-lives-and-it-is-glorious

  59. Do you warn users before starting a long process?

    You should never start a long process (>30 seconds) without first giving a warning message to warn the user approximately how long it will take.

    lengthyoperation
    Figure: Good example - Code Auditor message warning this is a long process

    You will need to have 2 things:

    1. A table to record processes containing the following fields:

      • ALogRecord (DateCreated, FunctionName, EmpUpdated, ComputerName, ActiveForm, ActiveControl, SystemsResources, ConventionalMemory, FormsCount, TimeStart, TimeEnd, TimeTaken, RecordsProcessed, Avg, Note, RowGuide, SSWTimeStamp)
    2. A function to change the number of seconds lapsed to words - see the "1 minute, 9 seconds" in the above messagebox - this requires a SecondsToWords() function shown. See our code base.
  60. Do you wrap the same logic in a method instead of writing it again and again whenever it's used?

    Is your code DRY? Any logic that is used more than once, should be encapsulated in a method, and the method called wherever it is needed.

    This will reduce redundancy, decrease maintenance effort, improve efficiency and reusability, and make the code more clear to read.

    public class WarningEmail
    {
    //...
    public void SendWarningEmail(string pFrom, string pTo, string pCC, string pUser, string pPwd, string pDomain)
    {
    //...
    MailMessage sMessage = new MailMessage();
    sMessage.From = new MailAddress(pFrom);
    sMessage.To.Add(pTo);
    sMessage.CC.Add(pCC);
    sMessage.Subject = "This is a Warning";
    sMessage.Body = GetWarning();
    SmtpClient sSmtpClient = new SmtpClient();
    sSmtpClient.Credentials = new NetworkCredential(pUser, pPwd, pDomain);
    sSmtpClient.Send(sMessage);
    //...
    }
    }
    public class ErrorEmail
    {
    public void SendErrorEmail(string pFrom, string pTo, string pCC, string pUser, string pPwd, string pDomain)
    {
    //...
    MailMessage sMessage = new MailMessage();
    sMessage.From = new MailAddress(pFrom);
    sMessage.To.Add(pTo);
    sMessage.CC.Add(pCC);
    sMessage.Subject = "This is a Error";
    sMessage.Body = GetError();
    SmtpClient sSmtpClient = new SmtpClient();
    sSmtpClient.Credentials = new NetworkCredential(pUser, pPwd, pDomain);
    sSmtpClient.Send(sMessage);
    //...
    }
    }

    Bad Example: Write the same logic repeatedly

    public class WarningEmail
    {
    //...
    public void SendWarningEmail(string pFrom, string pTo, string pCC, string pUser, string pPwd, string pDomain)
    {
    //...
    EmailHelper.SendEmail(pFrom, pTo, pCC, "This is a Warning", GetWarning(), pUser, pPwd, pDomain);
    //...
    }
    }
    public class ErrorEmail
    {
    public void SendErrorEmail(string pFrom, string pTo, string pCC, string pUser, string pPwd, string pDomain)
    {
    //...
    EmailHelper.SendEmail(pFrom, pTo, pCC, "This is an Error", GetError(), pUser, pPwd, pDomain);
    //...
    }
    }
    public class EmailHelper
    { 
    public static void SendEmail(string pFrom, string pTo, string pCC, string pSubject, string pBody, string pUser, string pPwd, string pDomain)
    {
    MailMessage sMessage = new MailMessage();
    sMessage.From = new MailAddress(pFrom);
    sMessage.To.Add(pTo);
    sMessage.CC.Add(pCC);
    sMessage.Subject = pSubject;
    sMessage.Body = pBody;
    SmtpClient sSmtpClient = new SmtpClient();
    sSmtpClient.Credentials = new NetworkCredential(pUser, pPwd, pDomain);
    sSmtpClient.Send(sMessage);
    } 
    }

    Good Example: Put the same logic in a method and make it reusable

  61. Use Enum Constants instead of Magic numbers?

    Using "Magic numbers" in your code makes it confusing and really hard to maintain.

    MagicNumberBad
    Figure: Bad example - "Magic Number" works, but is a bad idea

    MagicNumberGood
    Figure: Good example - No Magic Number, looks good and is easy to manage

  62. Do you avoid using magic string when referencing property/variable names

    Hard coded strings when referencing property and variable names can be problematic as your codebase evolves, and can make your code brittle.

    (if customer.Address.ZipCode == null) throw new ArgumentNullException("ZipCode");

    Figure: Bad Example - Hardcoding a reference to a property

    (if customer.Address.ZipCode == null) throw new ArgumentNullException(nameof(customer.Address.ZipCode));

    Figure: Good Example - Using nameof() operator to avoid hardcoded strings

  63. Do you use null condition operators when getting values from objects

    Null-conditional operators - makes checking for null as easy as inserting a single question mark. The Null-conditional operators feature boils down all of the previously laborious clunky code into a single question mark.

    int length = customer != null && customer.name != null ? customer.name.length : 0;

    Figure: Bad Example - Verbose and complex code checking for nulls

    int length = customers?.name?.length ?? 0;

    Figure: Good Example - Robust and easier to read code

  64. Do you use string interpolation when formatting strings

    String Interpolation - greatly reduces the amount of boilerplate code required when working with stringsFormatting strings on the fly was previously a task which required a stack of boilerplate code

    var s = String.Format("Profit is ${0} this year", p.TotalEarnings - p.Totalcost);

    Figure: Bad Example - Using String.Format() makes the code difficult to read

    var s = "Profit is ${p.TotalEarnings - p.Totalcost} this year";

    Figure: Good Example - String Interpolation is very human readable

We open source. This page is on GitHub