8

I have a SerialPort that I'm using to connect to a virtual COM port. Since the connection is persistent, I'm having to keep a reference to the SerialPort in order to open, close, and otherwise manage the port. I'm also implementing IDisposable on my class (not the full Dispose pattern, as I don't actually have any proper unmanaged resources, just the SerialPort).

My question has to do with the use of SerialPort.Dispose() vs SerialPort.Close(). I'm using Close() in several places, and I understand from the documentation that this calls the Dispose() method on the SerialPort. However, what if, as in my TryConnect() method, it is possible that the SerialPort was never opened? Should I simply call Dispose(), and leave it at that? Or is the Close() method a better option?

More broadly, is it always a good idea to use one of these methods over the other?

Some relevant snippets from my code are below.

public bool TryConnect() {
    CheckDisposed();
    try {
        connectedPort = new SerialPort(SelectedPort);
        connectedPort.WriteTimeout = 1000;
        connectedPort.DataReceived += P_DataReceived;
        connectedPort.Open();
        return true;
    } catch (Exception e) {
        if (connectedPort != null) {
            connectedPort.Dispose();
            connectedPort = null;
        }

        return false;
    }
}

public void Disconnect() {
    CheckDisposed();
    if (connectedPort != null) {
        connectedPort.Close();
        connectedPort = null;
    }
}

public void Dispose() {
    if (!disposed) {
        if (connectedPort != null) {
            connectedPort.Close();
            connectedPort = null;
        }

        disposed = true;
    }
}
Lauraducky
  • 674
  • 11
  • 25
  • 2
    @CodingYoshi I explained this in my question. I'm not asking about the implementation of the dispose pattern, rather about when to call the `Dispose()` and `Close()` methods of `SerialPort`. I can't use a `using` statement for the port because it has to be open longer than the scope of a single block. – Lauraducky Jan 23 '18 at 04:44
  • Probably duplicate - https://stackoverflow.com/questions/61092/close-and-dispose-which-to-call which explains (if regular MSDN and [reference source](https://referencesource.microsoft.com/#System/sys/system/io/ports/SerialPort.cs,cf0e12f10c51fcc6) is not enough) that both should be the same by design guidelines.. Some clarification why you believe otherwise may help. – Alexei Levenkov Jan 23 '18 at 04:44
  • Here is my 2 cents and what I do: if a class implements idisposable, i always dispose it (even if some other method such as close etc. calls it for me.) I dont care if some other method calls dispose because I find it easier to just program against the interface and not hidden secrets of some other method such as close. To answer your question: call dispose when you are done and you no longer need it. – CodingYoshi Jan 23 '18 at 04:52
  • 1
    @CodingYoshi to be honest, having `Close()` JUST call the `Dispose()` method seems like a strange choice to me. Yes, it adds some semantics for something like a SerialPort, as you open and then close it, but it also adds what seems to me to be unnecessary confusion by having two methods called different things that do exactly the same thing. If I hadn't read the docs, I might assume that after a `Close()` call, the object might still be safe to use or even reopen. – Lauraducky Jan 23 '18 at 04:57
  • 1
    Some objects especially streams and the like do that and thats why I decided long ago that I do not care if they call dispose for me and i always dispose it myself. I always stay consistent and i have no headaches. – CodingYoshi Jan 23 '18 at 05:00
  • You might want to be aware of this: https://stackoverflow.com/questions/8927410/objectdisposedexception-when-closing-serialport-in-net-2-0 Just in case, since it also happens in later versions of .Net – Markus Deibel Jan 23 '18 at 06:14

2 Answers2

6

Calling Close is equal to calling Dispose(true)

https://github.com/Microsoft/referencesource/blob/master/System/sys/system/IO/ports/SerialPort.cs

    // Calls internal Serial Stream's Close() method on the internal Serial Stream.
    public void Close()
    {
        Dispose();
    }


    protected override void Dispose( bool disposing )
    {
        if( disposing ) {
            if (IsOpen) {
                internalSerialStream.Flush();
                internalSerialStream.Close();
                internalSerialStream = null;
            }
        }
        base.Dispose( disposing );
    }
Ray Krungkaew
  • 6,652
  • 1
  • 17
  • 28
4

Close is the same thing as Dispose for this class. Using ILSpy, this is the code for the Close method:

public void Close()
{
    base.Dispose();
}
Titian Cernicova-Dragomir
  • 230,986
  • 31
  • 415
  • 357
  • 1
    Not sure why you need to decompile when you can look at the code https://referencesource.microsoft.com/#System/sys/system/io/ports/SerialPort.cs,cf0e12f10c51fcc6... but anyway OP seem to already know this info... – Alexei Levenkov Jan 23 '18 at 04:48
  • 2
    Interesting. I was under the impression that `Close()` might do something extra, as the docs say it "Closes the port connection, sets the IsOpen property to false, and disposes of the internal Stream object." – Lauraducky Jan 23 '18 at 04:48
  • 1
    @AlexeiLevenkov Yes you are right, I have ILSpy extension in VS so decompiling it is one click away, and if the method is that simple it does not really matter :) – Titian Cernicova-Dragomir Jan 23 '18 at 04:50
  • @CodingYoshi Yes he says he is aware close calls dispose, he was under the impression close does something more, which it does not – Titian Cernicova-Dragomir Jan 23 '18 at 04:51
  • @Lauraducky if the docs say that then follow the docs. It is never a good idea to code based on implementation because it can change. Even if you can decompile or view the source code, you still never want to code against the implementation. – CodingYoshi Jan 23 '18 at 04:57
  • @CodingYoshi The docs also say that it might throw an IOException. Not sure where that might come from after seeing the underlying code – Lauraducky Jan 23 '18 at 05:02
  • 1
    Again, don't stick your nose there (wink). Who cares how, maybe it will be implemented tomorrow or next month or never. Just go by the docs and the interface. – CodingYoshi Jan 23 '18 at 05:08
  • @CodingYoshi I disagree with your advice to avoid checking source code. 1) "It is never a good idea to code based on implementation because it can change." That sounds wise, but in the real world, documentation can also change, and it probably does so more often than the code in the case of system libraries like this. 2) Documentation is often wrong or vague. Verifying with the source code as needed is better than blind trust or speculation. 3) In this case, four short lines of source code give a clear and direct answer to the question, whereas multiple pages of documentation do not. – MarredCheese Mar 17 '23 at 19:02
  • 1
    @MarredCheese The problem is that most vendors, MS being one, will freely change implementation without broadcasting it to the consumers/customers. They are under no obligation to broadcast the changes. However, if they change the interface or introduce some other breaking change which goes against the documentation, they will broadcast it (at least they are supposed to). Most vendors operate using this philosophy. – CodingYoshi Mar 24 '23 at 16:46