Как вы предотвращаете распространение IDisposable на все ваши классы? - PullRequest
Новая
       63

Как вы предотвращаете распространение IDisposable на все ваши классы?

133 голосов
/ 19 марта 2009

Начните с этих простых классов ...

Допустим, у меня есть простой набор классов, подобный этому: 

class Bus
{
    Driver busDriver = new Driver();
}

class Driver
{
    Shoe[] shoes = { new Shoe(), new Shoe() };
}

class Shoe
{
    Shoelace lace = new Shoelace();
}

class Shoelace
{
    bool tied = false;
}

A Bus имеет Driver, Driver имеет два Shoe с, каждый Shoe имеет Shoelace. Все очень глупо.

Добавление идентифицируемого объекта в шнурок

Позже я решаю, что некоторые операции с Shoelace могут быть многопоточными, поэтому я добавляю EventWaitHandle, чтобы потоки могли обмениваться данными. Итак, Shoelace теперь выглядит так: 

class Shoelace
{
    private AutoResetEvent waitHandle = new AutoResetEvent(false);
    bool tied = false;
    // ... other stuff ..
}

Орудие IDisposable на шнурке

Но теперь Microsoft FxCop будет жаловаться: "Реализация IDisposable на 'Shoelace', поскольку она создает члены следующих типов IDisposable: 'EventWaitHandle'."

Хорошо, я реализую IDisposable на Shoelace, и мой аккуратный маленький класс превращается в этот ужасный беспорядок: 

class Shoelace : IDisposable
{
    private AutoResetEvent waitHandle = new AutoResetEvent(false);
    bool tied = false;
    private bool disposed = false;

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    ~Shoelace()
    {
        Dispose(false);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (!this.disposed)
        {
            if (disposing)
            {
                if (waitHandle != null)
                {
                    waitHandle.Close();
                    waitHandle = null;
                }
            }
            // No unmanaged resources to release otherwise they'd go here.
        }
        disposed = true;
    }
}

Или (как отмечают комментаторы), поскольку Shoelace сам не имеет неуправляемых ресурсов, я мог бы использовать более простую реализацию dispose без использования Dispose(bool) и Destructor: 

class Shoelace : IDisposable
{
    private AutoResetEvent waitHandle = new AutoResetEvent(false);
    bool tied = false;

    public void Dispose()
    {
        if (waitHandle != null)
        {
            waitHandle.Close();
            waitHandle = null;
        }
        GC.SuppressFinalize(this);
    }
}

С ужасом наблюдайте за тем, как IDisposable распространяется

Правильно, это исправлено. Но теперь FxCop будет жаловаться, что Shoe создает Shoelace, поэтому Shoe тоже должно быть IDisposable.

И Driver создает Shoe, поэтому Driver должно быть IDisposable. И Bus создает Driver, поэтому Bus должно быть IDisposable и т. Д.

Внезапно мое небольшое изменение на Shoelace вызывает у меня много работы, и мой босс задается вопросом, почему мне нужно оформить заказ Bus, чтобы внести изменения в Shoelace.

Вопрос

Как вы предотвратите это распространение IDisposable, но при этом убедитесь, что ваши неуправляемые объекты правильно расположены?

Ответы [ 8 ]

34 голосов
/ 19 марта 2009

Вы не можете «предотвратить» распространение IDisposable. Некоторые классы должны быть расположены, например, AutoResetEvent, и самый эффективный способ - сделать это в методе Dispose(), чтобы избежать накладных расходов финализаторов. Но этот метод должен вызываться каким-то образом, так же, как в вашем примере классы, которые инкапсулируют или содержат IDisposable, должны утилизировать их, поэтому они также должны быть одноразовыми и т. Д. Единственный способ избежать этого - это:

  • по возможности избегайте использования классов IDisposable, блокируйте или ожидайте события в одном месте, храните дорогие ресурсы в одном месте и т. Д.
  • создавайте их только тогда, когда они вам нужны, и утилизируйте их сразу после (шаблон using)

В некоторых случаях IDisposable можно игнорировать, поскольку он поддерживает необязательный регистр. Например, WaitHandle реализует IDisposable для поддержки именованного Mutex. Если имя не используется, метод Dispose ничего не делает. Еще один пример - MemoryStream: он не использует системные ресурсы и его реализация Dispose также ничего не делает. Тщательное размышление о том, используется ли неуправляемый ресурс или нет, может быть учебным. Так же можно изучить доступные источники для библиотек .net или использовать декомпилятор.

19 голосов
/ 19 марта 2009

С точки зрения корректности, вы не можете предотвратить распространение IDisposable через объектные отношения, если родительский объект создает и по существу владеет дочерним объектом, который теперь должен быть одноразовым. FxCop корректен в этой ситуации, и родитель должен быть IDisposable.

Что вы можете сделать, так это избегать добавления IDisposable к листовому классу в вашей иерархии объектов. Это не всегда простая задача, но интересное упражнение. С логической точки зрения, нет причин, по которым ShoeLace должен быть одноразовым. Вместо добавления здесь WaitHandle можно также добавить ассоциацию между ShoeLace и WaitHandle в той точке, в которой они используются. Самый простой способ - через экземпляр Dictionary.

Если вы можете переместить WaitHandle в свободную ассоциацию через карту в точке, где фактически используется WaitHandle, вы можете разорвать эту цепочку.

