Подтвердить что ты не робот

Я правильно ли реализую 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)?

4b9b3361

Ответ 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, где находится утилита. Вероятно, это не делает большой разницы, но вы, как правило, не должны играть с управляемыми объектами при размещении финализатором (что вам в этом случае не нужно, как указывали другие).