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.
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 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 - Footer.ascx.cs
protected void
Page_Load(object sender, EventArgs e)
{
lblVersion.Text =
PfastTrack.Functions.VersionInfo;
}
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:
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>
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?
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
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 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.
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
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 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
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
- <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.
- 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
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>
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
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.
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 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.
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:
- Upgrading to SSW LookOut version 10.0 which will be released later today
|
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!
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 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
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 |
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.
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
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.
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.
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.
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 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)
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 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.
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 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
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:
- Keep each line under 80 character, horizontal scrolling reduces readability.
- Break long patterns into multiple lines, usually after a space or a line break.
- Indent bracers to help think in the right scope.
- Format complicated OR patterns into multiple blocks like a case statement.
- 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!
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.
Figure: Test Harness for regular expressions in Code Auditor.
-
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)
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.
-
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 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"