How to properly dispose objects: injected vs. owned

Question

I have a question about disposing objects.

Consider this IDisposable class

public class MyClass : DisposableParentClass
{
    private MyProp _prop;        

    public MyClass(MyProp prop)
    {
        _prop = prop;
    }

    public MyClass()
    {            
        _prop = new MyProp();
    }

    protected override void Dispose(bool disposing)
    {
        if (disposing)
        {
            _prop.Dispose();
        }

        base.Dispose(disposing);
    }

}       

On the first constructor, MyProp is injected. So MyClass is not the owner of the object. But on the second constructor, MyProp is created locally. Should I always dispose MyProp, or should I check first if it is injected or not.

public class MyClass : DisposableParentClass
{
    private MyProp _prop;        
    private bool _myPropInjected = false;

    public MyClass(MyProp prop)
    {
        _prop = prop;
        _myPropInjected = true;
    }

    public MyClass()
    {            
        _prop = new MyProp();
    }

    protected override void Dispose(bool disposing)
    {
        if (disposing)
        {
            if (!_myPropInjected) { _prop.Dispose(); }
        }

        base.Dispose(disposing);
    }

}       

Answer 1

If your class should handle these two situations:

1. It is not the owner of the provided object, it should not dispose of it
2. It is the owner of the created object, it should dispose of it

Then yes, you need to have a mechanism that tells these two situations apart.

A common method (common to me anyway) is to use naming convention like this:

private MyProp _prop;        
private bool _ownsProp = false;

ie. reverse the meaning of your flags, but this is details, your solution is just fine, and yes, you need to have a solution like this.

---

If you have a ton of these fields, where each must have its own bool field to handle this, it might be worth creating a helper class, such as this LINQPad program demonstrates:

void Main()
{
    Injectable i1 = new Injectable();
    Injectable i2 = new Injectable(new Injected("A"));
    Injectable i3 = new Injectable(new Injected("A"), new Injected("B"));

    Debug.WriteLine("dispose a and b");
    i1.Dispose();

    Debug.WriteLine("dispose b");
    i2.Dispose();

    Debug.WriteLine("no dispose");
    i3.Dispose();
}

public class Injected : IDisposable
{
    public Injected(string name) { Name = name; }
    public string Name { get; set; }
    public void Dispose() { Debug.WriteLine(Name + " disposed"); }
}

public class Injectable : IDisposable
{
    private Ownable<Injected> _A;
    private Ownable<Injected> _B;

    public Injectable(Injected a, Injected b)
    {
        _A = Ownable.NotOwned(a);
        _B = Ownable.NotOwned(b);
    }

    public Injectable(Injected a)
    {
        _A = Ownable.NotOwned(a);
        _B = Ownable.Owned(new Injected("B"));
    }

    public Injectable()
    {
        _A = Ownable.Owned(new Injected("A"));
        _B = Ownable.Owned(new Injected("B"));
    }

    public void Dispose()
    {
        _A.Dispose();
        _B.Dispose();
    }
}

public class Ownable<T> : IDisposable
    where T : class
{
    private readonly T _Instance;
    private readonly Action _CleanupAction;

    public Ownable(T instance, bool isOwned)
    {
        _Instance = instance;

        if (isOwned)
        {
            IDisposable disposable = instance as IDisposable;
            if (disposable == null)
                throw new NotSupportedException("Unable to clean up owned object, does not implement IDisposable");

            _CleanupAction = () => disposable.Dispose();
        }
    }

    public Ownable(T instance, Action cleanupAction)
    {
        _Instance = instance;
        _CleanupAction = cleanupAction;
    }

    public T Instance { get { return _Instance; } }

    public void Dispose()
    {
        if (_CleanupAction != null)
            _CleanupAction();
    }
}

public static class Ownable
{
    public static Ownable<T> Owned<T>(T instance)
        where T : class
    {
        return new Ownable<T>(instance, true);
    }

    public static Ownable<T> Owned<T>(T instance, Action cleanupAction)
        where T : class
    {
        return new Ownable<T>(instance, cleanupAction);
    }

    public static Ownable<T> NotOwned<T>(T instance)
        where T : class
    {
        return new Ownable<T>(instance, false);
    }
}

Answer 2

A different note can be made here either.

It depends on what is your MyClass is doing actually.

For example, if we are talking about a class that reads video stream from device, after applies some filters to it and writes data to a user specified file, where file writing is made by stream passed from the outside, say like this:

public class VideoProcessor : IDisposable {
    private FileStream _videoFile = null;
    private VideoProcessor() {}

    //user specified FileStream
    public VideoProcessor(FileStream fs) {_videoFile = fs;}

    public void Dispose() {
        _videoFile.Dispose(); //Dispose user passed FileStream
    }   
}

disposing passed stream object during dispose call, makes actually sence.

In other cases, yes, it's better to not destroy object, if you are not an owner of it. Leave it to the caller to decide when it is appropriate time to do that.