Do you need to make your current class IDisposable if using an IDisposable field in a using statement?

71 Views Asked by At

Here's my example:

public sealed class BigLogicClass
{
    private SmallDisposableClass fMyDisposableClass;

    public void Setup(someParam)
    {
        var eventHandlerFactory = new EventHandlerFactory();
        var eventHandler = eventHandlerFactory.Create(someParam);
        eventHandler.SomeEvent += OnDoingThisEvent;
    }

    public void DoSomething()
    {
        using(fMyDisposableClass = new SmallDisposableClass())
        {
            // run a long process.
        }
    }

    private void OnDoingThisEvent()
    {
        fMyDisposableClass.RunSomething();
    }
}

I have a method 'OnDoingThisEvent' in this class that is invoked elsewhere by another class 'eventHandler'. 'OnDoingThisEvent' needs to be able to use 'fMyDisposableClass'. However, doing it like this raises a code analysis error that basically suggests that 'fMyDisposableClass' is not being disposed of properly.

The only place it is instantiated is in a using statement, which encompasses the life time of the process for which 'fMyDisposableClass' should live. During this process 'OnDoingThisEvent' can be invoked. Presumably this means the instance of 'fMyDisposableClass' will be disposed of once that process completes. So is this an issue? If so, how would I be able to go about using 'fMyDisposableClass' without making 'BigLogicClass' disposable and then implementing dispose?

1

There are 1 best solutions below

2
Dmitry Bychenko On

In your current implementation (which is suspicious) there's no need to declare the class as IDisposable, since class doesn't own unmanaged resources. The class methods allocates and then free some resources, and that's all: the class itself doesn't own these resources:

    public void DoSomething()
    {
        // Some resources are allocated here...
        using(fMyDisposableClass = new SmallDisposableClass())
        {
            // run a long process.
        }
        // ...and releases here
    }

    private void OnDoingThisEvent()
    {
        // Note, that fMyDisposableClass can be disposed here
        // Any calls - RunSomething - are very suspicious.
        fMyDisposableClass.RunSomething();
    }

You, probably, what something like this:

public sealed class BigLogicClass
{
    // Instance doesn't own unmanaged resources   

    public void Setup(someParam)
    {
        var eventHandlerFactory = new EventHandlerFactory();
        var eventHandler = eventHandlerFactory.Create(someParam);
        eventHandler.SomeEvent += OnDoingThisEvent;
    }

    public void DoSomething()
    {
        using (var myDisposableClass = new SmallDisposableClass())
        {
            // run a long process.

            ...

            // Fire event using myDisposableClass which is not disposed yet 
            OnDoingThisEvent(myDisposableClass);  

            ... 
        }
    }

    private void OnDoingThisEvent(SmallDisposableClass myClass)
    {
        myClass.RunSomething();
    }
}

Edit: if you want to fire an event

    ...
    private void OnDoingThisEvent(SmallDisposableClass myClass)
    {
        myClass.RunSomething();

        // Let user 
        EventHandler someEvent = SomeEvent;

        if (someEvent != null)
        { 
            someEvent(this, EventArgs.Empty);
        } 
    }

    public event EventHandler SomeEvent;
    ...