2013-07-30 2 views
16

У меня есть подкласс DbContextДекораторы и IDisposable

public class MyContext : DbContext { } 

и у меня есть IUnitOfWork абстракции вокруг MyContext, который реализует IDisposable, чтобы гарантировать, что ссылки, такие как MyContext утилизированы в соответствующее время

public interface IUnitOfWork : IDisposable { } 

public class UnitOfWork : IUnitOfWork 
{ 
    private readonly MyContext _context; 

    public UnitOfWork() 
    { 
     _context = new MyContext(); 
    } 

    ~UnitOfWork() 
    { 
     Dispose(false); 
    } 

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

    private bool _disposed; 

    protected virtual void Dispose(bool disposing) 
    { 
     if (_disposed) return; 

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

     _disposed = true; 
    } 
} 

Мой UnitOfWork зарегистрирован на весь срок службы (веб-запроса). У меня есть декораторы IUnitOfWork, которые могут быть зарегистрированы как переходные или пожизненные, и мой вопрос заключается в том, что они должны делать в отношении реализации IDisposable - особенно если они или не должны пройти по вызову Dispose().

public class UnitOfWorkDecorator : IUnitOfWork 
{ 
    private readonly IUnitOfWork _decorated; 

    public UnitOfWorkDecorator(IUnitOfWork decorated) 
    { 
     _decorated = decorated; 
    } 

    public void Dispose() 
    { 
     //do we pass on the call? 
     _decorated.Dispose(); 
    } 
}  

Я вижу 2 варианта (я предполагаю, что вариант 2 правильный ответ):

  1. Ожидается, что каждый декоратор будет знать, является ли она временной или срок службы области видимости. Если декоратор временно, то он не должен называть Dispose() на украшенном экземпляре. Если это срок действия, он должен.
  2. Каждый декоратор должен заботиться только об утилизации себя и должен никогда передать вызов украшенному экземпляру. Контейнер будет управлять вызовом Dispose() для каждого объекта в цепочке вызовов в соответствующее время. Объект должен только Dispose() экземпляров, которые он инкапсулирует и украшает, не является инкапсуляцией.
+2

Отличный вопрос. Кроме того, вы должны установить переменную _context в * null * в методе Dispose сразу после того, как вы настроили этот контекст. – Maarten

+1

Я согласен с @Maarten: Отличный и очень интересный вопрос. – Steven

ответ

12

, что должно [декораторы] делать в отношении реализации IDisposable

Это возвращается к общему принципу права собственности. Спросите себя: «Кто владеет этим одноразовым типом?».Ответ на этот вопрос: тот, кто владеет типом, несет ответственность за его удаление.

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

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

Каждый декоратор должен заниматься только с утилизацией себе и не должны не проходят по вызову украшенной инстанции.

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

С другой стороны, ваш UnitOfWork создает новый класс MyContext и, следовательно, имеет право владения этим экземпляром, и он должен распоряжаться им.

Существуют исключения из этого правила, но до сих пор остается в собственности. Иногда вы передаете право собственности на тип другим. Например, при использовании фабричного метода, по соглашению, фабричный метод передает права собственности на созданный объект на вызывающего. Иногда право собственности передается на созданный объект, например класс .NET StreamReader. В документации API это ясно, но, поскольку дизайн такой неинтуитивный, разработчики продолжают спотыкаться об этом. Большинство типов в .NET Framework не работают таким образом. Например, класс SqlCommand не удаляет SqlConnection, и было бы очень неприятно, если бы оно удалило соединение.

Другой способ взглянуть на этот вопрос - с точки зрения SOLID principles. Предоставив IUnitOfWork орудие IDisposable, вы нарушаете Dependency Inversion Principle, так как «Абстракции не должны зависеть от деталей, детали должны зависеть от абстракций». Внедряя IDisposable, вы просачиваете детали реализации в интерфейс IUnitOfWork. Реализация IDisposable означает, что у класса есть неуправляемые ресурсы, которые нуждаются в удалении, такие как дескрипторы файлов и строки подключения. Это детали реализации, потому что вряд ли когда-либо будет так, что каждая реализация такого интерфейса фактически нуждается в утилизации вообще. Вам просто нужно создать одну фальшивую или макетную реализацию для ваших модульных тестов, и у вас есть доказательство реализации, которая не требует удаления.

Итак, когда вы исправить это нарушение DIP, удаляя интерфейс IDisposable из IUnitOfWork й переместив его в реализации-, становится невозможным для декоратор распоряжаться decoratee, потому что это не имеет никакого способа узнать, является ли или нет decoratee реализует IDisposable. И это хорошо, потому что, согласно DIP, декоратор не должен знать - и мы уже установили, что декоратор не должен распоряжаться декоратором.

+1

Вы предоставляете такой хороший ответ, как всегда. Наверное, я влюбляюсь в тебя :). – crypted

+0

Всегда приятно иметь поклонников :-) – Steven

+0

@Steven, так как я могу обеспечить удаление чего-то вроде 'IDbConnection' (возможно, хромого примера, но это еще пример) в то время, когда' UnitOfWork' выходит за рамки? – qujck

3

