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 will be released
every day when there is a bug fixed, so testers can continue testing while
development continues.
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.
However, in ASP.NET 2.0 each page is built into it own assembly. So, you will
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 - Footer.ascx.cs
protected void
Page_Load(object sender, EventArgs e)
{
lblVersion.Text =
PfastTrack.Functions.VersionInfo;
}
Do you start versioning at 0.1 and change to 1.0 once approved by a client or tester?
Software and document version numbers will be consistent and meaningful to both the developer and the user.
Generally, version numbering will 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.
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 will contain at least the following:
- Copyright declaration
- Purpose of the document/file
- Author name
If the file was modified by another developer, the comment will also contain:
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>
///'----------------------------------------------
Do you comment each property and method?
Its 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 will be as simple and self-explanatory as possible.
Exceptions will 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 will put these comments between the
<HEAD> and </HEAD>
tags.
To delmit the comments (ie top and bottom), you will use the standard
block comment markers of
<!-- and -->.
A css file will 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.
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
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;
Do you declare variables when you need them?
Will 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
will 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
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
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 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
-
Bad Example with If statement
Now in an update to this code, the programmer wishes to add an Oracle backend
database option. So they modify the Backend property as Oracle 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 utilizes the ElseIf statement.
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
-
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
-
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"
' Ignore
Case 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.
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 will 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 will 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
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:
- Debugging is simpler because all your parameters are in one place
- If for some reason you need to change the control's name then you only have to
change it in one place.
- 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
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 will
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 will
be commented out, but the statements will 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
Do you avoid validating XML documents
unnecessarily?
Validating an XML document against a schema is expensive, and will 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 will be distributed in the same directory as the XML file and a
relative path will be used:
<Report> <Report xmlns="LinkAuditorReport.xsd">
... </Report>
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
will 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
Never start a long process (>30 seconds) without a
warning or make it obvious - like the start button on Code Auditor
You will never start a long process without first giving a warning message to
warn the user approximately how long it will take.
You will need to have 2 things:
- 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)
- 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.
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.
- Figure: Bad example - popup dialog at the end of a long process
- Figure: Good example - show time taken in the status bar
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
will change it, clean code will 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:
- Upgrading to SSW LookOut version 10.0 which will be released later today
|
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
Do you follow naming
conventions for your Boolean Property?
Boolean Properties must be prefixed by a verb. Verbs like "Supports", "Allow",
"Accept", "Use" will be valid. Also properties like "Visible", "Available"
will 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 |
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 will improve this.
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.
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 will 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)
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.
- 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.
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".
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.
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.
Do you know how to format your MessageBox code?
You will 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.
-
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.
-
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
-
Do you suffix unit test classes with "Tests"?
Unit test classes will 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"
-
Do you know that Enum types will 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"
-
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 will have unit tests for it, see our
Rules to Better Unit
Tests for more information.
-
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.
-
Do you add the Application Name in the SQL Server connection string?
You will 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 example - The connection string with Application Name.
-
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 the Message.resx
-
Catch(SqlNullValueException sqlex)
{
Response.Write("The valule can not 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 Example
-
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:
- It can be updated easily with normal SQL e.g. Rolling over the current setting
to a new value.
- It can be used in joins and in other queries easily without the need to pass in
parameters.
- It can be used to update settings and affect the other applications based on the
same database.
-
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
-
Do you initialize variables outside of the try block?
-
Cursor cur;
try
{
...
cur = Cursor.Current;
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;
try
{
...
Cursor.Current = Cursors.WaitCursor;
...
}
finally
{
Cursor.Current = cur;
}
-
Good Example : Because the initializing code is outside the try block.
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.
-
Do you add a comment when you use Thread.Sleep?
when a sleep is added to the code to delay a process, it will 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()
...
-
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 will 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 will only have access to the databases they use (e.g. SSWData2005)
-
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.
-
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 should 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.
-
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
-
-
Acknowledgementsnts
Adam Cogan
Edward Forgacs
David Klein
Peter Ahn
|