10

Доброго времени суток! У меня есть такой вопрос. Допустим, у меня есть класс, реализующий интерфейс IDisposable. У этого класса есть какой-то управляемый ресурс, который тоже реализует этот интерфейс. Вот минимальный код моего класса.

class MyClass : IDisposable
{
    private SomeContext _context = new SomeContext();

    public void Dispose() 
    {
        if(_context != null)    
            _context.Dispose();
    }
}

И тут у меня возник вопрос. Насколько правильна эта реализация? Точнее насколько она неправильна и чем это грозит? (везде где смотрел я видел более сложную реализацию Dispose паттерна). Вроде бы имеется всего одно IDisposable поле (предполагается что оно корректно реализует этот паттерн в своих внутренностях), и чтобы освободить ресурс корректно, достаточно просто в нужное время вызвать метод Dispose для поля _context. В чем я не прав? Применима ли такая простая реализация на практикке и чем она вообще плоха?

6
  • Сейчас найду дубликат :)
    – VladD
    Commented 22 нояб. 2016 в 13:58
  • 4
    ru.stackoverflow.com/q/486696/10105
    – VladD
    Commented 22 нояб. 2016 в 13:58
  • @VladD, а чего не бамкнул?
    – Grundy
    Commented 22 нояб. 2016 в 14:01
  • @Grundy, это разве вопросы. Там про сам IDisposable, а тут - про конкретную реализацию.
    – Qwertiy
    Commented 22 нояб. 2016 в 14:02
  • 2
    Dispose Pattern из Framework Design Guidelines (обязательно к прочтению полностью). Commented 22 нояб. 2016 в 14:30

2 ответа 2

9

Всё правильно, но можно убрать проверку на null.

Если класс сконструировался, то _context не равно null, т. к. new не может вернуть null.

Итого:

class MyClass : IDisposable
{
    SomeContext _context = new SomeContext();    
    public void Dispose() => _context.Dispose();
}

Обратите внимание, иногда уместно использовать такую конструкцию:

class MyClass : IDisposable
{
    bool isDisposed = false;
    void EnsureSelfAlive()
    {
        if (isDisposed)
            throw new ObjectDisposedException();
    }

    SomeContext _context = new SomeContext();    

    public void Dispose()
    {
        isDisposed = true;
        _context.Dispose();
    }
}

и в каждом публичном методе в начале вызывать EnsureSelfAlive(). Позволяет отлавливать какое-то количество ошибок.

Проверять isDisposed в Dispose не нужно, т. к. _context должен уметь «пережить» множественные вызовы _context.Dispose().

10
  • почему? а если кто-то где-то присвоит значение null? :)
    – Grundy
    Commented 22 нояб. 2016 в 14:01
  • @Grundy: Ну это если :) В текущем коде не нужно.
    – VladD
    Commented 22 нояб. 2016 в 14:03
  • Эм.. а если там исключение в самом new SomeContext()?
    – Qwertiy
    Commented 22 нояб. 2016 в 14:03
  • @Qwertiy, Тогда исключение улетит вверх, оно ж нигде не перехватывается, и не сконструируется основновной класс
    – Grundy
    Commented 22 нояб. 2016 в 14:04
  • 2
    @Qwertiy: Главное — конечно, семантический бонус. Но ещё например мы можем уже закрыть нужный файл, или отписаться от какого-то ивента или что-то ещё, и быть в нерабочем состоянии. Ну или просто разрушить внутренний инвариант. В этой ситуации мы просто не можем продолжать работать, и надо бы кинуть правильное исключение, а не крешнуться по NullReferenceException.
    – VladD
    Commented 22 нояб. 2016 в 15:05
6

Да, в этой схеме всё верно. Неуправляемых ресурсов нет, финализатор не нужен. Dispose вызов прокидывает дальше, т. е. своё дело делает.

А вообще, рекомендую почитать: http://sergeyteplyakov.blogspot.ru/2011/09/dispose-pattern.html, особенно раздел Упрощенная версия Dispose паттерна, который очень похож на этот вопрос.

Ваш ответ

Нажимая «Отправить ответ», вы соглашаетесь с условиями пользования и подтверждаете, что прочитали политику конфиденциальности.

Всё ещё ищете ответ? Посмотрите другие вопросы с метками или задайте свой вопрос.