Using statement in every function -> convert to class field with proper cleanup?

Question

Basically I have a few functions that look like this:

class MyClass
{
    void foo()
    {
       using (SomeHelper helper = CreateHelper())
       {
           // Do some stuff with the helper
       }
    }

    void bar()
    {
        using (SomeHelper helper = CreateHelper())
        {
           // Do some stuff with the helper
        }
    }
}

Under the assumption I can use the same resource instead of a different one [instance] in every function is it ok practice in regard to cleanup and such to do this?:

class MyClass
{
    SomeHelper helper = CreateHelper();

    // ...foo and bar that now just use the class helper....

    ~MyClass()
    {
      helper.Dispose();
    }
}

Answer 1

No, do not add a destructor (Finalizer).

You can reuse the resource but then your class has to implement IDisposable.

sealed class MyClass : IDisposable
{
    SomeHelper helper = CreateHelper();

    // ...foo and bar that now just use the class helper....

    //~MyClass()
    public void Dispose()    
    {
      helper.Dispose();
    }                         
}

And now you have to use MyClass instances in a using block. It self has become a managed resource .

A destructor is of no use, whenever a MyClass instance is being collected the associated helper object will also be in the same collection. But having a destructor still incurs considerable overhead.

The standard pattern for IDisposable uses a virtual void Dispose(bool disposing) method but when making the class sealed you can use the minimalistic implementation above.

Answer 2

In .NET you don't know when (or whether) finalizer is called at all.

Instead, explicitly indicate that your class is to be disposed of by implementing IDisposable:
(This is exactly what SomeHelper does)

class MyClass : IDisposable
{
    readonly SomeHelper helper = CreateHelper();

    // any method can use helper

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

using(var myObj = new MyClass()) {
    // at the end, myObj.Dispose() will trigger helper.Dispose()
}

I used readonly to ensure helper doesn't get re-assigned somewhere else in the class, but this really doesn't matter if you're careful.

You must be extra careful to never set it to null, or your Dispose will throw an exception. If the field is protected, you can check for nullity before calling Dispose on it so you know you're playing safe.