Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Dealing with nested "using" statements in C#

Tags:

c#

I've noticed that the level of nested using statements has lately increased in my code. The reason is probably because I use more and more of async/await pattern, which often adds at least one more using for CancellationTokenSource or CancellationTokenRegistration.

So, how to reduce the nesting of using, so the code doesn't look like Christmas tree? Similar questions have been asked on SO before, and I'd like to sum up what I've learnt from the answers.

Use adjacent using without indentation. A fake example:

using (var a = new FileStream()) using (var b = new MemoryStream()) using (var c = new CancellationTokenSource()) {     // ...  } 

This may work, but often there's some code between using (e.g. it may be too early to create another object):

// ...  using (var a = new FileStream()) {     // ...      using (var b = new MemoryStream())     {         // ...          using (var c = new CancellationTokenSource())         {             // ...          }     } } 

Combine objects of the same type (or cast to IDisposable) into single using, e.g.:

// ...  FileStream a = null; MemoryStream b = null; CancellationTokenSource c = null; // ... using (IDisposable a1 = (a = new FileStream()),      b1 = (b = new MemoryStream()),      c1 = (c = new CancellationTokenSource())) {     // ...  } 

This has the same limitation as above, plus is more wordy and less readable, IMO.

Refactor the method into a few methods.

This is a preferred way, as far as I understand. Yet, I'm curious, why would the following be considered a bad practice?

public class DisposableList : List<IDisposable>, IDisposable {     public void Dispose()     {         base.ForEach((a) => a.Dispose());         base.Clear();     } }  // ...  using (var disposables = new DisposableList()) {     var a = new FileStream();     disposables.Add(a);     // ...     var b = new MemoryStream();     disposables.Add(b);     // ...     var c = new CancellationTokenSource();     disposables.Add(c);     // ...  } 

[UPDATE] There are quite a few valid points in the comments that nesting using statements makes sure Dispose will get called on each object, even if some inner Dispose calls throw. However, there is a somewhat obscure issue: all nested exceptions possibly thrown by disposing of nested 'using' frames will be lost, besides the most outer one. More on this here.

like image 726
noseratio Avatar asked Oct 07 '13 05:10

noseratio


1 Answers

In a single method, the first option would be my choice. However in some circumstances the DisposableList is useful. Particularly, if you have many disposable fields that all need to be disposed of (in which case you cannot use using). The implementation given is good start but it has a few problems (pointed out in comments by Alexei):

  1. Requires you to remember to add the item to the list. (Although you could also say you have to remember to use using.)
  2. Aborts the disposal process if one of the dispose methods throws, leaving the remaining items un-disposed.

Let's fix those problems:

public class DisposableList : List<IDisposable>, IDisposable {     public void Dispose()     {         if (this.Count > 0)         {             List<Exception> exceptions = new List<Exception>();              foreach(var disposable in this)             {                 try                 {                     disposable.Dispose();                 }                 catch (Exception e)                 {                     exceptions.Add(e);                 }             }             base.Clear();              if (exceptions.Count > 0)                 throw new AggregateException(exceptions);         }     }      public T Add<T>(Func<T> factory) where T : IDisposable     {         var item = factory();         base.Add(item);         return item;     } } 

Now we catch any exceptions from the Dispose calls and will throw a new AggregateException after going through all the items. I've added a helper Add method that allows a simpler usage:

using (var disposables = new DisposableList()) {     var file = disposables.Add(() => File.Create("test"));     // ...     var memory = disposables.Add(() => new MemoryStream());     // ...     var cts = disposables.Add(() => new CancellationTokenSource());     // ...  } 
like image 180
Mike Zboray Avatar answered Oct 03 '22 01:10

Mike Zboray