摆脱嵌套的using(...)语句

27

有时候我需要在一个函数内使用多个一次性对象。最常见的情况是使用StreamReader和StreamWriter,但有时更复杂。

嵌套的using语句很快就会增加,并且看起来很丑。 为了解决这个问题,我创建了一个小类,收集IDisposable对象,并在自己被处理时释放它们。

public class MultiDispose : HashSet<IDisposable>, IDisposable
{
    public MultiDispose(params IDisposable[] objectsToDispose)
    {
        foreach (IDisposable d in objectsToDispose)
        {
            this.Add(d);
        }
    }

    public T Add<T>(T obj) where T : IDisposable
    {
        base.Add(obj);
        return obj;
    }

    public void DisposeObject(IDisposable obj)
    {
        obj.Dispose();
        base.Remove(obj);
    }


    #region IDisposable Members

    public void Dispose()
    {
        foreach (IDisposable d in this)
        {
            d.Dispose();
        }

    }

    #endregion
}

因此,我的代码现在看起来像这样:

        using (MultiDispose md = new MultiDispose())
        {
            StreamReader rdr = md.Add(new StreamReader(args[0]));
            StreamWriter wrt = md.Add(new StreamWriter(args[1]));
            WhateverElseNeedsDisposing w = md.Add(new WhateverElseNeedsDisposing());

            // code
        }

这种方法是否存在问题,可能会在未来造成问题? 我故意保留了HashSet继承的Remove函数,以使该类更加灵活。 当然,滥用此函数可能导致对象不能正确释放,但是还有许多其他方式可以让你自己踩到坑里,而不需要使用这个类。


1
在许多层面上,我认为IMO只是一个糟糕的想法。我希望你能从这里找到指引,为了你自己和你可能一起工作的团队成员的利益。请参考Skeet的答案以获得更清晰的解释。 - Sky Sanders
9
@Simon: 不要胡说八道。我们经常在这里提出关于命名、大写和缩进的问题。相比之下,这个问题是相当重要的。 - H H
我不确定是否理解有误,但是我们难道不是为了这个目的而拥有来自WinForms时代的System.ComponentModel.Container吗?使用一个简单的Container就可以了,它就像一个带有Add方法的集合。 - nawfal
6个回答

49

你可以只需执行以下操作:

using (var a = new A())
using (var b = new B())
{
    /// ...
}

1
我只会添加一次此评论,即使有三个人说了同样的话。如果您的编码标准中包含StyleCop,我认为它不会喜欢这个,因为它违反了禁止if没有花括号的规则。 - Stewart
2
哎呀!这就是从 IntelliSense 自学语言的结果啊!我用 C# 已经好几年了,却从没想过可以这样链式使用语句 :( - Ghostrider
1
@Stewart - 这是 StyleCop 中的一个缺陷。允许使用语句像这样“堆叠”是有意的;请注意 IDE 不会尝试缩进附加的 using 行。 - Joel Coehoorn
27
严格遵守Stylecop(或FxCop)的每个建议有点像跟随GPS设备走进湖里。有时候需要运用自己的判断力(软件无法做到这点)。 - Josh
我从未说过StyleCop是正确的 - 这只是基于我的经验所做的观察 - 只是如果在您的代码库中启用了它,那么禁用它的编译指示比原始代码更丑陋 :) - Stewart
显示剩余4条评论

21

关于这个通用原则,有几个要点:

  • 你的代码不符合C#的惯用写法。基本上,你要求与你共同工作的人承担一个不寻常的风格,但却几乎没有任何好处。
  • 正如其他人所指出的那样,你可以在不需要额外花括号的情况下嵌套使用using语句。
  • 如果你发现单个方法中有大量的using语句,你可能要考虑将其分成较小的方法。
  • 如果你有两个相同类型的变量,你可以使用一个using语句:

using (Stream input = File.OpenRead("input.dat"),
       output = File.OpenWrite("output.dat"))
{
}

