Is SerialPort in .NET unmanaged resource? Is my wrapped class correct?

I have adapter pattern (wrapper) over serial port class. S开发者_如何学Gohould I implement IDisposable pattern and call _wrappedSerialPort.Dispose() in it? There is my class, is it correct?

public class SerialPortAdapter : ISerialPortAdapter
{
    private bool _disposed;

    public event SerialDataReceivedEventHandler DataReceived;

    private readonly SerialPort _wrappedSerialPort;

    public SerialPort WrappedSerialPort
    {
        get { return _wrappedSerialPort; }
    }

    public string PortName
    {
        get { return _wrappedSerialPort.PortName; }
        set { _wrappedSerialPort.PortName = value; }
    }

    public BaudRate BaudRate
    {
        get { return (BaudRate)Enum.ToObject(typeof(BaudRate), _wrappedSerialPort.BaudRate); }
        set { _wrappedSerialPort.BaudRate = (int)value; }
    }

    public bool IsOpen
    {
        get { return WrappedSerialPort.IsOpen; }
    }

    public SerialPortAdapter(SerialPort serialPort)
    {
        _wrappedSerialPort = serialPort;
        _wrappedSerialPort.DataReceived += SerialPortDataReceived;
    }

    public void OpenPort()
    {
        if (!_disposed)
        {
            if (!WrappedSerialPort.IsOpen)
            {

                WrappedSerialPort.Open();

            }
        }
    }


    public void ClosePort()
    {
        if (!_disposed)
        {
            if (WrappedSerialPort.IsOpen)
            {

                WrappedSerialPort.Close();

            }
        }
    }


    public void WriteLine(string request)
    {
    ...
    }


    public void Write(byte[] request)
    {
       ....
    }


    public byte[] Read()
    {
      ....
    }


    public string ReadLine()
    {
       ...
    }


    private void SerialPortDataReceived(object sender, SerialDataReceivedEventArgs e)
    {
        if (DataReceived != null)
        {
            DataReceived(this, e);
        }
    }

    #region IDisposable Members

    public virtual void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    private void Dispose(bool disposing)
    {
        if (!_disposed)
        {
            if (disposing)
            {
                // Dispose managed resources.

            }
            // Dispose unmanaged resources.

            ClosePort();
            WrappedSerialPort.DataReceived -= SerialPortDataReceived;
            _wrappedSerialPort.Dispose();

            _disposed = true;

        }
    }

    ~SerialPortAdapter()
    {

        Dispose(false);
    }

    #endregion
}

Edit: Is it necessary to call this, or is enough to call only _wrappedSerialPort.Dispose();?

        ClosePort();
        WrappedSerialPort.DataReceived -= SerialPortDataReceived;
        _wrappedSerialPort.Dispose();

---

Henk Holterman's answer is correct: SerialPort is a managed resource, which itself owns an unmanaged resource and hence implements IDisposable.

Since your wrapper owns a SerialPort, it indirectly owns SerialPort's unmanaged resource and hence must implement IDisposable. Your implementation is wrong, the owned SerialPort instance should only be disposed if disposing is true, since it is a managed resource.

It should be implemented as follows:

   private void Dispose(bool disposing)
    {
        if (!_disposed)
        {
            if (disposing)
            {
                // Dispose managed resources.
                ClosePort();
                WrappedSerialPort.DataReceived -= SerialPortDataReceived;
                _wrappedSerialPort.Dispose();
            }
            _disposed = true;
        }
    }

Also, as Henk Holterman points out, you only need a destructor if you directly own unmanaged resources, which is not the case here, and you can simplify the IDisposable implementation by getting rid of the destructor.

---

The SerialPort itself is the owner of an unmanaged resource and that is why it implements the full Disposable pattern.

In your class, the _wrappedSerialPort is a managed resource. My definition: a managed resource is an indirect unmanaged resource.

Your class does not need the full pattern. You could and should omit the destructor (or finalizer), ~SerialPortAdapter () and following that you can skip the SupressFinalize.

It would be best to leave the rest, but you will see it would be easy to shorten the code a lot more (because void Dispose(bool) will never be called with false).

---

Yes, you are correct in implementing a Dispose method in this case. Make sure to add IDisposable to your class declaration. This makes it possible to use the very handy using construct, like this:

using (var port = new SerialPortAdapter(serialPort)) {
    port.OpenPort();
    // use port
}

Calling Dispose on the wrapped serial port ought to be enough, since this will close the port. According to MS documentation, Close will call Dispose internally. No harm in being clear in your intentions by explicitly calling Close, though.

Unregistering from the event, like you are doing, is good practice.

http://msdn.microsoft.com/en-us/library/system.io.ports.serialport.close.aspx

---

• Yes, the serial pot is an unmanaged report. Or did you ever see a garbage collector for the PHYSICAL HARDWARE? ;)

• This is shown by the class for access implementing IDisposable - which means your class hosting a lon term refernce pertty much HAS TO IMPLEMENT IDISPOSABLE, TOO.