All posts by Darren

Seven Steps To Quality Code – Introduction

Do you want to reduce the cost of producing your software? Do you want to reduce the number of defects, improve your customer satisfaction and make your developers (or yourself) happier? What about shorten your time to market and improve the speed at which you respond to feature requests? Of course you do! Well, I’m going to outline seven steps that you can take to enable you to achieve all these things.

My view is that software and the software development process can be massively improved by raising the quality of the written code.  What I mean by that, is that I will focus, at least initially, on the code that you see, not necessarily on its functionality.

Now, I’m not claiming that this is an original idea; improving quality is one of the core tenets of most, if not all, of the Agile methodologies, but if you haven’t yet embraced Agile, are struggling with some of the techniques or aren’t seeing the results you expected then the steps I’m going to outline will reap plenty of rewards. The approach could be particularly useful if you are suffering from poor quality code provided by a third party such as contractors or offshore teams.

The Benefits

Going a little further into the benefits of taking the effort to improve your code quality, you can fully expect to see:

    • Reduced costs
    • More reliable software
    • Higher customer satisfaction
    • Quicker response to change requests
    • Quicker response to defects
    • A happier development team

These benefits arise because of:

  • Quite simply – fewer bugs make it into builds
  • Bugs are caught earlier in the development lifecycle (when they are cheaper and easier to fix)
  • Reduced bug counts in system test, acceptance test and in production
  • Reduced time in test – potentially being able to drop a test cycle or two
  • Reduced fix times for any bugs that do occur – but more time to do them in
  • Easier refactoring for improving existing code or adding new features
  • More productive code reviews

The Approach

The approach is based on improving the quality of .Net code. The ideas will be applicable to other languages and environments, but the specific tools that I mention will target .Net and Visual Studio.

The aim is to take small highly practical steps, one at a time, until the end result is a powerful arsenal of tools and techniques that have become an integral part of the way the development team works. Each individual step should not involve any direct costs, the tools I will introduce are free. The only investment required will be a little time, effort and discipline.

The Steps

  1. Compiler Warnings
  2. XML Comments and GhostDoc
  3. Code Analysis – Minimum Rules
  4. StyleCop
  5. Code Analysis – Further Rules
  6. Peer Reviews
  7. Unit Tests

Let’s get started with step 1 – Compiler Warnings

Seven Steps to Quality Code – Step 1

Compiler Warnings

This is step 1 in a series of articles, starting with Seven Steps To Quality Code – Introduction.

I find it staggering how many organisations and how many developers accept and ignore compiler warnings.  If you are one of these people then these incredibly useful messages are your first step in improving your code quality.
Firstly, I know that not everyone is aware of what compiler warnings are, so let me explain.  When the Visual Studio compiler compiles your code, it detects potential problems in the code and produces a list for you to review.  These are problems that do not prevent the code from compiling but do indicate that something should be done to rectify the situation.  You’ll see something like this after compiling:

 

Compiler Warnings
Compiler Warnings after a Successful Build – Click for full size

These two warnings indicate that something is not quite right with your code.  There are a couple of variables that have been declared but never used.  This means, quite simply that you have one of the following two situations:

  1. Some unwanted declarations can be deleted (note that I said DELETED not commented out!).
  2. Visual Studio has caught a bug for you because the variables were supposed to be used.

In either case, responding to the warning appropriately (as opposed to ignoring it) will improve your code.  There are many different types of compiler warning, but they are all telling you something useful about your code.  Read them and act on them!

There is an option in Visual Studio to treat some or all warnings as errors so that if any warnings occur, the build fails. Jan Van Ryswyck in this blog entry takes a hard line and declares that the default setting is “Evil” and that changing the option so that all warnings are treated as errors is “simply none negotiable”.  I’m not going to suggest that you should be quite so strict with the Visual Studio setting, but I agree with the spirit of what is being said.  I allow the freedom to ignore compiler warnings during periods of debugging or ‘spiking’, but compiler warnings should never be  allowed to appear in checked-in code!

The important thing is to adopt a policy where compiler warnings are simply not acceptable.  As I said, I don’t suggest setting Visual Studio to treat errors as warnings, but I absolutely recommend that if you are using automated builds, then the build server is set to fail a build if it encounters any compiler warnings. (If using TFS, this can be achieved easily by adding /p:TreatWarningsAsErrors=true to the MSBUILD command line).

