Skip Navigation LinksHome > SSW Standards > Rules > SSW Rules to Better Code


I've been putting together Development Guidelines for my employer and in the process have reviewed many published standards (in the .Net arena) from around the world. In each category, the suggestions at SSW are always among the best. Leon Bambrick -
 

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 anytime.

Do you agree with them all? Are we missing some? Let us know what you think.

SSW Rules to Better Code

  1. Do you avoid clear text email addresses in web pages?
  2. Do you refactor your code and keep methods short?
  3. Do you separate code from design (aka use an N-tier architecture)?
  4. Do you follow naming conventions?
  5. Do you use the testing stage, in the file name?
  6. Do you remove spaces from your filename?
  7. Do you follow version conventions?
  8. Do you start versioning at 0.1 and change to 1.0 once approved by a client or tester?
  9. Do you have brand at the top of each file?
  10. Do you comment each property and method?
  11. Do you add comments for your code if it is updated?
  12. Do you create Task List Comments for your code?
  13. Do you declare variables when you need them?
  14. Do you avoid problems in if-statements?
  15. Do you avoid Double-Negative Conditionals in if-statements?
  16. Do you avoid logic errors by using Else If?
  17. Do you know where to store your application's files?
  18. Do you refer to form controls directly?
  19. What do you do with comments and Debug.Print statements?
  20. Do you avoid validating XML documents unnecessarily?
  21. Do you change the connection timeout to 5 seconds?
  22. Never start a long process (>30 seconds) without a warning or make it obvious - like the start button on Code Auditor
  23. Do you have the time taken in the status bar?
  24. Does backward compatibility kill good code?
  25. Do you put Exit Sub before End Sub?
  26. Do you follow naming conventions for your Boolean Property?
  27. Do you use Public/Protected Properties instead of Public/Protected Fields?
  28. Do you use String.Empty instead of ""?
  29. Do you pre-format your time strings before using TimeSpan.Parse()?
  30. Do you use interoperability mechanism for COM object?
  31. Do you import namespaces and shorten the references?
  32. Do you declare member accessibility for all classes?
  33. Do you do your validation with Exit Sub?
  34. Do you know how to format your MessageBox code?
  35. Do you use a regular expression to validate an email address?
  36. Do you use resource file to store messages?
  37. Do you suffix unit test classes with "Tests"?
  38. Do you know that Enum types should not be suffixed with the word "Enum"?
  39. Do you avoid using "Strings" in your code? Use "Enum"?
  40. Do you avoid using "Magic numbers" in your code? Use "Enum - Constants" instead?
  41. Do you use a regular expression to validate an Uri?
  42. Do you use "using" statement instead of use explicitly "dispose"?
  43. Do you add the Application Name in the SQL Server connection string?
  44. Do you use resource file to store all the messages and globlal strings?
  45. Do you store Application-Level Settings in your database rather than configuration files when possible?
  46. Do you always check your button's event handler hook-up?
  47. Do you initialize variables outside of the try block?
  48. Do you format "Environment.NewLine" at the end of a line?
  49. Do you add a comment when you use Thread.Sleep?
  50. Do you know the right way to define a connection string?
  51. Do you reference websites when you implement something you found on Google?
  52. Do you avoid putting business logic into the presentation layer?
  53. Do you use Environment.NewLine to make a new line in your string?
  54. Do you wrap the same logic in a method instead of writing it again and again whenever it's used?
  55. Do you know when to use named parameters?
  56. Do you put optional parameters at the end?
  57. Do you avoid casts and use the "as operator" instead?
  58. Do you expose events as events?
  59. Do you name your events property?
  60. Do you avoid "UI" in event names?
  61. Do you use a helper extension method to raise events?
  62. Do you know what to do with a work around?
  63. Do you follow boy scout rule?
  64. Do you always create suggestions when something is hard to do?
  65. Do you avoid Empty code block?
  66. Do you avoid using if-else instead of switch block
  67. Do you know String should be @-quoted instead of using escape character for "\\"?
  68. Do you know that no carriage returns without line feed?
  69. Do you know not to put Exit Sub before End Sub?
  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 address on web pages.

                
                        Contact Us
                        

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

              Contact Us
                        

    Good - Using an encoded email address

    Note: If you use Wordpress, use the Email Encoder Bundle plugin to help you encode email addresses easily.

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

    We have a program called SSW Link Auditor 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 behaviour.

    As a rule of thumb, no methods 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.

    NB: Visual Studio 2005 has refactoring tools built in.

  3. Do you separate code from design (aka use an N-tier architecture)?

    One of the major issues with ASP (versus 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. The same goes for Windows Forms, Access or any other code. Instead, try to move all data access and business logic code into separate methods - away from your GUI.

  4. Do you follow Naming Conventions?

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

  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:

    Stage Testing Description Naming Convention
    Alpha Developer testing with project team Northwind_v2-3_alpha.exe
    Beta Internal “Test Please” testing with non-project working colleagues Northwind_v2-3_beta.exe
    Production e.g. When moving onto production, this naming convention is dropped Northwind_v2-3.exe
  6. Do you remove spaces from your folders and filename?

    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, you should have the first letter of each word in UPPERCASE and the rest of the word in lowercase. Alternatively, you can choose to use underscores. This alternative method to spacing makes file names more readable when published on the web.

    Note that this rule should apply for any file or folder that is on the web. This includes TFS 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
    ExtremeEmails_v1_2.doc
    Figure – Good Examples: file names do not have spaces
    <a href="http://sharepoint.ssw.com.au/Training/UTSNET/Pages/UTS%20NET%20Short%20Course.aspx">UTS Short Course</a>
    <a href="file://fileserver/Shared%20Documents/Ignite%20Brisbane%20Talk.docx">Ignite Talk</a>
    Figure – Bad Examples: file names have been published with spaces so the URLs look ugly and are hard to read
    <a href="http://sharepoint.ssw.com.au/Training/UTSNET/Pages/UTSNETShortCourse.aspx">UTS Short Course</a>
    <a href="file://fileserver/SharedDocuments/Ignite_Brisbane_Talk.docx">Ignite Talk</a>
    Figure – Good Examples: file names have no spaces so are much easier to read
  7. Do you follow version conventions?

    It is important to follow a consistent version convention throughout a project development. This will help you keep track of changes and bug fixes. A new version should be released every day when there is a bug fixed, so testers can continue testing while development continues.

    This is how we display the version number publicly.

    VB.NET - AssemblyInfo.vb:
    'Version = "9.19"  'DDK 25/07/2003 - Application Block changes - replaced settings.
    'Version = "9.20" 'DDK 28/07/2003 - Fixed release summary path information in subject - full path now; Code review with Adam.; Views change automatically on refresh
    <Assembly: AssemblyVersion("9.10.*")>
                

    C#.NET - AssemblyInfo.cs:

    // 1.42 02/01/2002 PH - fix bugs, sample hyperlinks, ok button and rule
    // 1.43 03/01/2002 PH/JL - tool tip, fix reload new database, fix registration, setup
    // 1.44 07/01/2002 PH - fix bugs in FrmJob, report layout
    [assembly: AssemblyVersion("1.44.*")]
                
    Figure: Version Control in .NET Project

    The same principle applies to web project too.

    >

    In ASP.NET 1.x you can use the AssemblyInfo file for keeping the versions just like you do it in the Window Forms.

    ASP.NET 1 - Footer.ascx.cs
    protected void Page_Load(object sender, EventArgs e)
    {
        lblVersion.Text = System.Reflection.Assembly.GetExecutingAssembly().GetName().Version.ToString(3);
    }
                

    However, in ASP.NET 2.0 each page is built into it own assembly. So, you should add a static method to AssemblyInfo.cs or other class file in the App_Code that returns the version info, and call this method from your other pages. e.g. footer.ascx.cs.
    Otherwise it will return 0.0.0.

    ASP.NET 2 - Function.cs under App_code
    //get version number from Assembly
    //Note: this should stay in App_Code. Otherwise it will return 0.0.0.0
    public static string VersionInfo
    {
        get
        {
            return System.Reflection.Assembly.GetExecutingAssembly().GetName().Version.ToString(3);
        }
    }
    
    public static string ApplicationFullName
    {
        get
        {
            string productName = string.Empty;
            object[] atts = System.Reflection.Assembly.GetExecutingAssembly().GetCustomAttributes(false);
            foreach (object a in atts)
            {
                if (a is AssemblyProductAttribute)
                {
                    productName = ((AssemblyProductAttribute)a).Product;
                    break;
                }
            }
            return productName;
        }
    }
                
    ASP.NET 2 - Footer.ascx.cs
    protected void Page_Load(object sender, EventArgs e)
    {
        lblVersion.Text = PfastTrack.Functions.VersionInfo;
    }
                
  8. 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.

  9. Do you have brand at the top of each file?

    Brand is the summary of our company, the product and release information, and can be used by other developers to quickly know the release history and product summary.

    The brand should contain at least the following:

    • Copyright declaration
    • Purpose of the document/file
    • Author name

    If the file was modified by another developer, the comment should also contain:

    • Reviewer name

    Header comments:
    On top of the file:

    ///<summary>
    ///'----------------------------------------------
    /// Copyright 2005 Superior Software for Windows
    /// www.ssw.com.au All Rights Reserved.
    ///'----------------------------------------------
    /// Comment: User class to handle user preference and login information
    /// Authors:   DDK,PH
    /// Reviewers: AC,RD
    ///</summary>
    ///'---------------------------------------------- 
  10. Do you comment each property and method?

    It's important that you have a consistent code comment standard throughout an application, regardless of language. Therefore, other developers can quickly determine the workings of a function/sub/class/stored procedure. Ideally, code should be as simple and self-explanatory as possible. Exceptions should be noted in line, especially when there is a .NET catch statement for generic System.Exceptions (in VB6/Access - for a Resume Next statement or similar).

    E.g. catch (InteropServices.COMException ex) //Catch all COM Exceptions from third party COM component

    In JavaScript and HTML, you should put these comments between the
    <HEAD> and  </HEAD>
    tags. 

    To delmit the comments (ie top and bottom), you should use the standard block comment markers of
    <!-- and -->.

    A css file should be delimited with the block comment marks of
    /* and */.

    If the file contains any function/sub module/class declaration, comments will be containted to each of them containing at least the following:

    • function/sub module/class name
    • role of the function/sub module/class declaration

    Above a method or property declaration:
    ''' <summary>
    '''
    ''' </summary>
    ''' <param name="sender"></param>
    ''' <param name="e"></param>
    ''' <remarks ></remarks>

    The comments can be generated automatically by VS.NET
    /// - C#
    ''' - VB.NET
    Bonus - you can automatically generate documentation - but the number of clients that want this is minimal.
  11. Do you add comments for your code if it is updated?

    It's also important that you have a consistent code comment for your updating, which can be used by other developers to quickly determine the workings of the updating.

    Example of commentting a method, it is strong recommended that you add adequate comment for your updating;
                            'Private Sub iStopwatchOptionsForm_Resizing(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles MyBase.Resize
                            'If Me.WindowState = FormWindowState.Minimized Then
                            'Me.Hide()
                            'End If
                            'End Sub
                        

    Figure: Bad example in VB.NET

                            '
                            'Commented - we don't need to hide this from when it is minimum size, just leave it on taskbar.
                            ' FW, 11/10/2006
                            '
                            'Private Sub iStopwatchOptionsForm_Resizing(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles MyBase.Resize
                            'If Me.WindowState = FormWindowState.Minimized Then
                            'Me.Hide()
                            'End If
                            'End Sub
                        

    Figure: Good example in VB.NET

    Example of adding code inside a method:

                            Private Sub iStopwatchOptionsForm_Closing(ByVal sender As System.Object, ByVal e As System.ComponentModel.CancelEventArgs) Handles MyBase.Closing
                                'Don't close this form except closing this application - using hide instead;
                                If Not Me.m_isForceClose Then
                                    If Me.IsOptionsModified Then
                                        If MessageBox.Show("Do
                                        you want to save the changes?", Me.GetApplicationTitle, MessageBoxButtons.YesNo,
                                        MessageBoxIcon.Warning) = DialogResult.Yes Then
                                         Me.SaveOptions()
                                     End If
                                    End If
                                End If
                            End Sub                    
                        

    Figure: Bad example in VB.NET

                         Private Sub iStopwatchOptionsForm_Closing(ByVal sender As System.Object, ByVal e As System.ComponentModel.CancelEventArgs) Handles MyBase.Closing
                         'Don't close this form except closing this application - using hide instead; 
                         If Not Me.m_isForceClose Then
                            '<added by FW, 11/10/2006>
                            ' Remind saving the changes if the options were modified.
                             If Me.IsOptionsModified Then
                                 If MessageBox.Show("Do you want to save the changes?", Me.GetApplicationTitle, MessageBoxButtons.YesNo, MessageBoxIcon.Warning) = DialogResult.Yes Then
                                      Me.SaveOptions()
                                 End If
                             End If                    
                             '</added>
                            ...
                        End If
                        End Sub
                        

    Figure: Good example in VB.NET

  12. Do you create Task List Comments for your code?

    Task List comments can be used to indicate a variety of work to be done at the location marked, including:

    • features to be added;
    • problems to be corrected;
    • classes to implement;
    • place markers for error-handling code;
    • reminders to check in the file.

    As with other Task List entries, you can double-click any comment entry to display the file indicated in the Code Editor and jump to the line of code marked. More details for Task List comments

    Here is an example: when I open this options form, I see "ssw.com.au" in the email text box, but it is actually only half of the text in that textbox, see the following captures:

    Figure: You can see that ssw.com.au is highlighted.. and it is actually only half of the text in that textbox;


    Figure: scrolling left displays the full contents of the textbox;


    Figure: Bad example - the comment doesn't show in Task List window;

    Figure: Good example - Marked TODO in the comment, so you can see it in Task List window and double click to jump to;

    Figure: Good example - Marked HACK in the comment, so you can see it in Task List window and double click to jump to;

  13. Do you declare variables when you need them?

    Should you declare variables at the top of the function, or declare them when you need to use them? If you come back to your code after a few weeks and you no longer need a variable, you are quite likely to forget to delete the declaration at the top, leaving orphaned variables. Here at SSW, we believe that variables should be declared as they are needed.

                        Private Sub Command0_Click()
                        Dim dteTodayDate As Date
                        Dim intRoutesPerDay As Integer
                        Dim intRoutesToday As Integer
                        Dim dblWorkLoadToday As Double
                        dblWorkLoadToday = Date.Now()
                        .
                        ....many lines of code...
                        .
                        intRoutesPerDay = 2
                        End Sub
                    

    Figure: Bad example

                        Private Sub Command0_Click()
                        Dim dteTodayDate As Date
                        dteTodayDate = Date.Now()
                        .
                        ....many lines of code...
                        .
                        Dim intRoutesPerDay As Integer
                        intRoutesPerDay = 2
                        .
                        ....continuing code...
                        .End Sub                
                    

    Figure: Good Example

  14. Do you 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

  15. 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 what is positive state of two negatives. So always try to make a single positvie 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: Good as well

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

    I 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. This is being stored as a property - Backend in config file. At the moment only Access and SQL Server are options.

                        Private Sub Command0_Click()
                        If My.MySettings.Default.Backend = "Access" Then
                        'Call this code ie. SQL commands
                        Else
                        'Must be SQL Server
                        'Call this other code ie. Stored Proc
                        End If
                        
                        .....processing code
                        End Sub
                    

    Figure: Bad Example with If statement

    Consider later on this code is updated... the programmer wishes to add an Oracle backend database option. So they modify the Backend property to include Oracle...

    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).

    The user will then get a Logic Error and can report it to the programmer.

                        Private Sub Command0_Click()
                        If My.MySettings.Default.Backend = "Access" Then
                        'Call this code ie. SQL
                        ElseIf My.MySettings.Default.Backend = "SQL Server" Then
                        'Call this other code ie. Stored Proc
                        Else
                        Throw New Exception( "Logic Error -- BackEnd is: "
                        & My.MySettings.Default.Backend)
                        End If
                        End Sub

    Figure: Good Example with If statement

    When writing code to trap Logic Errors, use "Select Case" or "switch" statements to enhance readability. e.g. in VB.NET

                        Private Sub Command0_Click()
                        Select Case mDataset.Tables(0).Rows(0)("Key")
                        Case "1"
                        ' Initialize the column list
                        strTempColumn = ""
                        Case "2"
                        ' Ignore
                        End Select
                        .....processing code
                        End Sub
                    

    Figure: Bad Example with Case statement in VB.NET

                        Private Sub Command0_Click()
                        Select Case mDataset.Tables(0).Rows(0)("Key")
                        
                        Case "1"
                        ' Initialize the column list
                        strTempColumn = ""
                        Case "2"
                        ' IgnoreCase Else
                        Throw New Exception("Logic Error")
                       
                        End Select
                        .....processing code
                       
                        End Sub

    Figure: Use 'Select Case' or 'Switch' statements to enhance readability when coding to find logic errors.

  17. Do you know where to store your application's files?

    Although many have differing opinions on this matter, Windows has standard storage locations for files for application, 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:

    When you're working with ... Store the files in ... How to get path in code ...
    User created documents (default) C:\Documents and Settings\[User Name]\My Documents
                            System.Environment.GetFolderPath(System.Environment.SpecialFolder.Personal) 
                            
    Read only application files and sample data or libraries C:\Program Files\[Application Name] AppDomain.BaseDirectory method (recommend)
    The directory of the .exe which started the current AppDomain
    System.AppDomain.CurrentDomain.BaseDirectory 

    Application.StartupPath method (okay)
    It requires Windows.Forms and that is non generic for business classes

    System.Windows.Forms.Application.StartupPath 

    Environment.CurrentDirectory method (bad)
    This is the startup path of the .exe

    System.Environment.CurrentDirectory 

    User customizable per-user application data C:\Documents and Settings\[User Name]\Application Data\[Company Name]\[Product Name]
    Application.UserAppDataPath
    User customizable system-wide application data C:\Documents and Settings\All Users\Application Data\[Company Name]\[Product Name]
                                System.IO.Path.Combine(
                                System.Environment.GetFolderPath(
                                System.Environment.SpecialFolder.CommonApplicationData),
                                System.IO.Path.Combine(
                                Application.CompanyName, 
                                Application.ProductName)) 
                            

    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 http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnwue/html/ch11b.asp You are about to leave the SSW site

  18. 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 more 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 more 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

  19. What do you do with comments and Debug.Print statements?

    When you create comments in your code, it is better to document why you've done something a certain way than to document how you did it. The code itself should tell the reader what is happening, there's no need to create "how" comments that merely restate the obvious unless you're using some technique that won't be apparent to most readers.

    What do you do with your print statements? Sometimes a programmer will place print statements at critical points in the program to print out debug statements for either bug hunting or testing. After the testing is successful, often the print statements are removed from the code. This is a bad thing to do. Debugging print statements are paths that show where the programmer has been. They should be commented out, but the statements should be left in the code in the form of comments. Thus, if the code breaks down later, the programmers (who might not remember or even know the program to start with), will be able to see where testing has been done and where the fault is likely to be - i.e., elsewhere.

                        Private Sub Command0_Click()
                            rst.Open "SELECT * FROM Emp" 'Open recordset with employee records
                            'Exit sub if the count is greater than 1,000
                            If intCount > 1000 Then
                              Exit Sub
                            Else
                        EndIf
                        
                        .....processing code
                        End Sub 

    Bad Example

                        Private Sub Command0_Click()
                            'Count will exceed 1,000 during eighteenth century
                            'leap years, which we aren't prepared to handle.
                            If intCount > 1000 Then
                             Exit Sub
                            Else
                            EndIf
                        End Sub

    Good Example

  20. 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>
  21. 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"

    Bad Connection String

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

    Good Connection String

  22. Never start a long process (>30 seconds) without a warning or make it obvious - like the start button on Code Auditor

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

    Lengthy operation

    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.
  23. 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 he know the particular amount of time, if the time taken dialog is shown after finish. If the status bar contains the time taken and the progress bar contains the progress percentage, he can evaluate how long it will take according to the time taken and percentage. Then he can switch to other work or go get a cup of coffee.

    Also for 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.

    Time Taken
    Figure: Bad example - popup dialog at the end of a long process
    Time Taken
    Figure: Good example - show time taken in the status bar
  24. Does backward compatibility kill good code?

    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 expense for all the apps that break, the decision isn't so easy to make.

    Our views on backward compatibility starts 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?

    Lets look at an example:

    We have a public web service /ssw/webservices/postcode/
    If we change the URL of this public Web Service, we'd have to answer the questions as follows:

    • Answer 1: Externally - Dont 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: Web site 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 sent 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 all wish we never need to support old code, but sometimes the world doesn't go that way, if your answer to question 3 scares you, then you might need to provide some form of backward compatibility or warning.

    From: John Liu www.ssw.com.au
    To: SSWALL
    Subject: Changing LookOut settings

    The stored procedure procSSWLookOutClientIDSelect (currently used only by LookOut any version prior to 10) is being renamed to procSSWLookOutClientIDSelect. The old stored procedure will be removed within 1 month.

    You can change your settings either by

    • Going to LookOut Options -> Database tab and select the new stored procedure:
      Lookout settings

    • Upgrading to SSW LookOut version 10.0 which will be released later today
  25. Do you put Exit Sub before End Sub?

    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.
  26.   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 ReadOnly Property Enable As Boolean Get Return true End Get End Property
                            Public ReadOnly Property Invoice As Boolean Get Return m_Invoice End Get End Property

    Bad Example

                            Public ReadOnly Property Enabled As Boolean Get Return true End Get End Property
                            Public ReadOnly Property IsInvoiceSent As Boolean Get return m_IsInvoiceSent End
                            Get End Property

    Good Example

    Figure: Naming Convention for Boolean Property
    We have a program called SSW Code Auditor to check for this rule.
  27. 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.


  28. 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.
  29. 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 parameter, as 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 becuase 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.
  30. Do you use interoperability mechanism for COM object?

    VB.NET includes the CreateObject () Method for creating the COM object. This is a old relationship between VB and COM.


                            Sub CreateADODBConnection()
                            Dim adoApp As Object
                            adoApp = CreateObject("ADODB.Connection")
                            End Sub 

    Figure: Bad code. Uses a VB technique - CreateObject() - for creating a COM object.

    Using the CreateObject() method affects the performance of your application. The variable adoApp is of type Object and this results in "late binding"
    which might lead to so much uncertainty. It is more efficient to use the interoperability features of .NET,which allows you to work with existing
    unmanaged code (code running outside the CLR) in COM components as well as Microsoft Win32 DLLs. The interoperability feature uses run-time
    callable wrappers for handling all interaction between the .NET client code (managed code) and the COM component (unmanaged code).


    To add references to COM objects:

    • On the Project menu, select Add Reference and then click the COM tab.
    • Select the component you want to use from the list of COM objects.


    • Using COM objects in .NET

    • To access to the interoperability assembly in your application, add an Imports statement to the top of the class or module in which you will
      use the COM object.


    You can also create interoperability assemblies using the Tlbimp command line utility.

    We have a program called SSW Code Auditor to check for this rule.
  31. Do you import namespaces and shorten the references?

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

    Figure: Long reference to object name. (Bad)

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

    Figure: Import the namespace and remove the repeated System.Text reference. (Good)

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

    1)

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

    2)

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

     

  32. Do you declare member accessibility for all classes?

    Not explicitly specifying the access type for members of a structure or class can be deceiving for other developers that are using this structure or class. The default structure and class members access in Visual C# .NET is always private. The default class member access in Visual Basic .NET is private. However, the default structure member access in Visual Basic .NET 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.

    We have a program called SSW Code Auditor to check for this rule.
  33. Do you do your validation with Exit Sub?

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

                        Private Sub AssignRightToLeft()                     
                        'Validate Right 
                            If lstParaRight.SelectedIndex >= 0 Then 
                               'Validate Left 
                               If lstParaLeft.SelectedIndex >= 0 Then 
                                  Dim paraID As String = lstParaRight.SelectedValue.ToString                    
                                  Dim mParagraph As BusinessLayer.Paragraph = New Paragraph 
                                  mParagraph.MoveRight(paraID) 
                                  RefreshData() 
                               End If 
                            End If
                         End Sub 

    Figure: Bad example using nested if for validation.

                        Private Sub AssignRightToLeft() 
                            'Validate Right and Left 
                            If lstParaRight.SelectedIndex = -1 Then Return
                                If lstParaLeft.SelectedIndex = -1 Then Return                        
                                    Dim paraID As String = lstParaRight.SelectedValue.ToString 
                                    Dim mParagraph As BusinessLayer.Paragraph = New Paragraph 
                                    mParagraph.MoveRight(paraID)                    
                                    RefreshData() 
                                End If
                            End If
                        End Sub 

    Figure: Good example using Exit Sub to exit early if invalid.

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

    You should always write each parameter of MessageBox in 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) 
                             
                            End Sub 
                        

    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.

  35. 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.

  36. Do you use resource file to store messages?

    All message is stored in one centre place so it's easy to reuse. Further more it is strong typed - easy to type with Intellisence 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 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 constant message

  37. 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.

  38. 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.
  39. Do you avoid using "Strings" in your code? Use "Enum"?

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

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

    Figure: Good example - Used Enums, looks good and is easy to manage
  40. Do you avoid using "Magic numbers" in your code? Use "Enum - Constants" instead?

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

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

    Figure: Good example - No Magic Number, looks good and is easy to manage
  41. Do you use a regular expression to validate an Uri?

    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 verify 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 verify Uri.

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

  42. Do you use "using" statement 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.

                    SqlConnection conn = null; 
                        SqlCommand cmd = null; 
                        try 
                        { 
                            conn = new SqlConnection(ConnectionString); 
                            cmd = new SqlCommand(sql, conn); 
                            conn.Open(); 
                            cmd.ExecuteNonQuery(); 
                        } 
                        catch(SqlException ex) 
                        { 
                            ... 
                        } 
                        finally 
                        { 
                            if(cmd!=null) 
                            {cmd.Dispose();} 
                            if(conn!=null) 
                            {conn.Dispose();} 
                        } 
                    

    Figure: Bad example of dispose of resources.

    FileStream fs = new FileStream(filePath, FileMode.Open, FileAccess.Read); 
                        StreamReader sr = new StreamReader(fs); 
                    

    Figure: Bad example of dispose of resources.

                    try 
                        { 
                            using (SqlConnection conn = new SqlConnection(ConnectionString)) 
                            { 
                                using (cmd = new SqlCommand(sql, conn)) 
                                { 
                                    conn.Open(); 
                                    cmd.ExecuteNonQuery();  
                                    conn.Close();   
                                } 
                            } 
                        } 
                        catch(SqlException ex) 
                        { 
                            ... 
                        } 
                    

    Figure: Good example of dispose of resources.

                    using(FileStream fs = new FileStream(filePath, FileMode.Open, FileAccess.Read)) 
                        { 
                            using(StreamReader sr = new StreamReader(fs)) 
                            { 
                                ... 
                            } 
                        } 
                    

    Figure: Good example of dispose of resources.

    We have a program called SSW Code Auditor to check for this rule.
  43. 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 application which help 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.

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

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

    Store messages in Message.resx
    Store messages in the Message.resx
                        Catch(SqlNullValueException  sqlex)
    	                {
    	                    Response.Write("The valule 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

  45. 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.

  46. Do you always check your button's event handler hook-up?

    Sometimes the button's event handler hook-up could be lost by accident, but there will be no warning or error reported when you complie your applications.

                    this.button1 = new System.Windows.Forms.Button();
                    this.button1.FlatStyle = System.Windows.Forms.FlatStyle.System;
                    this.button1.Location = new System.Drawing.Point(419, 115);
                    this.button1.Name = "button1";
                    this.button1.Size = new System.Drawing.Size(75, 23);
                    this.button1.TabIndex = 60;
                    this.button1.UseVisualStyleBackColor = true;
    	            

    Bad Example - the event handler hook-up is lost, so there will be no response after you click the button.

                        this.btnResetAll = new System.Windows.Forms.Button();
                        this.btnResetAll.FlatStyle = System.Windows.Forms.FlatStyle.System;
                        this.btnResetAll.Location = new System.Drawing.Point(417, 410);
                        this.btnResetAll.Name = "btnResetAll";
                        this.btnResetAll.Size = new System.Drawing.Size(75, 23);
                        this.btnResetAll.TabIndex = 54;
                        this.btnResetAll.Text = "Reset &All";
                        this.btnResetAll.UseVisualStyleBackColor = true;
                        this.btnResetAll.Click += new System.EventHandler(this.btnResetAll_Click);       
    	            

    Good Example : keep the event handler hook-up together with the initialization of the button

  47. Do you 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 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.

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

                        DialogResult diaResult = MessageBox.Show(this,
                        "The database is not valid." + Environment.NewLine +  "Do you want to upgrade it? ", 
                        "Tip", 
                        MessageBoxButtons.YesNo,
                        MessageBoxIcon.Warning);
    	            

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

                        DialogResult diaResult = MessageBox.Show(this,
                        "The database is not valid." + Environment.NewLine
                        + "Do you want to upgrade it? ", 
                        "Tip", 
                        MessageBoxButtons.YesNo,
                        MessageBoxIcon.Warning);
    	            

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

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

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

  49. Do you add a comment when you use Thread.Sleep?

    when a sleep is added to the code to delay a process, it should always be commented

                Friend Function RefreshSchema() As DialogResult
                SSW.SQLAuditor.WindowsUI.QueryAnalysisForm.RunScript(Startup.PageQueryAnalyzer.txtScript.Text)
                System.Windows.Forms.Application.DoEvents()
                'This is a sleep to delay the Application.DoEvent process        
                System.Threading.Thread.Sleep(500)
                System.Windows.Forms.Application.DoEvents()   
                ...
                
  50. 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 
                        Your connection string should always contain: 
                     

    Good example - The connection string with Application Name.

    . Application Name (e.g. SSWWebsite)
        o 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)
        o These logins should only have access to the databases they use (e.g. SSWData2005)

  51. 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 off to 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.

  52. 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.

  53. 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.";
    			    

    Good Example: The new line is created with Enviroment.NewLine

  54. Do you wrap the same logic in a method instead of writing it again and again whenever it's used?

    If a logic (a piece of code) will occur more than once, please make it a method and call it whenever it's used. 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

  55. 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
  56. 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 parameters

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

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

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

    BAD

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

    GOOD

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

  58. Do you expose events as events?

               public Action
    < connectioninformation > ConnectionProblem;
              

    Bad code

               public event Action
    < connectioninformation > ConnectionProblem;
              

    Good code

  59. Do you name your events property?

    Events should end in "ing" or "ed"

               public event Action
    < connectioninformation > ConnectionProblem;
              

    Bad code

               public event Action
    < connectioninformation > ConnectionProblemDetected;
              

    Good code

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

    No "UI" in event names, the event raiser should be unaware of the UI in MVVM and usercontrols
    The 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 current selected item, change the UI correspondingly (or even better: use MVVM and databinding)

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

    Instead of:

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

    ...use this event extension methods:

               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 ;)

  62. Do you know what to do with a work around?

    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 and you have added the work around.
    http://www.codeproject.com/Articles/39194/Disable-a-timer-at-every-level-of-your-ASP-NET-con.aspx
    Now what do you do?
    Current code:
    Sub OnPagePreRender(ByVal sender As Object, ByVal e As EventArgs)
     ' Fix for pages that allow edit in grids
     Me.FindControls(Of System.Web.UI.Timer).ForEach(Sub (c) c.Enabled = False)
    End Sub
                
    Figure: Work around code in the Page Render
    If you have to use a work around you should always comment your code and reference. You should also make a Suggestion to [Product] if you think it is something that the product should do.
    1. Comment in the code with URL to and existing or new Suggestion
    2. Create a Suggestion to .NET 3 that points to blog post
    "This is a work around as per the suggestion
    "[URL]
    Figure: Always add a URL to the suggestion that you are compensating for
  63. Do you follow boy scout rule?

    The idea is simply, each time leave code a little cleaner then you found it.

    Common examples are:

    1. Make sure one rule is enabled in code analysis and
    2. One rule in StyleCop and
    3. Get Code Auditor to zero

    Related links:

  64. 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.

    Imports System.Windows.Threading
    Imports System.Threading
    
    ''' <summary>
    ''' base class for command implementations 
    ''' This is a work around as standard MVVM commands 
    ''' are not provided by default
    ''' </summary>
    ''' <remarks></remarks>
    Public MustInherit Class Command
        Implements ICommand
    
        Private m_dispatcher As Dispatcher
    
        Protected Sub New()
            If Not Application.Current Is Nothing Then
                m_dispatcher = Application.Current.Dispatcher
            Else
                m_dispatcher = Dispatcher.CurrentDispatcher
            End If
            Debug.Assert(Not m_dispatcher Is Nothing)
        End Sub
    
        Public MustOverride Function CanExecute(ByVal parameter As Object) As Boolean Implements System.Windows.Input.ICommand.CanExecute
        Public MustOverride Sub Execute(ByVal parameter As Object) Implements System.Windows.Input.ICommand.Execute
        Public Event CanExecuteChanged(ByVal sender As Object, ByVal e As System.EventArgs) Implements System.Windows.Input.ICommand.CanExecuteChanged
    
    
        Public Sub OnCanExecuteChanged()
            If Not m_dispatcher.CheckAccess Then
                m_dispatcher.Invoke(New ThreadStart(AddressOf OnCanExecuteChanged), DispatcherPriority.Normal)
            Else
                CommandManager.InvalidateRequerySuggested()
                RaiseEvent CanExecuteChanged(Me, New EventArgs)
            End If
        End Sub
    
    End Class
    Figure: Bad example - The link to the suggestion should be in the comments
    Imports System.Windows.Threading
    Imports System.Threading
    
    ''' <summary>
    ''' base class for command implementations 
    ''' This is a work around as standard MVVM commands 
    ''' are not provided by default
    ''' http://www.ssw.com.au/ssw/Standards/BetterSoftwareSuggestions/WPF.aspx#EmbraseMVVM
    ''' </summary>
    ''' <remarks></remarks>
    Public MustInherit Class Command
        Implements ICommand
    
        Private m_dispatcher As Dispatcher
    
        Protected Sub New()
            If Not Application.Current Is Nothing Then
                m_dispatcher = Application.Current.Dispatcher
            Else
                m_dispatcher = Dispatcher.CurrentDispatcher
            End If
            Debug.Assert(Not m_dispatcher Is Nothing)
        End Sub
    
        Public MustOverride Function CanExecute(ByVal parameter As Object) As Boolean Implements System.Windows.Input.ICommand.CanExecute
        Public MustOverride Sub Execute(ByVal parameter As Object) Implements System.Windows.Input.ICommand.Execute
        Public Event CanExecuteChanged(ByVal sender As Object, ByVal e As System.EventArgs) Implements System.Windows.Input.ICommand.CanExecuteChanged
    
    
        Public Sub OnCanExecuteChanged()
            If Not m_dispatcher.CheckAccess Then
                m_dispatcher.Invoke(New ThreadStart(AddressOf OnCanExecuteChanged), DispatcherPriority.Normal)
            Else
                CommandManager.InvalidateRequerySuggested()
                RaiseEvent CanExecuteChanged(Me, New EventArgs)
            End If
        End Sub
    
    End Class
    Figure: Good example - When you link to a suggestion everyone can find it and vote it up
  65. Do you avoid Empty code block?

    Empty Visual C# .NET method consume program resources unnecessarily. Put comment in code block if its stub for future application.

    Exception: If a Class implements an Inherited Interface method, you should add a comment within the Code block

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

    Figure: Good Example - Method implements some code.

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

    Figure: Bad Example - Method is empty.

                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.

  66. 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. .NET compiler generates a jump list for switch block for that reason it is faster for long list of condition. And also complier has the ability to optimize the switch block. Condition in the switch block is faster as compiler evaluated the condition once but for if-else block, compiler need to evaluate the condition for each 'else if' block

    Note: Performance is very much negligible when number of condition is less than 5. So if the code design is clearer & easily maintainable by using if-else block, then Developer should use if-else block for fewer conditions

            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

  67. C# Code- 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 @

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

  68. Do you know that no carriage returns without line feed?

    Text files created on DOS/Windows machines have different line endings than files created on Unix/Linux. DOS uses carriage return and line feed ("\r\n") as a line ending, which Unix uses just line feed ("\n").

    Figure: Bad example
    Figure: Good example

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

  69. VB.NET Code - Do you know not to put Exit Sub before End Sub?

    This is often a bad practice if you already are ending Sub you don't need another line

    Sub MySub
    …
    End Sub
    Exit sub
    
                        
    Figure: Bad example
    Sub MySub
    …
    End sub
    
                        
    Figure: Good example

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

Acknowledgements

Adam Cogan
Peter Gfader

Benefit from our knowledge and experience!

SSW is the industry leader in Custom Software Development, Software Auditing & Developer Training.

Call us on +61 2 9953 3000 or email us for a free consultation

What does it cost? I’m not in Australia. Can you still help?