14 голосов
/ 09 июня 2010

Чтобы предотвратить распространение IDisposable, вы должны попытаться инкапсулировать использование одноразового предмета внутри одного метода. Попробуйте оформить Shoelace по-другому: 

class Shoelace { 
  bool tied = false; 

  public void Tie() {

    using (var waitHandle = new AutoResetEvent(false)) {

      // you can even pass the disposable to other methods
      OtherMethod(waitHandle);

      // or hold it in a field (but FxCop will complain that your class is not disposable),
      // as long as you take control of its lifecycle
      _waitHandle = waitHandle;
      OtherMethodThatUsesTheWaitHandleFromTheField();

    } 

  }
}

Область действия дескриптора ожидания ограничена методом Tie, и у класса не должно быть одноразового поля, и поэтому он не должен быть одноразовым сам.

Поскольку дескриптор ожидания - это деталь реализации внутри Shoelace, он никоим образом не должен изменять свой открытый интерфейс, как добавление нового интерфейса в его объявлении. Что произойдет, если вам больше не нужно одноразовое поле, удалите ли вы декларацию IDisposable? Если вы думаете об абстракции Shoelace , вы быстро понимаете, что она не должна быть загрязнена инфраструктурными зависимостями, такими как IDisposable. IDisposable должен быть зарезервирован для классов, абстракция которых инкапсулирует ресурс, который требует детерминированной очистки; то есть для классов, где одноразовость является частью абстракции .

3 голосов
/ 06 января 2010

Я не думаю, что есть технический способ предотвратить распространение IDisposable, если вы сохраняете свой дизайн так тесно связанным. Тогда нужно задаться вопросом, правильный ли дизайн.

В вашем примере, я думаю, имеет смысл иметь туфлю на шнуровке, и, возможно, водитель должен владеть своей туфлей. Однако автобус не должен владеть водителем. Как правило, водители автобусов не следуют за автобусами на свалку :) В случае водителей и обуви, водители редко делают свою обувь, а это значит, что они на самом деле их не "владеют".

Альтернативный дизайн может быть: 

class Bus
{
   IDriver busDriver = null;
   public void SetDriver(IDriver d) { busDriver = d; }
}

class Driver : IDriver
{
   IShoePair shoes = null;
   public void PutShoesOn(IShoePair p) { shoes = p; }
}

class ShoePairWithDisposableLaces : IShoePair, IDisposable
{
   Shoelace lace = new Shoelace();
}

class Shoelace : IDisposable
{
   ...
}

Новый дизайн, к сожалению, более сложен, так как требует дополнительных классов для создания экземпляров и утилизации конкретных экземпляров обуви и водителей, но это усложнение присуща решаемой проблеме. Хорошо, что автобусы больше не должны быть одноразовыми просто с целью утилизации шнурков.

3 голосов
/ 19 марта 2009

Интересно, если Driver определено как указано выше: 

class Driver
{
    Shoe[] shoes = { new Shoe(), new Shoe() };
}

Затем, когда Shoe сделано IDisposable, FxCop (v1.36) не жалуется, что Driver также должно быть IDisposable.

Однако, если он определен так: 

class Driver
{
    Shoe leftShoe = new Shoe();
    Shoe rightShoe = new Shoe();
}

тогда он будет жаловаться.

Я подозреваю, что это всего лишь ограничение FxCop, а не решение, потому что в первой версии Shoe экземпляры все еще создаются Driver и все равно должны быть как-то удалены.

3 голосов
/ 19 марта 2009

Это похоже на проблему проектирования более высокого уровня, как это часто бывает, когда «быстрое исправление» превращается в трясину. Для более подробного обсуждения путей вы можете найти эту тему полезной.

2 голосов
/ 19 марта 2009

Это в основном то, что происходит, когда вы смешиваете составление или агрегирование с одноразовыми классами. Как уже упоминалось, первым выходом будет рефакторинг waitHandle из шнурка.

Сказав это, вы можете значительно сократить шаблон Disposable, если у вас нет неуправляемых ресурсов. (Я все еще ищу официальную ссылку для этого.)

Но вы можете опустить деструктор и GC.SuppressFinalize (this); и, возможно, немного очистить виртуальную пустоту. Утилизировать (утилизировать).

1 голос
/ 02 июля 2014

Как насчет использования Inversion of Control? 

class Bus
{
    private Driver busDriver;

    public Bus(Driver busDriver)
    {
        this.busDriver = busDriver;
    }
}

class Driver
{
    private Shoe[] shoes;

    public Driver(Shoe[] shoes)
    {
        this.shoes = shoes;
    }
}

class Shoe
{
    private Shoelace lace;

    public Shoe(Shoelace lace)
    {
        this.lace = lace;
    }
}

class Shoelace
{
    bool tied;
    private AutoResetEvent waitHandle;

    public Shoelace(bool tied, AutoResetEvent waitHandle)
    {
        this.tied = tied;
        this.waitHandle = waitHandle;
    }
}

class Program
{
    static void Main(string[] args)
    {
        using (var leftShoeWaitHandle = new AutoResetEvent(false))
        using (var rightShoeWaitHandle = new AutoResetEvent(false))
        {
            var bus = new Bus(new Driver(new[] {new Shoe(new Shoelace(false, leftShoeWaitHandle)),new Shoe(new Shoelace(false, rightShoeWaitHandle))}));
        }
    }
}
...