If warnings are not treated as errors by a tool (i.e. Visual Studio or the Build Server), it is critical that the number of warnings is zero.  And that means zero.  Not 1 or 2 or “just a few”, but zero.  There is a theory called Broken Windows Theory  that suggests that if there is a derelict building its windows will stay intact for a certain period time.  When one window gets broken, the others will soon follow suit and the building will deteriorate very quickly.  And so it is with code, once a single compiler warning creeps in, it is then suddenly “OK to ignore warnings” and the code deteriorates.  The bugs that could have been caught in our example above become lost and ignored in a sea of yellow triangles.

In conclusion then, the first step to quality code is a simple one, but an absolutely critical one.  Get rid of all your compiler warnings and take the first step on the journey.  Once you’ve done that, we can move on to step 2.

Refreshing Dependencies in SQL Server 2005

If you need to drop a stored procedure (or other object) in SQL Server, it’s often useful to check whether it’s used by other stored procedures etc. That is, what other objects have a dependency on the stored procedure you want to drop.

As you probably know, this can easily be achieved by right clicking the stored procedure in object explorer and selecting the Dependencies option. The problem with this approach is that previously SQL Server hasn’t been too good at keeping track of the dependencies (for example, if you’ve dropped and recreated a stored procedure, the dependencies get lost). This is improved in SQL Server 2008, but for those of us using SQL Server 2005, here’s a simple script that will force an update of dependencies on all the user stored procedures in the database:

DECLARE @sql nvarchar(max)
SELECT @SQL = IsNull(@SQL, '') + 'exec sp_refreshsqlmodule ' + name + char(13)
FROM sysObjects
WHERE Type = 'P'
AND Category = 0
ORDER BY name
-- Uncomment this line to see the resulting SQL
-- print @sql
EXEC sp_ExecuteSQL @sql

You’ll need at least service pack 2, because the sp_refreshsqlmodule was introduced then.

Delete Unused Rows in SQL Server

Recently, I’ve been developing a SQL Server Data Warehouse solution where I needed to remove all the “unused” rows from particular dimension tables, that is, rows that weren’t referenced by a foreign key relationship.

In itself, this isn’t particularly tricky. I had code that did something like this (using the AdventureWorks2008 sample database):

DELETE p
FROM [Person].[Address] p
WHERE NOT EXISTS (
SELECT 1 FROM [Person].[BusinessEntityAddress] f
WHERE f.[AddressID] = p.[AddressID])

I added a NOT EXISTS clause for each foreign key relationship and table.

Now, as you would expect, as the number of tables and relationships grew, this became cumbersome and prone to error. I reasoned that SQL Server “knows” what can and can’t be deleted and if you try to delete a row that is referenced from another table will report an error such as:

The DELETE statement conflicted with the REFERENCE constraint “FK_BusinessEntityAddress_Address_AddressID”

As a result, I developed a script that will check the foreign key relationships on a given table and only attempt to delete the rows that are not referenced. If tables and relationships are added at a later date, I no longer needed to worry about updating my code to take this into account.

The script (for SQL Server 2008) is shown here:

-- Set the name of the schema and table here
DECLARE @SCHEMA sysname
SET @SCHEMA = 'Person'
DECLARE @TABLE sysname
SET @TABLE = 'Address';
 
-- Determine unused rows for the above specified table
DECLARE @SQL nvarchar(max);
WITH ForeignKeys AS
(
SELECT
	ROW_NUMBER() OVER(ORDER BY fk.object_id) as RowNumber,
	SCHEMA_NAME(fk.schema_id) as ReferencingSchemaName,
	OBJECT_NAME(fk.parent_object_id) AS ReferencingTableName,
	COL_NAME(col.parent_object_id, col.parent_column_id) AS ReferencingColumnName,
	COL_NAME(col.referenced_object_id, col.referenced_column_id) AS ReferencedColumnName
FROM sys.foreign_keys AS fk
INNER JOIN sys.foreign_key_columns AS col
	ON fk.object_id = col.constraint_object_id
WHERE fk.referenced_object_id = OBJECT_ID(@SCHEMA + '.' + @TABLE)
	AND is_disabled = 0
)
SELECT @SQL = ISNULL(@SQL, 'DELETE p FROM [' + @SCHEMA + '].[' + @TABLE + '] p WHERE 1=1')
	+ ' AND NOT EXISTS (SELECT 1 FROM [' + ReferencingSchemaName + '].['+ ReferencingTableName + '] f' + cast(RowNumber as varchar)
	+ ' WHERE f' + cast(RowNumber as varchar) + '.[' + ReferencingColumnName + '] = p.[' + ReferencedColumnName + '])'
FROM ForeignKeys
EXEC sp_ExecuteSQL @SQL

Currently, it is only suitable for tables that are referenced using a single column key. I’ll expand it to handle multiple columns in another post.