假设您确实想要继续:

  • 您的代码将以难以预测的顺序处理其包含的资源。不要使用 set,应该嵌入一个列表 - 然后按照调用 Add 的相反顺序处理它。
  • 没有理由从 HashSet<T> 或任何集合派生。您应该只在类中作为私有成员变量拥有一个列表。
  • 如果其中一个 Dispose 调用失败,则不会进行其他 Dispose 调用;使用传统的 using 语句,每个 Dispose 调用都在其自己的 finally 块中进行。

基本上,我认为这是一个糟糕的主意。两级嵌套远非痛苦之源;三级嵌套应该很少出现;四级或更多的嵌套强烈提示需要重构。与其试图应对深度嵌套的痛苦,您应该设计避免它。


1
“+1 for 'bad idea'”。我的感觉更进一步,除非代码永远不会再被修改,否则隐式块就是等待发生的错误。我总是毫不例外地使用显式块。正如你所说,如果我对嵌套感到困扰,我会设计避免它。 - Sky Sanders
我不是完全确定,但我理解使用语句的意思是你提供的两个相同类型变量的例子是不安全的。首先,我非常确定这两个对象不能互相依赖,因为构造顺序可能是不确定的,而且如果第二个构造函数抛出异常,我非常确定第一个对象将不会被处理。 - Stewart
+1 建议将其拆分为更小的方法。此外,+1 @Sky 建议始终使用没有异常的块。 - Travis
“+1” 表示同意以下观点:“你的代码不符合 C# 的惯用写法。基本上,你要求其他人在很少的收益下接受一种不寻常的风格。” - user1068352
Jon,关于你提到的*如果Dispose调用失败...*,我在Dispose实现中总是看到这种模式:if (item1 != null) item1.Dispose(); if (item2 != null) item2.Dispose();。即使在这种情况下,如果item1.Dispose失败,也不会调用item2.Dispose。但这是一个非常常见的模式。你建议在顶层Dispose方法中使用更健壮的using (item1){}; using (item2){}吗? - nawfal
1
@nawfal:是的,我通常会这样做。 - Jon Skeet

13
也许您只是展示了一个简单的例子,但我认为以下代码更易读。
 using (StreamReader rdr = new StreamReader(args[0])) 
 using (StreamWriter wrt = new StreamWriter(args[1])) 
 {     
   // code 
 }

7

您可以通过只使用一对大括号使嵌套的using语句更加美观,就像这样:

using (StreamReader rdr = new StreamReader(args[0])) 
using (StreamWriter wrt = new StreamWriter(args[1])) 
{
    ///...
}

回答您的问题,您需要按照添加的相反顺序进行处理。
因此,您不能使用HashSet
此外,没有理由将IDisposable列表暴露给外部世界。
因此,您不应继承任何集合类,而是维护一个私有的List<IDisposable>
然后,您应该拥有公共的Add<T>Dispose方法(没有其他方法),并在Dispose中向后循环遍历列表。

4
个人而言,这会让我疯掉。如果您发现嵌套的using语句很烦人,可以回归到try/finally语法。Dispose方法不应该抛出异常,因此您可以假设多个可处理对象不需要单独包装在try/finally块中。
还值得注意的是,对于相邻的using块,您只需要一个括号即可:
using (var stream = File.Open(...))
using (var reader = new StreamReader(stream)) {

   // do stuff

}

2

我必须说,我不同意那些想要像这样一次又一次地使用using语句的人:

using (var a = new StreamReader())
using (var b = new StreamWriter())
{
 // Implementation
}

在我看来,没有任何包装的代码块非常难以阅读,这是一种不好的风格,并且可能会导致问题,除非所有开发人员都非常小心。

我认为这与以下内容相似:

if (someBoolean) DoSomething();
{
  // Some code block unrelated to the if statement
}

从技术上讲,它并不是无效的,但看起来非常糟糕。

我同意MultiDispose的概念可能不是最佳选择,因为它不是一种被接受的模式,但我绝对不会选择这种方法。如果你无法将事物分解成更小的块,则建议仍然使用嵌套的using语句。


网页内容由stack overflow 提供, 点击上面的
可以查看英文原文,
原文链接