关闭函数中的流，这是个坏主意吗？答案

【问题标题】：Closing a stream in function, a bad idea?关闭函数中的流，这是个坏主意吗？
【发布时间】：2012-04-19 14:14:11
【问题描述】：

我今天写了一些代码，另一位开发人员更改了它，说它更安全。我不确定这是否正确，因为我看不到这里所做的优势是一些代码示例

public byte[] ReadFile(Stream stream)
{
    byte[] result = null;

    try
    {
       // do something with stream
       result = <result of operation>
    }
    finally
    {
        stream.Close();
    }

    return result;
}

现在改成

public byte[] ReadFile(Stream stream)
{
    byte[] result = null;

    // do something with stream
    result = <result of operation>

    return result;
}

我对 c# 很陌生，当你完成它时不应该关闭它吗？

【问题讨论】：

并不是说在任何一种情况下，您的ReadFile 记录它是否关闭流都是有益的。例如，接受流的StreamReader 的构造函数明确表示它会处理该流，因此调用者不应再次处理它。
@Servy：更准确的说法是调用者不需要再次处理它。 Dispose 设计为幂等的，所以重复调用它应该没有效果。
@Douglas 知道一个方法是否处理传入的IDisposable 仍然很重要，因为调用者可能希望继续使用它。如果您将其传递给处理它的人并再次使用它，BAM！
@Servy：是的，当然，我没有对此提出异议。我只是指出允许调用者再次处理流。

标签： c# exception-handling io

【解决方案1】：

一般来说，第一个版本的设计很糟糕。

是的，应该关闭 Stream，但最好使用打开它的相同代码（相同方法）。这被称为关注点分离，它使错误和混乱的可能性大大降低。

所以

您的ReadFile() 接受例如string fileName 并打开和关闭流，或者
您将其留给呼叫者。

你的方法（第二版）应该这样使用：

using (var reader = new FileReader(...))
{
  // maybe some pre-reading
  var r = ReadFile(reader);
  // maybe some post-reading
}

请注意，第二种方法也使该方法更具可重用性。

【讨论】：

现在说得通了，非常感谢，我回去查看并意识到我已经将调用包装在 using 所以认为这是不这样做的唯一原因，看来你已经打开了我的眼睛。谢谢。
@Henk Holterman：我完全同意你关于关注点分离的说法，但不幸的是，这种设计似乎被 .NET 破坏了，至少在构造函数的情况下。将Stream 传递给读取器（例如StreamReader）、写入器（例如BinaryWriter）甚至另一个流（例如GZipStream）的构造函数将导致原始流在其他读取器/写入器时被释放/stream 是。
@Douglas - 是的，这是一个备受争议的设计。有时方便，有时令人讨厌的惊喜。但至少它是在对象之间而不是方法之间移动所有权。
@HenkHolterman：公平点重新转让所有权；但是，这确实意味着不应将Stream 传递给StreamReader，除非一个人“拥有”该流，从而使StreamReader 的使用不适用于诸如OP 方法之类的情况。我只是希望我们不必经常求助于第三方解决方法，例如 NonClosingStreamWrapper。
@HenkHolterman：StreamReader 的正确解决方案应该是有一个构造函数参数来指定它是否拥有所有权，因为有时每种方法都可能非常有利。

【解决方案2】：

这个问题没有正确答案，因为它取决于您的应用架构。

我会说，是的，因为如果在这个函数中stream 没有被创建，而只是使用，所以关闭它让我们让调用者来处理。

但我再说一遍，这取决于您的应用架构。

【讨论】：

【解决方案3】：

开门的人一定要记得关门。

所以最好在打开的方法中关闭流。

【讨论】：

【解决方案4】：

代码审查是正确的 - 永远不要做这样的事情（或者如果外部代码需要这样的事情，那么该代码可能没有正确设计 - 有例外，但对于像流这样的事情几乎从来没有，并且始终遵循一些默认的“模式”。
在大多数情况下，使用这样的流（来自“外部”）...

using(Stream stream = new ...)
{
    ...call your method
}

（或者例如，读者正在处理它 - 但建议您在任何一种情况下都这样做 - 等效于使用 'finally' 块，但归结为同一件事）

...基本上，调用函数永远不会知道您是否将其处理“内部” - 直到它崩溃。如果“双方”就这样的事情达成一致，那么这仍然是不可接受的，但可以不受惩罚。

【讨论】：

【解决方案5】：

通常，Stream 的创建者应该是关闭它的人，最好是通过 using 块来处理它：

using (var myStream = getMeAStream()) {
    ReadFile(myStream);

    // If you want to be really sure it is closed:
    myStream.Close();
    // Probably not neccessary though, since all 
    // implementations of Stream should Close upon Disposal
}

【讨论】：

【解决方案6】：

流已经被其他东西传递到这个函数中，你不负责关闭它，由调用代码来处理这个问题。

原因是可能需要在此函数外部执行另一个操作（如重置或另一个读取），如果关闭它会导致异常。

我曾经每天多次像这样重构代码，直到终于有人听我的意见。

我已经看到至少一个由这种代码引起的严重错误，所以总的来说这是个坏主意

【讨论】：

【解决方案7】：

这不仅仅是一个 C# 问题。调用“ReadFile”函数后，您需要将流用于其他操作吗？如果不是，您可以选择在读取文件后关闭流。使用完流后最好关闭它们，但通常我更喜欢在相同的上下文中关闭它们而不是打开它们，因为在那里你知道是否需要流进行其他操作：

Stream s = open_the_stream()
try {
  ReadFile(s)...
} finally {
  s.Close();
}

无论如何，当您终止使用它们时关闭流。

【讨论】：