26 January, 2011

Static code analysis and CA2000 warning

After upgrading to visual studio 2010, i started to get a lot of CA2000 warnings when running static code analysis.

Let's look at this example function:
public static void PopulateMyTable1(HtmlTable table)
{
    table.Rows.Clear();
    HtmlTableRow row = new HtmlTableRow();
    row.ID = "MyRow";
    row.Visible = false;
    HtmlTableCell cell = new HtmlTableCell();
    cell.Attributes.Add("class", "MyClass");
    cell.Height = "20px";
    row.Cells.Add(cell);
    table.Rows.Add(row);
}

Generates these warnings:
CA2000 : Microsoft.Reliability : In method 'TestClass.PopulateMyTable1(HtmlTable)', object 'cell' is not disposed along all exception paths.
Call System.IDisposable.Dispose on object 'cell' before all references to it are out of scope.
CA2000 : Microsoft.Reliability : In method 'TestClass.PopulateMyTable1(HtmlTable)', object 'row' is not disposed along all exception paths.
Call System.IDisposable.Dispose on object 'row' before all references to it are out of scope.


The warnings are generated because we are calling properties that can cause an exception, which will cause the object not beeing disposed properly.
So how can we fix this ?

public static void PopulateMyTable2(HtmlTable table)
{
    table.Rows.Clear();
    HtmlTableRow row = null;
    try
    {
        row = new HtmlTableRow();
        row.ID = "MyRow";
        row.Visible = false;
    }
    catch (Exception)
    {
        row.Dispose();
        throw;
    }
 
    table.Rows.Add(row);
 
    HtmlTableCell cell = null;
    try
    {
        cell = new HtmlTableCell();
        cell.Attributes.Add("class", "MyClass");
        cell.Height = "20px";
    }
    catch (Exception)
    {
        cell.Dispose();
        throw;
    }
 
    row.Cells.Add(cell);            
}

This generates no CA2000 warnings, but talk about code cluttering.
Let's try something else:

public static void PopulateMyTable3(HtmlTable table)
{
    table.Rows.Clear();
    HtmlTableRow row = new HtmlTableRow();
    table.Rows.Add(row);
    row.ID = "MyRow";
    row.Visible = false;
    HtmlTableCell cell = new HtmlTableCell();
    row.Cells.Add(cell);
    cell.Attributes.Add("class", "MyClass");
    cell.Height = "20px";
}

Since we always add the new objects to the collection before calling any properties/methods then there will be no CA2000 warnings.

A much safer approach, because if an exception is thrown by calling a property then it will be taken care of when disposing the collection.

No comments:

Post a Comment