StringWriter is not disposed along all exception paths

Question

I am working with a StringWriter which I am passing to a method to write values in a foreach loop. I believe this is causing the generation of two warnings:

CA2000 : Microsoft.Reliability : In method 'ToCsvService.ToCsv()', object 'sw' is not disposed along all exception paths. Call System.IDisposable.Dispose on object 'sw' before all references to it are out of scope.

and

CA2202 : Microsoft.Usage : Object 'sw' can be disposed more than once in method 'ToCsvService.ToCsv()'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.

public string ToCsv()
{
    IEnumerable<string> props = GetProperties();
    StringWriter sw = new StringWriter(); // first warning here
    sw.WriteLine(GetHeadings(props));
    WriteValues(props, sw);

    sw.Close();
    string returnCsv = sw.ToString();
    sw.Dispose(); // second warning here

    return returnCsv;
}

I've left out GetProperties() from the list of methods called as it didn't seem pertinent.

private string GetHeadings(IEnumerable<string> props)
{
    string headings = String.Join(",",
        props.Select(prop =>
            _headings.ContainsKey(prop) ? _headings[prop] : prop));

    return headings;
}

private void WriteValues(IEnumerable<string> props, StringWriter sw)
{
    foreach (object obj in _collection)
    {
        var x = obj.GetType().GetProperties()
            .Where(pi => props.Contains(pi.Name))
            .Select(pi =>
                _format.ContainsKey(pi.Name)
                ? String.Format("{0:" + _format[pi.Name] + "}",
                                pi.GetGetMethod().Invoke(obj, null))
                : pi.GetGetMethod().Invoke(obj, null).ToString());

        string values = String.Join<string>(",", x);

        sw.WriteLine(values);
    }
}

Why are these warnings being generated?

Answer 1

Your code allows for the possibility that a thrown exception causes execution to skip over the statement that closes your StringWriter. You want to ensure that, before an exception causes execution to leave ToCSV, you close sw.

The easiest way to handle this is with a using block. An object constructed within a using clause is guaranteed to be disposed before the scope of the block is exited:

public string ToCsv()
{
    IEnumerable<string> props = GetProperties();
    using (StringWriter sw = new StringWriter())
    {
        sw.WriteLine(GetHeadings(props));
        WriteValues(props, sw);
        return sw.ToString();
    }
}

Note that you don't need to call both Close and Dispose on the StringWriter. Just Dispose is enough.

In general, you'll want to wrap a using block around the creation and use of all objects that implement IDisposable (as StringWriter does). That'll ensure that, no matter what exceptions get thrown, the object is always disposed of properly.

Answer 2

The second warning is because StringWriter.Close() calls StringWriter.Dispose(), http://msdn.microsoft.com/en-us/library/system.io.stringwriter.close.aspx , so you are calling Dispose twice.

The first warning is because if there is an exception after you new StringWriter() you don't have any catch statements to call Dispose on it. I would suggest rewriting your code to be

using (StringWriter sw = new StringWriter()) { 
    sw.WriteLine(GetHeadings(props));
    WriteValues(props, sw);
    return sw.ToString();
}