CODEWITHSUNDEEP
c#idisposable
Bruno Klein
Bruno Klein

Reputation: 3367

Is this proper usage of IDisposable?

I have a FTP class which has a Dipose() method

public class Ftp : IFtp
    {
        //other methods, properties and fields

        public void Dispose()
        {
            if (_ftp.IsConnected)
                _ftp.Close();

            _ftp.Dispose();
            _ftp = null;
        }
    }

This is the interface I am using for accessing the Ftp class

public interface IFtp : IDisposable
    {
        //other methods signatures
        void Dispose();
    }

Is this a proper and functional way of disposing the content used by Ftp?

Upvotes: 2

Views: 214

Answers (3)

Sepster
Sepster

Reputation: 4848

Refer MSDN documentation for IDisposable interface generally.

The MSDN IDisposable.Dispose() documentation specifically states:

When implementing this method, ensure that all held resources are freed by propagating the call through the containment hierarchy. For example, if an object A allocates an object B, and object B allocates an object C, then A's Dispose implementation must call Dispose on B, which must in turn call Dispose on C. An object must also call the Dispose method of its base class if the base class implements IDisposable.

...which is what you're doing.

For detail about your design warning, refer MSDN CA1063: Implement IDisposable correctly.

Assuming your underlying FTP implementation is not managed, you should also include a finalizer, if one is not already present in the base implementation. The MSDN IDisposable.Dispose() documentation specifically states:

Because the Dispose method must be called explicitly, objects that implement IDisposable must also implement a finalizer to handle freeing resources when Dispose is not called.

Refer also MSDN Object.Finalize and MSDN Implementing Finalize and Dispose to Clean Up Unmanaged Resources

If there's no appropriate finalizer in your inheritance chain somewhere, perhaps this is the reason for your warning?

Note also (as a counter example) that Microsoft recommends that consumers shouldn't call Dispose() directly. MSDN using statement documentation states:

As a rule, when you use an IDisposable object, you should declare and instantiate it in a using statement.

The preferred approach for consumers is to "use" an IDisposable resource, like this:

   using (FTP myDisposableFTP = new FTP()) {
     ...
   }

This is basically "syntactic sugar" to ensure that the Dispose() method is called when you're done using your disposable resource (ie the end of the using block), or importantly, if an exception occurs within the using block.

This pattern also avoids the requirement for null checks on your instance reference, as the instance has been constructed at the start of the block. It also ensures the object can't be reassigned (protecting the underlying resource from being accidentally left open, even when your original reference goes out of scope).

But I think it's doubtful that you could incorporate this pattern (or an equivalent try/catch/finally) into your inheritance/interface implementation, though.

So yes, calling the Dispose() method is functionally Ok, so long as you remember to call it under all circumstances where it is required (eg in your exception handling), or where you're re-exposing that out to your consumers to properly handle as part of a "wrapped" IDisposable implementation. But you should also add a finalizer if your resource is unmanaged.

Upvotes: 2

mhttk
mhttk

Reputation: 1696

I presume _ftp is an IDisposable object. If so, it should be enough to do the follwing:

public class Ftp : IFtp
{
    //other methods, properties and fields

    public void Dispose()
    {
        _ftp.Dispose();    
    }
}

also

public interface IFtp : IDisposable
{
    //other methods signatures    
}

That is, you do not need to re-specify the Dispose() method (in your interface) if your interface inherits from IDisposable, as it's already specified in IDisposable.

Upvotes: 3

Guffa
Guffa

Reputation: 700720

That works fine for the regular use of the class. However, you should check if the _ftp reference is null before you start to access it. Common practice is that you should be able to call the Dispose method more than once without causing any harm.

You might also want to add a Finalizer to the class, where you can dispose it in case of the one using the class fails to dispose of it properly for some reason.

The coding example in the documentation for the IDisposable interface is quite complete and well commented.

Upvotes: 3

Related Questions