Я правильно ли реализую IDisposable?
Этот класс использует StreamWriter
и поэтому реализует IDisposable
.
public class Foo : IDisposable
{
private StreamWriter _Writer;
public Foo (String path)
{
// here happens something along the lines of:
FileStream fileWrite = File.Open (path, FileMode.OpenOrCreate, FileAccess.Write, FileShare.ReadWrite);
_Writer = new StreamWriter (fileWrite, new ASCIIEncoding ());
}
public void Dispose ()
{
Dispose (true);
GC.SuppressFinalize (this);
}
~Foo()
{
Dispose (false);
}
protected virtual void Dispose (bool disposing)
{
if (_Disposed) {
return;
}
if (disposing) {
_Writer.Dispose ();
}
_Writer = null;
_Disposed = true;
}
private bool _Disposed;
}
}
Есть ли какие-либо проблемы с текущей реализацией? I.e., должен ли я освободить базовый FileStream
вручную? Правильно ли написано Dispose(bool)
?
Ответы
Ответ 1
Вам не нужно использовать эту расширенную версию реализации IDisposable, если ваш класс напрямую не использует неуправляемые ресурсы.
Простой
public virtual void Dispose()
{
_Writer.Dispose();
}
будет достаточно.
Если ваш потребитель не удалит свой объект, он будет GC'd обычно без вызова Dispose, объект, хранящийся _Writer, также будет GC'd, и у него будет финалист, поэтому он все равно будет очищать свой неуправляемый ресурсов.
Edit
Проведя некоторые исследования по ссылкам, предоставленным Мэттом и другими, я пришел к выводу, что мой ответ здесь. Вот почему: -
Предпосылка "шаблона" одноразовой реализации (под этим я подразумеваю, что защищенный виртуальный Dispose (bool), SuppressFinalize и т.д. marlarky) в наследуемом классе заключается в том, что подкласс может удерживать к неуправляемому ресурсу.
Однако в реальном мире подавляющее большинство из нас, разработчиков .NET, никогда не приближаются к неуправляемому ресурсу. Если вам нужно было количественно определить ", то какой вероятностью вы придумаете для своего рода .NET-кодирование?
Предположим, что у меня есть тип Person (который для аргумента имеет одноразовый тип в одном из своих полей и, следовательно, должен быть сам по себе). Теперь у меня есть наследники Customer, Employee и т.д. Действительно ли мне разумно загромождать класс Person этим "Шаблоном" на случай, если кто-то наследует Person и хочет сохранить неуправляемый ресурс?
Иногда разработчики могут усложнять ситуацию, пытаясь кодировать все возможные обстоятельства, не используя здравого смысла относительно относительной вероятности таких обстоятельств.
Если бы мы когда-либо хотели использовать неуправляемый ресурс, то разумный шаблон мог бы обернуть такую вещь в своем классе, где полный "одноразовый шаблон" был бы разумным. Следовательно, в значительно большем теле "нормального" кода нам не нужно беспокоиться обо всем этом. Если нам нужно IDisposable, мы можем использовать простой шаблон выше, наследуемый или нет.
Фу, рад, что сдержу это с моей груди.;)
Ответ 2
Вам не нужен метод Finalize
(деструктор), так как у вас нет неуправляемых объектов. Тем не менее, вы должны сохранить вызов GC.SuppressFinalize
, если класс, наследующий Foo, имеет неуправляемые объекты и, следовательно, финализатор.
Если вы сделаете класс запечатанным, то вы знаете, что неуправляемые объекты никогда не войдут в уравнение, поэтому нет необходимости добавлять перегрузку protected virtual Dispose(bool)
или GC.SuppressFinalize
.
Edit:
@AnthonyWJones возражает против этого, так как если вы знаете, что подклассы не будут ссылаться на неуправляемые объекты, то все Dispose(bool)
и GC.SuppressFinalize
не нужны. Но если это так, вы должны действительно сделать классы internal
, а не public
, а метод Dispose()
должен быть virtual
. Если вы знаете, что делаете, то не стесняйтесь следовать шаблону Microsoft, но вы должны знать и понимать правила, прежде чем их нарушать!
Ответ 3
Рекомендуемая практика заключается в использовании финализатора только тогда, когда у вас есть неуправляемые ресурсы (например, собственные файлы, указатели на память и т.д.).
У меня есть два небольших предложения, хотя
Нет необходимости иметь "m_Disposed
" для проверки, если вы ранее называли Dispose
на ваших управляемых ресурсах. Ты мог используйте аналогичный код, например,
protected virtual void Dispose (bool disposing)
{
if (disposing) {
if (_Writer != null)
_Writer.Dispose ();
}
_Writer = null;
}
Откройте файлы только до тех пор, пока это необходимо. Итак, в вашем примере вы проверили бы существование файла в конструкторе с помощью File.Exists
, а затем, когда вам нужно будет прочитать/записать файл, вы откроете его и используйте его.
Кроме того, если вы просто хотите написать текст в файл, посмотрите File.WriteAllText
или File.OpenText
или даже File.AppendText
, который предназначен для текстовых файлов специально с помощью ASCIIEncoding
.
Кроме того, да, вы правильно реализуете шаблон .NET Dispose
.
Ответ 4
У меня много таких классов, и моя рекомендация - сделать класс запечатанным, когда это возможно. IDisposable
+ наследование может работать, но в 99% случаев вам это не нужно - и с этим легко ошибиться.
Если вы не пишете публичный API (в этом случае вам будет приятно разрешить людям реализовать ваш код, но они хотят - то есть использовать IDisposable
), вам не нужно его поддерживать.
просто выполните:
public sealed class Foo : IDisposable
{
private StreamWriter _Writer;
public Foo(string path)
{
FileStream fileWrite = File.Open (path, FileMode.OpenOrCreate, FileAccess.Write, FileShare.ReadWrite);
try {
_Writer = new StreamWriter (fileWrite, new ASCIIEncoding());
} catch {
fileWrite.Dispose();
throw;
}
}
public void Dispose()
{
_Writer.Dispose();
}
}
Обратите внимание на try... catch; технически конструктор StreamWriter
может генерировать исключение, и в этом случае он никогда не получает права собственности на FileStream
, и вы должны распоряжаться им самостоятельно.
Если вы действительно используете много IDisposable
s, подумайте о том, чтобы поместить этот код в С++/CLI: он делает для вас весь этот шаблонный код (он прозрачно использует соответствующие технологии детерминированного уничтожения как для собственных, так и для управляемых объектов).
В Википедии есть достойный образец IDisposable для С++ (действительно, если у вас много IDisposable, С++ на самом деле намного проще С#):
Википедия: Финализаторы С++/CLI и автоматические переменные.
Например, реализация "безопасного" одноразового контейнера в С++/CLI выглядит как...
public ref class MyDisposableContainer
{
auto_handle<IDisposable> kidObj;
auto_handle<IDisposable> kidObj2;
public:
MyDisposableContainer(IDisposable^ a,IDisposable^ b)
: kidObj(a), kidObj2(b)
{
Console::WriteLine("look ma, no destructor!");
}
};
Этот код будет правильно размещать kidObj и kidObj2, даже не добавляя настраиваемую реализацию IDisposable, и он устойчив к исключениям в реализации Dispose (не для того, чтобы это происходило, но все же), и просто для поддержки перед многими другими IDisposable
членов или собственных ресурсов.
Не то, чтобы я был огромным поклонником С++/CLI, но для сильно ресурсо-ориентированного кода он легко справился с С#, и он обладает абсолютно блестящим взаимодействием как с управляемым, так и с собственным кодом - короче говоря, совершенным кодом клея;). Я хочу писать 90% моего кода на С#, но использовать С++/CLI для всех запросов interop (особенно если вы хотите вызвать любую функцию win32 - MarshalAs и другие атрибуты interop повсюду ужасающие и полностью непонятные).
Ответ 5
Вы должны проверить, что _Writer
не null
, прежде чем пытаться его уничтожить. (Похоже, что это никогда не будет null
, но на всякий случай!)
protected virtual void Dispose(bool disposing)
{
if (!_Disposed)
{
if (disposing && (_Writer != null))
{
_Writer.Dispose();
}
_Writer = null;
_Disposed = true;
}
}
Ответ 6
Если вы откроете StreamWriter, вам также придется утилизировать его, иначе у вас будет утечка.
Ответ 7
Я согласен со всем сказанным в других комментариях, но также хотел бы отметить, что
-
На самом деле вам не нужно устанавливать _Writer = null в любом случае.
-
Если вы собираетесь сделать это, лучше всего поместить его в папку if, где находится утилита. Вероятно, это не делает большой разницы, но вы, как правило, не должны играть с управляемыми объектами при размещении финализатором (что вам в этом случае не нужно, как указывали другие).