CODEWITHSUNDEEP
c#idisposable
Shmoopy
Shmoopy

Reputation: 5534

How to return an object that's dependent on a chain of using statements?

I'd like to write a method similar to this one:

C Make()
{
    using (var a = new A())
    using (var b = new B(a))
    {
        return new C(b);
    }
}

This is bad since when the method returns, c keeps a reference to a disposed object.

Note that:

Upvotes: 4

Views: 161

Answers (3)

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726599

This is bad since I cant reference a and b from outside and dispose of them.

This is bad only if you keep references to a or b, which can be disposed from outside of your code. However, these objects are disposable, and because C is not creating them or getting the ownership transferred, it should take whatever it needs from A or B before finishing with the constructor, and not keep a reference to a disposable object:

class C {
    private readonly string x;
    private readonly int y;
    public C(B b) {
        // Using b here is OK
        x = b.X;
        y = b.Y;
        // We are done with b, so when b is disposed, C will not break
    }
}

Unfortunately, C keeps a reference to it's b throughout its lifetime and expects the caller to dispose of it when C is no longer needed

If you have no control over C, make an IDisposable wrapper for it, take ownership of B, and dispose of it when C is no longer necessary:

class WrapC : IDisposable {
    private readonly B b;
    public C C { get; private set; }
    public WrapC (B b) {
        this.b = b;
        C = new C(b);
    }
    public void Dispose() {
        b.Dispose();
    }
}

Remove the using statement for B, and dispose of WrapC when you are done with it.

Upvotes: 2

sstan
sstan

Reputation: 36483

Your situation is very similar to what I have sometimes seen when querying the database. In an attempt to separate the logic, you sometimes see code like this:

var reader = ExecuteSQL("SELECT ...");
while (reader.Read()) // <-- this fails, because the connection is closed.
{
    // process row...
}

public SqlDataReader ExecuteSQL(string sql)
{
    using (SqlConnection conn = new SqlConnection("..."))
    {
        conn.Open();
        using (SqlCommand cmd = new SqlCommand(sql, conn))
        {
            return cmd.ExecuteReader();
        }
    }
}

But, of course, that can't work, because by the time the SqlDataReader returns from the ExecuteSQL method, the connection is already closed (disposed).

So an alternative to the above makes use of delegates to achieve the separation of concerns, but still allowing the code to work properly:

ExecuteSQL(reader => {
    // write code that reads from the SqlDataReader here.
});

public void ExecuteSQL(string sql, Action<SqlDataReader> processRow)
{
    using (SqlConnection conn = new SqlConnection("..."))
    {
        conn.Open();
        using (SqlCommand cmd = new SqlCommand(sql, conn))
        {
            using (SqlDataReader reader = cmd.ExecuteReader())
            {
                while (reader.Read())
                {
                    processRow(reader);
                }
            }
        }
    }
}

So, perhaps you can try something similar in your case? Something like this:

MakeAndProcess(c => {
    // write code that uses C here.
    // This will work, because A and B are not disposed yet.
});

public void MakeAndProcess(Action<C> processC)
{
    using (var a = new A())
    using (var b = new B(a))
    {
        processC(new C(b));
    }
}

Upvotes: 0

Olly
Olly

Reputation: 6026

This is a good example of somewhere where documentation is important. It forms part of the contract for a class.

You've already realized this, of course, since you state,

the author of C stated that C does not take ownership of b.

This means that you can't achieve exactly what you're after here. What you have is probably incorrect, since a and b will be disposed immediately before the new C is returned.

You'll need to refactor this code somewhat. Either change Make() such that it takes a parameter of type B; the caller will remain responsible for the lifetime of B as well as C. Or write a new class that implements IDisposable and wraps A, B and C and exposes C through a property.

If you own type C you might consider modifying it to allow it optionally to take ownership of b. This is a fairly common pattern in .NET itself. See, for example, XmlReaderSettings.CloseInput.

Upvotes: 3

Related Questions