Search code examples
c#asp.net-mvcinversion-of-controlunity-container

What is the best practice to dispose base class in Unity


I have the following structure. What I'm confusing is how to dispose PhoneBase. I have no idea if Unity disposes the PhoneBase.

public class PhoneBase : IDisposable
{
    protected int GetSignal()
    {
    }

    //something needs to dispose
    public void Dispose()
    {
    }
}

public interface IMyPhone
{
    void SwitchOn();
    void SwitchOff();
}

public class MyPhone : PhoneBase, IMyPhone
{
    public void SwitchOn()
    {
        //implement
    }

    public void SwitchOff()
    {
        //implement
    }

}

public class PhoneQuestionController : Controller
{
    IMyPhone myPhone;
    public PhoneQuestionController(IMyPhone myPhone)
    {
        this.myPhone = myPhone;
    }
}

Should I put Dispose method in the IMyPhone interface then call it while overriding Dispose of controller like below? Or is there a better way to do that?

public interface IMyPhone
{
    void SwitchOn();
    void SwitchOff();
    void Dispose();
}

public class PhoneQuestionController : Controller
{
    IMyPhone myPhone;
    public PhoneQuestionController(IMyPhone myPhone)
    {
        this.myPhone = myPhone;
    }

    override Dispose(bool disposing)
    {
        base.Dispose(disposing);
        myPhone.Dispose();
    }
}

Solution

  • I don't think you understand the purpose of IDisposable. A class should only implement IDisposable if it owns resources that are themselves disposable. That means the class must actually new something up, not be injected. A class should never dispose of injected resources; that's the job of the object lifetime manager - Unity, in this case.

    Assuming PhoneBase does actually own some disposable resource, then MyPhone would inherit both this resource and the IDisposable implementation. In fact, PhoneBase doesn't actually even exist in this scenario. It's not like MyPhone somehow has an internal reference to a PhoneBase instance. All you have is a MyPhone instance, with all the same properties and methods as PhoneBase, plus anything defined specifically on it.

    Then, if you're injecting MyPhone in place of IMyPhone, Unity is the entity that owns that object, and therefore is the one responsible for disposing of it (which it will do, at the end of its lifetime, as defined in your DI config). Your controller should absolutely not dispose of it. Period. The controller does not own it, and if Unity is injecting it anywhere else, you'll then get an exception in those other instances, because the object has been improperly disposed.

    As a side note, it's far worse to improperly implement and use IDisposable than to not dispose of objects you should be. Ultimately, the GC will take care of anything left behind. Manually disposing is just good housekeeping. However, improperly disposing can cause memory leaks, throw exceptions, and inflict all sorts of nastiness on your application. As a result, the standard recommendation is that you should only implement IDisposable if you know what you're doing and exactly why you're doing it. If you're not sure, don't implement it.