30

using最近、自分のコードでネストされたステートメントのレベルが上がっていることに気付きました。その理由はおそらく、私がより多くの pattern を使用しているためであり、これにより、またはasync/awaitの少なくとも 1 つが追加されることがよくあります。usingCancellationTokenSourceCancellationTokenRegistration

usingでは、コードがクリスマス ツリーのように見えないように、のネストを減らすにはどうすればよいでしょうか。以前に SO で同様の質問がありました。回答から学んだことをまとめたいと思います。

usingインデントなしで隣接して使用します。偽の例:

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

これはうまくいくかもしれませんが、多くの場合、いくつかのコードが間usingにあります (たとえば、別のオブジェクトを作成するには時期尚早かもしれません):

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

同じ型 (または にキャストIDisposable) のオブジェクトを結合して singleusingにします。例:

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

これには上記と同じ制限があり、さらに冗長で読みにくい、IMO.

メソッドをいくつかのメソッドにリファクタリングします。

私が理解している限り、これは好ましい方法です。それでも、私は興味があります.なぜ次のことが悪い習慣と見なされるのでしょうか?

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);
    // ... 
}

[更新]一部の内部呼び出しがスローされた場合でも、ネストされたusingステートメントが各オブジェクトで呼び出されることを確認するコメントには、かなりの数の有効なポイントがあります。ただし、ややあいまいな問題があります。ネストされた「using」フレームを破棄することによってスローされる可能性のあるすべてのネストされた例外は、最も外側のものを除いて失われます。詳細はこちら。DisposeDispose

4

5 に答える 5

16

単一の方法では、最初のオプションが私の選択になります。ただし、状況によってDisposableListは が役立ちます。特に、すべてを破棄する必要がある破棄可能なフィールドが多数ある場合 (この場合、 は使用できませんusing)。与えられた実装は良いスタートですが、いくつかの問題があります (Alexei によるコメントで指摘されています):

  1. アイテムをリストに追加することを忘れないでください。(ただし、 を使用することを覚えておく必要があるとも言えますusing。)
  2. 破棄メソッドの 1 つがスローされた場合、破棄プロセスを中止し、残りのアイテムを破棄しないままにします。

これらの問題を修正しましょう。 

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;
    }
}

ここで、呼び出しから例外をキャッチし、すべての項目を調べた後にDispose新しいをスローします。より簡単な使用法を可能にするAggregateExceptionヘルパー メソッドを追加しました。Add

using (var disposables = new DisposableList())
{
    var file = disposables.Add(() => File.Create("test"));
    // ...
    var memory = disposables.Add(() => new MemoryStream());
    // ...
    var cts = disposables.Add(() => new CancellationTokenSource());
    // ... 
}
于 2013-10-07T07:04:05.040 に答える
1

aあなたの最後の提案は、bとcを明示的に破棄する必要があるという事実を隠しています。だからこそ醜い。

私のコメントで述べたように、クリーンなコードの原則を使用すれば、これらの問題に遭遇することはありません (通常)。

于 2013-10-07T05:46:14.000 に答える
1

別のオプションは、単純にtry-finallyブロックを使用することです。これは少し冗長に思えるかもしれませんが、不要なネストを減らします。

FileStream a = null;
MemoryStream b = null;
CancellationTokenSource c = null;

try
{
   a = new FileStream();
   // ... 
   b = new MemoryStream();
   // ... 
   c = new CancellationTokenSource();
}
finally 
{
   if (a != null) a.Dispose();
   if (b != null) b.Dispose();
   if (c != null) c.Dispose();
}
于 2013-10-07T06:17:42.373 に答える