Лично я подозреваю, что вам необходимо обработать это на индивидуальной основе. У некоторых декораторов могут быть веские причины понять область видимости; для большинства, это, вероятно, хороший дефолт, чтобы просто передать его. Очень немногие должны быть явно never распоряжаться цепью - основное время, которое я видел, что это было специально, чтобы противодействовать сценарию, в котором другой декоратор, который должен рассмотрел область охвата: не всегда (всегда удаляется).

В качестве родственного примера рассмотрим такие вещи, как GZipStream - для большинства людей они имеют дело только с одним логическим куском - так что по умолчанию «распоряжаться потоком» отлично; но это решение доступно через constructor overload, который позволяет вам сообщить об этом. В последних версиях C# с дополнительными параметрами это можно было бы сделать в одном конструкторе.

Вариант 2 является проблематичным, так как он требует от вас (или контейнера) отслеживать все промежуточные объекты; если ваш контейнер делает это удобно, то хорошо, но также обратите внимание, что они должны быть расположены в правильном порядке (внешний для внутреннего). Потому что в цепочке декоратора могут быть ожидающие операции - запланированные для сброса по потоку по запросу или (в крайнем случае) во время удаления.

+1

Я почти никогда не соглашаюсь с вашими комментариями, но на этот раз я действительно делаю. Но мне все равно нужно дать вам +1, потому что после прочтения вашего ответа я понял, что есть ошибка в Simple Injector. S.I. распоряжается экземплярами в неправильном порядке. Как вы сказали, экземпляр должен быть удален из внешнего в внутренний, но это не то, что происходит в версии 2.3. – Steven

+0

@Steven, с каким вы не согласны, из любопытства? –

+0

Вы прочитали мой ответ? Вы даете 'GZipStream' в качестве примера класса, который предоставляет ресурс, который он получает извне, но этот проект идет вразрез с основными принципами разработки каркаса и является противоположностью того, как большинство реализаций работают .NET.Без контейнера вы бы использовали серию операторов 'using' для размещения вложенного графа, и очень немногие реализации должны фактически распоряжаться цепочкой, так как это поведение, которое обычно обычно не ожидает пользователей, и запрещает повторное использование внешних объектов. Внутренние объекты просто не могут знать, должны ли их родители повторно использоваться или нет. – Steven

6

Не ответ, но ваш UnitOfWork может быть упрощен.

  • Поскольку сам класс не имеет родных ресурсов, нет необходимости в его финализаторе. Поэтому финализатор можно удалить.
  • Контракт интерфейса IDisposable гласит, что он действителен для Dispose, который будет вызываться несколько раз. Это не должно приводить к исключению или к любому другому наблюдаемому поведению. Поэтому вы можете удалить флаг _disposed и отметку if (_disposed).
  • Поле _context всегда будет инициализироваться, когда конструктор успешно завершит успешное выполнение, и Dispose никогда не может вызываться, когда конструктор генерирует исключение. Поэтому проверка if (_context != null) является излишней. Поскольку DbContext можно безопасно удалять несколько раз, нет необходимости его аннулировать.
  • Реализация шаблона утилизации (с защищенным методом Dispose(bool)) требуется только тогда, когда тип предназначен для наследования. Шаблон особенно полезен для типов, которые являются частью многоразовой рамки, поскольку нет контроля над тем, кто наследует этот тип. Если вы сделаете этот тип sealed, вы можете безопасно удалить защищенный метод Dispose(bool) и переместить его логику в общедоступный метод Dispose().
  • Поскольку тип не содержит финализатор и не может быть унаследован, вы можете удалить вызов GC.SuppressFinalize.

Следуя этим шагам, это то, что осталось от типа UnitOfWork:

public sealed class UnitOfWork : IUnitOfWork, IDisposable 
{ 
    private readonly MyContext _context; 

    public UnitOfWork() 
    { 
     _context = new MyContext(); 
    } 

    public void Dispose() 
    { 
     _context.Dispose(); 
    } 
} 

В случае вы переместили создание MyContext из UnitOfWork путем введения его в UnitOfWork, вы даже можете упростить UnitOfWork для следующее:

public sealed class UnitOfWork : IUnitOfWork 
{ 
    private readonly MyContext _context; 

    public UnitOfWork(MyContext context) 
    { 
     _context = context; 
    } 
} 

с UnitOfWork принимает MyContext не иметь ownersh ip over, не разрешается удалять MyContext (поскольку другой потребитель может по-прежнему требовать его использования, даже после того, как UnitOfWork выходит за рамки). Это означает, что UnitOfWork не нужно ничего утилизировать и, следовательно, не нужно реализовывать IDisposable.

Это, конечно, означает, что мы переносим ответственность за удаление MyContext до «кого-то еще». Этот «кто-то» обычно будет тем же самым, который контролировал создание и удаление UnitOfWork. Обычно это Composition Root.

+0

шаблон, над которым я работал, появился здесь http://lostechies.com/chrispatterson/2012/11/29/idisposable-done-right/ – qujck

+0

Практика создания шаблонов статей почти прямо из Руководства по дизайну рамок, но есть тонкие различия и некоторые рекомендации изменились во втором издании. Хотя важно применять шаблоны, важно понять их. Шаблон размещения предназначен для иерархии наследования. Если вам не нужна такая иерархия, все становится намного проще. Если у вас нет родных ресурсов, все становится проще. – Steven

+0

Также взгляните на комментарии. Кажется, есть много консенсуса с тем, что я говорю :-) – Steven

Смежные вопросы