How could one refactor code involved in nested usings?

Question

I've got some code that has an awful lot of duplication. The problem comes from the fact that I'm dealing with nested IDisposable types. Today I have something that looks like:

public void UpdateFromXml(Guid innerId, XDocument someXml)
{
    using (var a = SomeFactory.GetA(_uri))
    using (var b = a.GetB(_id))
    using (var c = b.GetC(innerId))
    {
        var cWrapper = new SomeWrapper(c);
        cWrapper.Update(someXml);
    }
}

public bool GetSomeValueById(Guid innerId)
{
    using (var a = SomeFactory.GetA(_uri))
    using (var b = a.GetB(_id))
    using (var c = b.GetC(innerId))
    {
        return c.GetSomeValue();
    }
}

The whole nested using block is the same for each of these methods (two are shown, but there are about ten of them). The only thing that is different is what happens when you get to the inner level of the using blocks.

One way I was thinking would be to do something along the lines of:

public void UpdateFromXml(Guid innerId, XDocument someXml)
{
    ActOnC(innerId, c =>
    { 
        var cWrapper = new SomeWrapper(c);
        cWrapper.Update(someXml);
    });
}

public bool GetSomeValueById(Guid innerId)
{
    var result = null;

    ActOnC(innerId, c => { result = c.GetSomeValue(); });

    return result;
}

private void ActOnC(Guid innerId, Action<TheCType> action)
{
    using (var a = SomeFactory.GetA(_uri))
    using (var b = a.GetB(_id))
    using (var c = b.GetC(innerId))
    {
        action(c);
    }        
}

This works, it's just kind of clunky to parse (as a human). Does anyone have any other suggestions on how one could reduce the code duplication around nested using blocks like this? If they weren't IDisposable then one would likely just create a method to return the results of b.GetC(innerId) ... but that isn't the case here.

Answer 1

You can always make a larger context in which to manage which objects should be created/disposed. Then write a method to create that larger context...

public class DisposeChain<T> : IDisposable where T : IDisposable
{
    public T Item { get; private set; }
    private IDisposable _innerChain;

    public DisposeChain(T theItem)
    {
        this.Item = theItem;
        _innerChain = null;
    }

    public DisposeChain(T theItem, IDisposable inner)
    {
        this.Item = theItem;
        _innerChain = inner;
    }

    public DisposeChain<U> Next<U>(Func<T, U> getNext) where U : IDisposable
    {
        try
        {
            U nextItem = getNext(this.Item);
            DisposeChain<U> result = new DisposeChain<U>(nextItem, this);
            return result;
        }
        catch  //an exception occurred - abort construction and dispose everything!
        {
            this.Dispose()
            throw;
        }
    }

    public void Dispose()
    {
        Item.Dispose();
        if (_innerChain != null)
        {
            _innerChain.Dispose();
        }
    }
}

Then use it:

    public DisposeChain<DataContext> GetCDisposeChain()
    {
        var a = new DisposeChain<XmlWriter>(XmlWriter.Create((Stream)null));
        var b = a.Next(aItem => new SqlConnection());
        var c = b.Next(bItem => new DataContext(""));

        return c;
    }

    public void Test()
    {
        using (var cDisposer = GetCDisposeChain())
        {
            var c = cDisposer.Item;
            //do stuff with c;
        }
    }

Answer 2

I like the answer provided by BFree as a start, but I'd make a few modifications.

//Give it a better name; this isn't designed to be a general purpose class
public class MyCompositeDisposable : IDisposable 
{
    public MyCompositeDisposable (string uri, int id, int innerid)
    {
        A = SomeFactory.GetA(uri);
        B = A.GetB(id);
        C = B.GetC(innerId);
    }

    //You can make A & B private if appropriate; 
    //not sure if all three or just C should be exposed publicly.
    //Class names are made up; you'll need to fix.  
    //They should also probably be given more meaningful names.
    public ClassA A{get;private set;}
    public ClassB B{get;private set;}
    public ClassC C{get;private set;}

    public void Dispose()
    {
        A.Dispose();
        B.Dispose();
        C.Dispose();
    }
}

After doing that you can do something like:

public bool GetSomeValueById(Guid innerId)
{
    using(MyCompositeDisposable d = new MyCompositeDisposable(_uri, _id, innerId))
    {
        return d.C.GetSomeValue();
    }
}

Note that the MyCompositeDisposable will likely need to have try/finally blocks in the constructor and Dispose methods so that errors in creation/destruction properly ensure nothing ends up un-disposed.

Answer 3

In the Rx framework there's a class called CompositeDisposable http://msdn.microsoft.com/en-us/library/system.reactive.disposables.compositedisposable%28v=vs.103%29.aspx

Shouldn't be too hard to roll your own (albeit very stripped down version):

public class CompositeDisposable : IDisposable
{
    private IDisposable[] _disposables;

    public CompositeDisposable(params IDisposable[] disposables)
    {
        _disposables = disposables;
    }

    public void Dispose()
    {
        if(_disposables == null)
        {
            return;
        }

        foreach(var disposable in _disposables)
        {
            disposable.Dispose();
        }
    }
}

Then this looks a little cleaner:

public void UpdateFromXml(Guid innerId, XDocument someXml)
{
    var a = SomeFactory.GetA(_uri);
    var b = a.GetB(_id);
    var c = b.GetC(innerId);
    using(new CompositeDisposable(a,b,c))
    {
        var cWrapper = new SomeWrapper(c);
        cWrapper.Update(someXml);
    }
}

Answer 4

If your Dispoable types correctly dispose all disposable members, you would only need one using statement.

For example, this:

public bool GetSomeValueById(Guid innerId)
{
    using (var a = SomeFactory.GetA(_uri))
    using (var b = a.GetB(_id))
    using (var c = b.GetC(innerId))
    {
        return c.GetSomeValue();
    }
}

could become this if a had members of type's b and c, and a disposed of b and c in its dispose method:

public bool GetSomeValueById(Guid innerId)
{
    using (var a = SomeFactory.GetA(_uri))
    {
        return a.GetSomeValue();
    }
}

class A : IDisposable
{
  private a;
  private b;

  public A (B b, C c)
  {
     this.b = b; this.c = c;
  }

  public void Dispose()
  {
     Dispose(true);
  }

  protected void Dispose(bool disposing)
  {
     if (disposing)
     {
        b.Dispose();
        c.Dispose();
     }
  }
}

You would have to modify your factory to inject b and c into a, however.