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.
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()) { // ... } } }
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.
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.
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):
using
.)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()); // ... }
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With