开发者

Generic IDisposable wrapper for stream - alternative to catching all exceptions

So I've just written this implementation for a type that abstracts a file and implements IDisposable over the stream that it contains. It is intended to be used in any project:

public void Dispose()
{
  if (_contentStream == null)
    return;
  lock (_contentStreamLocker)
  {
    if (_contentStream == null)
      return;

    try
    {
      _contentStream.Dispose();
    }
    catch (Exception)
    {
      //legitimate? - don't let any exception prevent 
      //us from successfully disposing?
    }  
    _contentStream = null;
  }
}

So I feel dirty, obviously, because:

  • I'm catching Exception
  • I'm doing nothing with Exception

But since I can't guarantee the behaviour of the underlying stream (also accounting for custom implementations) - i.e. if it's already disposed or if it's in a state where the implementation decides disposal is impossible - I can't really see what else to do.

Obviously I shouldn't be hiding bad implementations of Stream's Dispose(); but I think there is a half-way-house here.

In particular is the fact that this object doesn't necessarily 'own' the stream, and therefore won't have to be always responsible for Dispose()ing. It should therefore be tolerant to when the str开发者_StackOverflow中文版eam has been disposed by somebody else.

Clearly, in most cases this won't be an issue; get a file in a using block perhaps, read it, do something with it and then dispose of it. But there's those edge-cases; and they bother me!

Initially I wrote it as catch(ObjectDisposedException){ } - but that's only an informal pattern that can't be relied upon either.

What should I do? Keep it as this, despite the fact that exception gulping is bad karma? Gulp only ObjectDisposedException, even though that's not much better? Or should I just not have any exception gulping or handling at all?

I can see merits in all cases; just can't figure out which of them wins.


Generally, if you can't figure out which one wins, it's because there's going to be useful information that can't be handled "generically". In this case, I think it preferable to let the caller deal with it, because they'll be in a concrete situation that allows them to better judge.

How is the stream introduced to this class? If you can engineer such that it is passed in, then why not let the caller deal with its disposal and not concern yourself with it. It's always bugged me that StreamReader takes on this responsibility. I wouldn't much care if it didn't.

DO NOT SWALLOW the exception, especially in code intended for external consumption.


As you have said, if you do not own the disposable, then you should not dispose it. You should not be even temped to call Dispose.

Have a look at my answer here.

Disposing of arguments for an iterator block


UPDATE

OK, the question of whether you own the disposable is not answered by who created the disposable. Ownership can be passed to other classes. StreamReader will assume ownership as soon as a stream is passed to it. So the creator of stream does not have to calls dispose and can rely on StreamReader to do it.


I suggest that you want to implement your dispose pattern according to the Microsoft guidelines, If the client of your wrapper has "pulled the rug" on the base stream you should let the exception get to them so they can deal with it or, at least know about it.

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