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


What others have to say about us
See what people think about this product 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. See what people think about this product
- 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 follow naming conventions?
  2. Do you include version numbers in your setup filename?
  3. Do you use the testing stage, in the file name?
  4. Do you remove spaces from your filename?
  5. Do you follow version conventions?
  6. Do you comment and brand your code?
  7. Do you add comments for your code if it is updated?
  8. Do you create Task List Comments for your code?
  9. Do you declare variables when you need them?
  10. Do you avoid problems in if-statements?
  11. Do you avoid logic errors by using ElseIf?
  12. Do you know where to store your application's files?
  13. Do you refer to form controls directly?
  14. What do you do with comments and Debug.Print statements?
  15. Do you keep your databinder.eval clean?
  16. Do you avoid validating XML documents unnecessarily?
  17. Do you change the connection timeout to 5 seconds?
  18. Never start a long process (>30 seconds) without a warning.
  19. Do you have the time taken at the end of a long process?
  20. Does backward compatibility kill good code?
  21. Do you always use Option Explicit?
  22. Do you put Exit Sub before End Sub?
  23. Do you always avoid On Error Resume Next?
  24. Do you follow naming conventions for your Boolean Property?
  25. Do you always make file paths @-quoted?
  26. Do you always prefix SQL stored procedure names with the owner?
  27. Do you refactor your code and keep methods short?
  28. Do you separate code from design (aka use an N-tier architecture)?
  29. Do you use Public/Protected Properties instead of Public/Protected Fields?
  30. Do you use String.Empty instead of ""?
  31. Do you pre-format your time strings before using TimeSpan.Parse()?
  32. Do you use interoperability mechanism for COM object?
  33. Do you import namespaces and shorten the references?
  34. Do you declare member accessibility for all classes?
  35. Do you name all your exception object as ex?
  36. Do you do your validation with Exit Sub?
  37. Do you use Asynchronous method and CallBack when invoke web method?
  38. Do you format and comment your regular expression?
  39. Do you test your regular expression?
  40. Do you use resource file to store your regular expressions?
  41. Do you know how to format your MessageBox code?
  42. Do you use a regular expression to validate an email address?
  43. Do you use resource file to store messages?
  44. Do you suffix unit test classes with "Tests"?
  45. Do you know that Enum types should not be suffixed with the word "Enum"?
  46. Do you use a regular expression to validate an Uri?
  47. Do you use "using" statement instead of use explicitly "dispose"?
  48. Do you add the Application Name in the SQL Server connection string?
  49. Do you use resource file to store all the messages and globlal strings?
  50. Do you store Application-Level Settings in your database rather than configuration files when possible?
  51. Do you always check your button's event handler hook-up?
  52. Do you initialize variables outside of the try block?
  53. Do you format "Environment.NewLine" at the end of a line?
  54. Do you add a comment when you use Thread.Sleep?
  1. 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...


  2. Do you include version numbers in your setup filename?

    I'm always downloading my favourite 3rd party software utilities from the web. But I find it very frustrating if the developer has not made it easy to check whether I've already got the latest version. It is not OK to expect a user to run the setup.exe in order to find out what version it is.

    The easy solution is to include the version number in the name of the setup file. We use [productname]_ver[major version]-[minor version].exe E.g. SSWCodeAuditor_Ver1-89.exe

    In .NET, however, you strike a problem. .NET removes any 0 in the prefix and so e.g. _v1-02 becomes _v1-2.  The rather contrived solution is to start minor versions at 11. Therefore _v1-11.exe is your first version.

    To deal with those naughty developers who don't include version numbers, rename the file after you download it...


    Figure: Download the software then rename it with version.
    setup.exe
    ExtremeEmailsSetup.exe
    Bad setup file name

    ExtremeEmails_Ver1-12.exe
    Good setup file name
  3. 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
  4. Do you remove spaces from your filename?

    It is not a good idea to have spaces in a file name as they might 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. You can even choose to use underscores. This alternative method to spacing makes file names more readable and will be easy on eyes when listed.

    extremeemailsversion1.2.doc
    Extreme Emails version 1.2.doc
    Bad file naming conventions

    Extreme_Emails_v1_2.doc
    ExtremeEmails_v1_2.doc
    Good file naming conventions
  5. Do you follow version conventions?

    Update pending

    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.

    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;
    }
  6. Do you comment and brand your code?

    Its important that you have a consistent code comment standard throughout an application, regardless of language. This delivers a consistent message about our company, the product and release information, and can be used by other developers to 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

    For Header comments (ie not in-line), the comment should contain at least the following:

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

    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 was modified by another developer, the comment should also contain:

    • Reviewer name

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

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

    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>
    ///'---------------------------------------------- 

    Above a method or property declaration:
    /// <summary>
    /// The main entry point for the application.
    /// </summary>

  7. Do you add comments for your code if it is updated?

    Its 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 (yellow block) 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
  8. 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;

  9. 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
  10. 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
    if (ProductName == null) { ProductName = string.Empty; } if (ProductVersion == null) { ProductVersion = string.Empty; } if (StackTrace == null) { StackTrace = string.Empty; }
    Figure: Good as well
  11. Do you avoid logic errors by using ElseIf?

    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 in a field called Backend. At the moment only Access and SQL Server are options.

    Private Sub Command0_Click()
     If rst!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
    Bad Example with If statement

    Now in an update to this code, the programmer wishes to add an Oracle backend database option. So they add Oracle Or more likely there was no value in the table so a null in the Backend field but you are running the SQL Server code...

    By using the above code, the user will either get an error or the wrong code will run because the above code only assume two possible situations. To avoid this problem, use this code instead which utilises the ElseIf statement.

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

    Private Sub Command0_Click()
     If rst!Backend = "Access" Then
         'Call this code ie. SQL
     ElseIf rst!Backend = "SQL Server" Then
         'Call this other code ie. Stored Proc
     Else
         MsgBox "Logic Error -- BackEnd is: " & rst!Backend
     End If
    End Sub
    Good Example with If statement

    When using boolean variables in .NET the following code is acceptable as the compiler will generate errors if the datatype of the variable bolFillFactor is changed. Do not use this method in VBA as it does not have strict-type checking.

    Private Sub Command0_Click()
     Select Case (CInt(drd("KeyNum")))

        Case 1
            ' Initialize the column list
            strTempColumn = ""

        Case Is > 1
            ' Ignore

     End Select
     .....processing code
    End Sub
    Bad Example with Case statement in VB.NET

    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 (CInt(drd("KeyNum")))

        Case 1
            ' Initialize the column list
            strTempColumn = ""

        Case Is > 1
            ' Ignore

        Case Else
            MessageBox.Show("Logic Error")

     End Select

     .....processing code

    End Sub
    Figure: Use 'Select Case' or 'Switch' statements to enhance readability when coding to find logic errors.
  12. 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

  13. 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")
     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")
     dteDateEnd = Format(pdteDateEnd, "dd/mm/yyyy")
     ....processing code
    End Sub
    Good Example
  14. 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
  15. Do you keep your "DataBinder.Eval" clean?

    Remember ASP code, you had lots of inline processing. Using DataBinder.Eval encourages the same tendencies. DataBinder.Eval is OK, so is formatting a number to a currency. But not formatting based on business rules. The general rule is, any code between <%# and DataBinder.Eval is bad and should be moved into protected method on the form.

    Here is a good and a bad way to binding fields in ASP.NET in a datagrid.

    Putting all the field binding code AND the business rule in the control

    • Bad: Business logic is in the presentation layer (.aspx file)

    • Bad: No intellisense

    • Bad: Compile errors are not picked up

    <asp:Label id="tumorSizeLabel" runat="server" Text='<%# iif( Container.DataItem.Row.IsNull("TumorSize"), "N/A",DataBinder.Eval(Container, "DataItem.TumorSize", "0.00")) %>'/>
    Bad code

    Putting the code on the ItemDataBound Event.

    • Good: Business logic is in the code behind (.vb or .cs file)

    • Good: intellisense

    • Bad: Code Bloat

    • Bad: Have to use server control for all controls (viewstate bloat)

    In server page: <asp:Label id="tumorSizeLabel" runat="server" />
    In code behind:
    Private Sub patientDataGrid_ItemDataBound( ByVal sender As Object, ByVal e As DataGridItemEventArgs)_
    Handles patientDataGrid.ItemDataBound
    If( e.Item.ItemType = ListItemType.Item Or e.Item.ItemType = ListItemType.AlternatingItem) Then
    Dim tumorSizeLabel As Label = e.Item.FindControl("tumorSizeLabel")
    Dim rowView As DataRowView = CType(e.Item.DataItem, DataRowView)
    Dim row As PatientDataSet.PatientRow = CType(rowView.Row, PatientDataSet.PatientRow)
    If row.IsTumorSizeNull() Then
    tumorSizeLabel.Text = "N/A"
    Else
    tumorSizeLabel.Text = row.TumorSize.ToString("0.00")
    End If
    End If
    End Sub
    Good code
    We have a program called SSW Code Auditor to check for this rule.
  16. 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>
  17. 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
  18. Never start a long process (>30 seconds) without a warning.

    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:
    2. ALogRecord (DateCreated, FunctionName, EmpUpdated, ComputerName, ActiveForm, ActiveControl, SystemsResources, ConventionalMemory, FormsCount, TimeStart, TimeEnd, TimeTaken, RecordsProcessed, Avg, Note, RowGuide, SSWTimeStamp)
    3. 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.
  19. Do you have the time taken at the end of a long process?

    This feature is Particularly important if the user runs a semi long task (e.g. 30 seconds) once a day, so he knows it will take a particular amount of time and 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

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

    I believe when you realize there is a better way to do something, then you should 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
  21. Do you always use Option Explicit?

    Option Explict should always be used in VB (and VB.NET) projects.

    This will turn many of your potential runtime errors into compile time errors, thus saving you from potential time bombs!

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

  22. 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
    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.
  23. Do you always avoid On Error Resume Next?

    Never use On Error Resume Next in VB (and VB.NET) projects.

    If an error occurred, On Error Resume Next will hide the error and things can go very haywire! In .NET, stop using the On Error syntax and use the try-catch exception syntax for better structural exception handling.

    In VB/VBA you should use On Error Resume Next with line of comment and after an offending line of code there should be statement On Error GoTo 0 to reset Errors collection.

    Private Sub cmdSelect_Click()
    Dim varTemp As Variant
    On Error Resume Next
    varTemp = columnADOX.Properties("RelatedColumn").Value
     .
     ....many lines of code...
     .
     intRoutesPerDay = 2
    End Sub
    Bad Example
    Private Sub cmdSelect_Click()
    Dim varTemp As Variant
    On Error Resume Next
    'Sometimes there is no related column value
    varTemp = columnADOX.Properties("RelatedColumn").Value
    On Error GoTo 0

     .
     ....continuing code...
     .
    End Sub
    Good Example
    We have a program called SSW Code Auditor to check for this rule.
  24.   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.
  25. Do you always make file paths @-quoted?

    In C#, backslashes in strings are special characters used to produce "escape sequences", for example \r\n creates a line break inside the string. This means that if you want to put a backslash in a string you must escape it out by inserting two backslashes for every one, e.g. to represent C:\Temp\MyFile.txt you would use C:\\Temp\\MyFile.txt. This makes the file paths hard to read, and you can't copy and paste them out of the application.

    By inserting an @ character in front of the string, e.g. @"C:\Temp\MyFile.txt", you can turn off escape sequences, making it behave like VB.NET. File paths should always be stored like this in strings.

    We have a program called SSW Code Auditor to check for this rule.
  26. Do you always prefix SQL stored procedure names with the owner?

    Stored procedure names in code should always be prefixed with the owner (usually dbo). This is because if the owner is not specified, SQL Server will look for a procedure with that name for the currently logged on user first, creating a performance hit.

    SqlCommand sqlcmd = new SqlCommand(); sqlcmd.CommandText = "proc_InsertCustomer" sqlcmd.CommandType
                                = CommandType.StoredProcedure; sqlcmd.Connection = sqlcon;
    Bad Example
    SqlCommand sqlcmd = new SqlCommand(); sqlcmd.CommandText = "
                                dbo.proc_InsertCustomer"; sqlcmd.CommandType
                                = CommandType.StoredProcedure; sqlcmd.Connection = sqlcon;
    Good Example
    We have a program called SSW Code Auditor to check for this rule.
  27. 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.

     

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

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


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

  31. 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.
  32. 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.
  33. 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".

     

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

     

  35. Do you name all your exception object as ex?

    We suggested to use ex as all Exception Object.

    catch (SomethingException myException)

    {

      Console.Writeline(myException.message);

    }

    Figure: Bad example not using ex for the exception object.
    catch (SomethingException ex)

    {

    doSomething();

    throw;

    }

    Figure: Good example using ex for the exception object.
    We have a program called SSW Code Auditor to check for this rule.
     
  36. 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 Sub
    Figure: Good example using Exit Sub to exit early if invalid.
  37. Do you use Asynchronous method and CallBack when invoke web method?

    Web service and web invoking becomes more and more popular today as the distributed systems are widely deployed. However, the normal method invoking may cause a disaster when apply to web method because transmitting data over Internet may cause your program to hang for a couple of minutes.

    private static string LoadContentFromWeb(string strUri)

    {
    ...

    WebResponse response = request.GetResponse();

    ...
    }
    Figure: Invoke web method by the normal way (Bad - because this will hang your UI thread)

    The correct way to invoke web method is using asynchronous call to send a request and use the delegated CallBack method to read the response, see code below:

     public static void GetOnlineVersionAsync(string strUri)
    {
        try
        {
         ...

            IAsyncResult r = request.BeginGetResponse(new AsyncCallback(ResCallBack), request);
         }
         catch(WebException ex)
        {
            Console.WriteLine(ex.ToString()) ;
         }
    }



    private static void ResCallBack(IAsyncResult ar)
    {
       try
       {
          string content = string.Empty;
          WebRequest req = (WebRequest)ar.AsyncState;
          WebResponse response = req.EndGetResponse(ar);


          ...

          RaiseOnProductUpdateResult(content);

       }
       catch(WebException ex)
       {
          Console.WriteLine(ex.ToString());
          RaiseOnProductUpdateResult(string.Empty);
        }
    }
    Figure: Invoke web method by using asynchronous method and CallBack (Good - UI thread will be free once the request has been sent)

    When working with Web Service, asynchronous methods will be automatically generated by your web services proxy.


    Figure: Automatically generated asynchronous methods

     

  38. Do you format and comment your regular expression?

    Regular expression is a very powerful tool for pattern matching, but a complicated regex can be very difficult for human to read and to comprehend. That is why like any good code, a good regular expression must be well formatted and documented.

    Here are some guideline when formatting and documenting your regex:

    1. Keep each line under 80 character, horizontal scrolling reduces readability.
    2. Break long patterns into multiple lines, usually after a space or a line break.
    3. Indent bracers to help think in the right scope.
    4. Format complicated OR patterns into multiple blocks like a case statement.
    5. Comment your regex on what it does, don't just translate it into English.
    # Match <BODY
    <BODY
    # Match any non > char for zero to infinite number of times
    [^>]*
    # MATCH >
    >
    Bad example: Comment that translate regex into English.
    # Match the BODY tag
    <BODY
    # Match any character in the body tag
    [^>]*
    # Match the end BODY tag
    >
    Good example: Comment that explain the purpose of the pattern.
    (?six-mn:(Label|TextBox)\s+(?<Name>\w+).*(?<Result>\k<Name>\.TextAlign\s*=\s* ((System\.)?Drawing\.)?ContentAlignment\.(?! TopLeft|MiddleLeft|TopCenter|MiddleCenter)\w*)(?!(?<=\k<Name>\.Image.*)|(?
    =.*\k<Name>\.Image)))
    Bad Example: Pray you never have to modify this regex.
    (?six-mn:
        # Match for Label or TextBox control
        # Store name into <name> group
        (Label|TextBox)\s+(?<Name>\w+).*

        # Match any non-standard TextAlign
        # Store any match in Result group for error reporting in CA
        (?<Result>
            # Match for control's TextAlign Property
            \k<Name>\.TextAlign\s*=\s*

            # Match for possible namespace
            ((System\.)?Drawing\.)?ContentAlignment\.

            # Match any ContentAlignment that is not in the group
            (?!TopLeft|MiddleLeft|TopCenter|MiddleCenter)\w*
        )

        # Skip any Control that has image on it
        (?!
            (?<=
                \k<Name>\.Image
                .*
            )
        |
            (?=
                .*
                \k<Name>\.Image
            )
        )
    )
    Good Example: Now it make sense!
  39. Do you test your regular expressions?

    Everyone writes unit tests for their code, because it helps developer to make changes in future without breaking existing functionalities. The same goes for regular expressions.  A good regular expression will have a set of test cases to make sure any future changes does not invalidate existing requirements.

    At SSW, we do not fix a regular expression until we have added a good and a bad test case.

    If your application is driven by regular expressions, you need a good test harness. Here is an example of a test harness we use in Code Auditor.

    Test Harness for regular expressions in Code Auditor.
    Figure: Test Harness for regular expressions in Code Auditor.
  40. Do you use resource file to store your regular expressions?

    public static Queue getFilesInProject(string projectFile)
    {
    	Queue tempQueue = new Queue();
    
    	TextReader tr = File.OpenText(projectFile);
    
    	// RT (10/10/2005): New regex to support VS 2005 project files (.csproj & .vbproj)
    	//(?ixm-sn:
    	//# VS 2003
    	//(?:RelPath\s=\s\"(?.*?)\")
    	//|
    	//# VS 2005
    	//(?:(?<=Compile|EmbeddedResource|Content|None)\sInclude=\"(?<FileName>.*?)\")
    	//)
    	Regex regex = new Regex
    	    (@"(?ixm-sn:(?:RelPath\s=\s\""(?<FileName>.*?)\"")|(?:(?<=Compile|EmbeddedResource|Content|None)\sInclude=\""(?<FileName>.*?)\""))");
    	MatchCollection matches = regex.Matches(tr.ReadToEnd());
    
    }
    Figure: Regular expression is embedded in code (Bad)

    The problem with this code is that the regular expression is embedded within the method and not easily testable without creating mock files on-the-fly, etc. Another issue with embedding regular expressions in-code is escaping issues - often people will forget to escape the special characters or escape them incorrectly and thus cause the regular expression to behave differently between the design and execution environments.

    The way we deal with this is to put the regular expression in a resource file. Using a resource file, it solves the aforementioned issues, and it also allows us to leave a comment for the regular expression.

    Figure: The regular expression (with comment) is stored in a resource file (Good)

    public static Queue getFilesInProject(string projectFile)
    {
    	Queue tempQueue = new Queue();
    
    	TextReader tr = File.OpenText(projectFile);
    
    	Regex regex = new Regex(RegularExpression.GetFilesInProject);
    	MatchCollection matches = regex.Matches(tr.ReadToEnd());
    
    }
    Figure: We can easily get the regular expression from resource file (Good)

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

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

  43. 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
  44. 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"