Search code examples
c#design-patternsdisposeidisposable

Instance method call Dispose


Is correct that a public method calls the Dispose of IDisposable in the same class?

E.g.

public class Worker : IDisposable
{
    public void Close()
    {
       SendCloseCommand();
       Dispose();
    }

    public void Dispose()
    {
       ////other resources release
    }

    private void SendCloseCommand()
    {
    }
}

Another thing about disposable pattern: If the Close should be always called, is better that is called from the Dispose method or not?

If not, I should put the usage of the Worker class in a try/catch/finally everywhere, to call the Close. Right?

The correct design should instead be?

public class Worker : IDisposable
{
    public void Close()
    {
       SendCloseCommand();
    }

    public void Dispose()
    {
       Close();
       ////other resources release
    }

    private void SendCloseCommand()
    {
        ////other stuff
    }
}

Solution

  • If you look at Microsoft implementation for StreamReader, Dispose gets called from Close,

    public override void Close()
    {
        Dispose(true);
    }
    

    and also the implementation for Dispose closes the stream as well by calling Close of base Stream.

    protected override void Dispose(bool disposing)
    {
        // Dispose of our resources if this StreamReader is closable.
        // Note that Console.In should be left open.
        try {
            // Note that Stream.Close() can potentially throw here. So we need to 
            // ensure cleaning up internal resources, inside the finally block.  
            if (!LeaveOpen && disposing && (stream != null))
                stream.Close();
        }
        finally {
            if (!LeaveOpen && (stream != null)) {
                stream = null;
                encoding = null;
                decoder = null;
                byteBuffer = null;
                charBuffer = null;
                charPos = 0;
                charLen = 0;
                base.Dispose(disposing);
            }
        }
    }
    

    In your class implementation, I would call the Dispose from your Close method as you are doing in your first code snippet. In Dispose, I would check for Worker status, if it is not closed then close it using SendCloseCommand, and then release resources.

    public void Dispose()
    {
       if(this.Status != Closed) // check if it is not already closed
         {
             SendCloseCommand();
         }
    
       ////other resources release
    }
    

    This will ensure your resources disposal, even if your class is used with using statement, or if any one calls Close manually.

    Just remember to check status of your Worker object, before issuing Close Command.