Use Code Analysis to measure the quality of sample code

Sample code is great because it allows you to quickly get up to speed with a new technology or learn how an API works and sometimes, you might even use some of the code in your own application. The major downside of reusing sample code is that most often, the code wasn't written with production quality in mind, so things like error handling, performance, security, etc. are omitted from the code.

Here is an example. The BeerHouse E-commerce sample application that is available from contains the UpdateOrder method which unsurprisingly updates a customer order. By looking at the code, you might decide that it looks ok and therefore decide to reuse it in your own application.

    1: /// <summary>
    2: /// Updates an existing order
    3: /// </summary>
    4: public override bool UpdateOrder(OrderDetails order)
    5: {
    6:    using (SqlConnection cn = new SqlConnection(this.ConnectionString))
    7:    {
    8:       object shippedDate = order.ShippedDate;
    9:       if (order.ShippedDate == DateTime.MinValue)
   10:          shippedDate = DBNull.Value;
   12:       SqlCommand cmd = new SqlCommand("tbh_Store_UpdateOrder", cn);
   13:       cmd.CommandType = CommandType.StoredProcedure;
   14:       cmd.Parameters.Add("@OrderID", SqlDbType.Int).Value = order.ID;            
   15:       cmd.Parameters.Add("@StatusID", SqlDbType.Int).Value = order.StatusID;
   16:       cmd.Parameters.Add("@ShippedDate", SqlDbType.DateTime).Value = shippedDate;
   17:       cmd.Parameters.Add("@TransactionID", SqlDbType.NVarChar).Value = order.TransactionID;
   18:       cmd.Parameters.Add("@TrackingID", SqlDbType.NVarChar).Value = order.TrackingID;
   19:       cn.Open();
   20:       int ret = ExecuteNonQuery(cmd);
   21:       return (ret == 1);
   22:    }
   23: }

However, if you look closer at the parameter order and follow its usage throughout the method, you will notice that it's not validated before being used. That, combined with the fact that the method is declared public means that the UpdateOrder method is a ticking time bomb that could blow up at any time. All it takes is to call the method and pass in null for the parameter order. The following screenshot shows the extent of the problem where the usage of the parameter order is highlighted.

Sample code

In Visual Studio Team System 2010, Code Analysis provides the ability to detect this exact problem. Code Analysis can detect if a parameter of a public method is validated and if not, it will warn you about it. When I ran Code Analysis on this application, Code Analysis displayed the following warning in the Error List (among other errors):

c:\Users\Administrator\Desktop\TheBeerHouse\TBH_Web\App_Code\DAL\SqlClient\SqlStoreProvider.cs(610) : warning: CA1062 : Microsoft.Design : In externally visible method 'SqlStoreProvider.UpdateOrder(OrderDetails)', validate parameter 'order' before using it.

The fix for this bug is as simple as adding an assert and checking for the null condition.

    1: /// <summary>
    2: /// Updates an existing order
    3: /// </summary>
    4: public override bool UpdateOrder(OrderDetails order)
    5: {
    6:     Debug.Assert(order != null);
    8:     if (order == null)
    9:         return false;
   11:    using (SqlConnection cn = new SqlConnection(this.ConnectionString))
   12:    {

The first time that I ran Code Analysis on a sample application I was amazed at the number of bugs in the code, including performance and severe security bugs. Nowadays, I've gotten into the habit of running Code Analysis first to see the overall health of the sample code before using it.

Habib Heydarian.