Click here to Skip to main content
15,907,497 members
Please Sign up or sign in to vote.
4.00/5 (1 vote)
See more:
On the MSDN site in the topic about code analysis warning CA2000, a general pattern is recommended for a method which returns a disposable object. Can anyone explain why this pattern needs the temporary variable and why it uses a 'finally' clause rather than a 'catch' clause for code which is only relevant when an exception occurs? In other words, is there any reason why the MSDN recommmendation would be preferable to the second code sample below?

MSDN Recommendation
public SerialPort OpenPort2(string portName)
{
   SerialPort tempPort = null;
   SerialPort port = null;
   try
   {
      tempPort = new SerialPort(portName);
      tempPort.Open();
      SomeMethod();
      //Add any other methods above this line
      port = tempPort;
      tempPort = null;

   }
   finally
   {
      if (tempPort != null)
      {
         tempPort.Close();
      }
   }
   return port;
}

Why Not This Simplification?
public SerialPort OpenPort2(string portName)
{
    SerialPort port = null;
    try
    {
        port = new SerialPort(portName);
        port.Open();
        SomeMethod();
        //Add any other methods above this line
    }
    catch
    {
        if (port != null)
        {
            port.Close();
        }
        throw;
    }
    return port;
}
Posted
Updated 15-May-12 5:42am
v2

The reason is that the catch block gets executed only if an exception is thrown, whereas the finally block gets executed both in case of normal execution without an exception and in case an exception occurs.

So, in the second example the port.Close gets executed only when an exception occurs and remains open in the normal execution. As per MSDN documentation for an object implementing the IDisposable interface the Dispose method is to be called in the finally block.

The disposable pattern is implemented with IDisposable interface to ensure that the resources held by an object are released properly.

The try finally block is implicitly implemented by the using block and it is convenient to use using block with an object implementing IDisposable interface as explained here http://msdn.microsoft.com/en-us/library/yh598w02.aspx[^]
 
Share this answer
 
v2
Comments
Robert Ranck 15-May-12 11:52am    
In this specific coding pattern, the only work done inside the finally block is inside an if block which can only possibly be true when an exception has been thrown. So this particular finally block seems equivalent to a catch block.
A using block is appropriate with disposable objects in most cases, but does not address this situation where the object is being created within a method and returned from that method. The calling code will need to be responsible for the disposal of the object. It is therefore intentional that the port.Close remains open during normal execution.
I'd guess it depends on the disposable object. In your simplification, you attempt to open a port, allow any error to occur, attempt to close the port, and then return that same port. So if the port object holds any state information about what's been done to it, you've got whatever mess was created by your attempted initialization instead of a clean, null copy, as in the MSDN example.
 
Share this answer
 
Comments
Robert Ranck 15-May-12 11:45am    
I made an error when entering the simplification, which is now corrected. There is now a 'throw' in the catch clause. So in either set of code, an error should result in the object being disposed if it was created and an exception being thrown.
woopsydoozy 15-May-12 11:55am    
Agreed, now they're basically the same. One caution: I don't think you should count on a Close counting as a Dispose--again, depends on the object.
Robert Ranck 15-May-12 12:08pm    
According to Microsoft's guidelines for .NET Framework classes, when a class implements both a Dispose() method and a Close() method, they should be functionally equivalent. In this particular case, I didn't do any research to verify that for the SerialPort class because I assumed that whoever wrote the MSDN code sample had already done so. Normally I would verify that Close() does a Dispose(), or just use a Close() followed by a Dispose() if the documentation was unclear.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900