2012-01-21 14 views
7

Przeczytałem ostatnio Effective C# i kilka innych takich książek/blogów, a gdy mówimy o standard Dispose pattern (którego już używam), wszystkie one polecają używanie zmiennej "dispose (jak zdefiniowano w tym kodzie przykładowym MSDN) na początku każdej metody. Zasadniczo, aby upewnić się, że po wywołaniu Dispose, każda próba użycia obiektu spowodowałaby ObjectDisposedException. Ma to sens, ale jest ogromną ilością pracy ręcznej w wystarczająco dużej bazie kodu i polega na tym, że ludzie pamiętają, aby to zrobić. Więc szukam lepszego sposobu.Pomocnik kodu dla standardowego wzoru usuwania?

Niedawno natknąłem się i zacząłem używać notifypropertyweaver, który automatycznie wypełnia cały standardowy kod wywołujący procedury obsługi PropertyChanged (działa jako zadanie msbuild, więc nie wymaga dodatkowej zależności od wysyłki). Zastanawiam się, czy ktokolwiek wie o podobnym rozwiązaniu dla standardowego wzoru utylizacji. Co to w zasadzie zrobić, to zaakceptować nazwę zmiennej jako config (The bool disposed w przypadku próbki MSDN), a następnie dodaj następujący kod do każdej metody, która nie jest Finalizer lub nazwie Usuwać w każdej klasie, która implementuje IDisposable:

if(disposed) 
    throw new ObjectDisposedException(); 

Czy coś takiego istnieje? Ewentualnie, co ludzie robią, aby osiągnąć to w swoim kodzie, ręcznie dodaj instrukcję if?

Wyjaśnienie Cel
zwiększonemu zapotrzebowaniu na to nie jest jakiś „najlepszych praktyk” twardy, ale że mamy użytkownikom Zarządzanie cykli życia naszych obiektów nieprawidłowo. W tej chwili otrzymują NullReference lub inną podobną rzecz z bazowego zasobu, co może oznaczać, że mamy błąd w naszej bibliotece, chcę im przekazać, że to oni tworzą problem i jak go tworzą (jestem w stanie to wiedzieć). Zatem sugestie, że użytkownicy naszych typów są tymi, którzy powinni się tym zająć, nie są tu naprawdę produktywne.

+0

Nie znam żadnej z takich rzeczy ... Zazwyczaj dodaje się to ręcznie w prostszych klasach ... w klasach złożonych Mam centralną metodę wywołania 'Zapewnienie niezawrotności', która jest nazywana jako pierwsza w dowolnej metodzie i dba o to kilku istotnych aspektów, na które reszta klasy polega na ... że kontrola jest częścią mojego kodu 'Zapewnienie niezawisłości' ... – Yahia

+2

Pisanie tego kodu jest bardzo silnym wskaźnikiem, że używasz IDisposable źle. Możesz ją zaimplementować tylko wtedy, gdy klasa ma pola typu implementującego IDisposable. Twoja metoda Dispose() po prostu wywołuje metodę Dispose(). Generowanie ObjectDisposedException jest zadaniem * tych * klas, nie twoich. Wprowadzasz jednorazowy wzorzec tylko wtedy, gdy twoja klasa ma finalizatora. To też robi źle, powinieneś zawsze zawinąć finalizowalny zasób we własnej klasie. Jak SafeHandle. –

+1

@Hans Passant Twój argument nie ma sensu. Wprowadza nowy wektor błędu (ponieważ polega na tym, że ludzie używają poprawnie obiektu, na co prawie nigdy nie można polegać). Ostatecznie to twój obiekt jest rzeczą, która wie, czy dispose został wywołany, i musi zabronić nielegalnego korzystania z niego z komunikatem o błędzie (wyjątek tutaj), który kształci użytkowników w błąd ich sposobów. –

Odpowiedz

2

Aktualizacja: Moja oryginalna odpowiedź tak naprawdę nie odpowiedziała na pytanie, więc oto kolejna próba ...

Aby pomóc w rozwiązaniu problemu rootowego, programiści zapominają rzucić ObjectDisposedExceptions, być może zautomatyzowane testowanie jednostkowe może załatwić sprawę. Jeśli chcesz być rygorystycznie wymagający, aby wszystkie metody/właściwości natychmiast rzuciły ObjectDisposedException, jeśli Dispose został już wywołany, możesz użyć następującego testu jednostki. Po prostu określ zespoły, które chcesz przetestować. Prawdopodobnie będziesz musiał zmodyfikować metodę IsExcluded w razie potrzeby, a kpina z obiektu może nie działać we wszystkich przypadkach.

using System; 
using System.Collections.Generic; 
using System.Linq; 
using System.Reflection; 
using MbUnit.Framework; 
using Moq; 

[TestFixture] 
public class IDisposableTests 
{ 
    [Test] 
    public void ThrowsObjectDisposedExceptions() 
    { 
     var assemblyToTest = Assembly.LoadWithPartialName("MyAssembly"); 

     // Get all types that implement IDisposable 
     var disposableTypes = 
      from type in assemblyToTest.GetTypes() 
      where type.GetInterface(typeof(IDisposable).FullName) != null 
      select type; 

     foreach (var type in disposableTypes) 
     { 
      // Try to get default constructor first... 
      var constructor = type.GetConstructor(Type.EmptyTypes); 

      if (constructor == null) 
      { 
       // Otherwise get first parameter based constructor... 
       var constructors = type.GetConstructors(); 

       if (constructors != null && 
        constructors.Length > 0) 
       { 
        constructor = constructors[0]; 
       } 
      } 

      // If there is a public constructor... 
      if (constructor != null) 
      { 
       object instance = Activator.CreateInstance(type, GetDefaultArguments(constructor)); 

       (instance as IDisposable).Dispose(); 

       foreach (var method in type.GetMethods()) 
       { 
        if (!this.IsExcluded(method)) 
        { 
         bool threwObjectDisposedException = false; 

         try 
         { 
          method.Invoke(instance, GetDefaultArguments(method)); 
         } 
         catch (TargetInvocationException ex) 
         { 
          if (ex.InnerException.GetType() == typeof(ObjectDisposedException)) 
          { 
           threwObjectDisposedException = true; 
          } 
         } 

         Assert.IsTrue(threwObjectDisposedException); 
        } 
       } 
      } 
     } 
    } 

    private bool IsExcluded(MethodInfo method) 
    { 
     // May want to include ToString, GetHashCode. 
     // Doesn't handle checking overloads which would take more 
     // logic to compare parameters etc. 
     if (method.Name == "Dispose" || 
      method.Name == "GetType") 
     { 
      return true; 
     } 

     return false; 
    } 

    private object[] GetDefaultArguments(MethodBase method) 
    { 
     var arguments = new List<object>(); 

     foreach (var parameter in method.GetParameters()) 
     { 
      var type = parameter.ParameterType; 

      if (type.IsValueType) 
      { 
       arguments.Add(Activator.CreateInstance(type)); 
      } 
      else if (!type.IsSealed) 
      { 
       dynamic mock = Activator.CreateInstance(typeof(Mock<>).MakeGenericType(type)); 
       arguments.Add(mock.Object); 
      } 
      else 
      { 
       arguments.Add(null); 
      } 
     } 

     return arguments.ToArray(); 
    } 
} 

Original Response: To nie wygląda jak coś jest jak NotifyPropertyWeaver dla IDisposable więc jeśli chcesz, że trzeba stworzyć podobny projekt samodzielnie. Możesz potencjalnie zaoszczędzić sobie trochę pracy, mając podstawową klasę Jednorazowe, takie jak w tym blog entry. Wtedy masz tylko jedną linijkę u góry każdej metody: ThrowExceptionIfDisposed().

Jednak żadne z możliwych rozwiązań nie wydaje się właściwe ani konieczne. Ogólnie rzucanie ObjectDisposedException nie jest tak często potrzebne. Zrobiłem szybkie wyszukiwanie w Reflectorze i ObjectDisposedException jest wyrzucane bezpośrednio przez zaledwie 6 typów w BCL, a dla próbki poza BCL System.Windows.Forms ma tylko jeden typ, który rzuca: Cursor podczas uzyskiwania Uchwyt.

Zasadniczo wystarczy rzucić ObjectDisposedException, jeśli wywołanie obiektu nie powiedzie się, ponieważ zostało już wywołane Dispose, na przykład po ustawieniu pola na wartość null wymaganą przez metodę lub właściwość. Numer ObjectDisposedException będzie bardziej pouczający niż losowy NullReferenceException, ale dopóki nie wyczyścisz niezarządzanych zasobów, zwykle nie jest konieczne ustawienie grupy pól na wartość null. W większości przypadków, jeśli po prostu dzwonisz Wyrzuć na inne przedmioty, to do nich należy rzucanie ObjectDisposedException.

Tutaj jest trywialny przykład można rzucać ObjectDisposedException wyraźnie:

public class ThrowObjectDisposedExplicity : IDisposable 
{ 
    private MemoryStream stream; 

    public ThrowObjectDisposedExplicity() 
    { 
     this.stream = new MemoryStream(); 
    } 

    public void DoSomething() 
    { 
     if (this.stream == null) 
     { 
      throw new ObjectDisposedException(null); 
     } 

     this.stream.ReadByte(); 
    } 

    protected virtual void Dispose(bool disposing) 
    { 
     if (disposing) 
     { 
      if (this.stream != null) 
      { 
       this.stream.Dispose(); 
       this.stream = null; 
      } 
     } 
    } 

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

W powyższym kodzie, choć naprawdę nie ma potrzeby ustawić strumień null. można po prostu polegać na tym, że MemoryStream.ReadByte() rzuci ObjectDisposedException na swój własny jak poniższy kod:

public class ThrowObjectDisposedImplicitly : IDisposable 
{ 
    private MemoryStream stream; 

    public ThrowObjectDisposedImplicitly() 
    { 
     this.stream = new MemoryStream(); 
    } 

    public void DoSomething() 
    { 
     // This will throw ObjectDisposedException as necessary 
     this.stream.ReadByte(); 
    } 

    protected virtual void Dispose(bool disposing) 
    { 
     if (disposing) 
     { 
      this.stream.Dispose(); 
     } 
    } 

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

Pierwsza strategia ustalania strumień null może mieć sens w niektórych przypadkach, na przykład, jeśli wiesz, obiekt będzie wyrzuć wyjątek, jeśli Dispose zostanie wywołane wiele razy. W takim przypadku chcesz być defensywny i upewnić się, że twoja klasa nie rzuca wyjątku na wiele połączeń z numerem Dispose. Poza tym nie mogę wymyślić żadnych innych przypadków, w których trzeba by ustawić pola na zero, co prawdopodobnie wymagałoby rzucenia ObjectDisposedExceptions.

Rzucanie ObjectDisposedException nie jest często potrzebne i należy je uważnie przeanalizować, więc narzędzie do tkania kodu prawdopodobnie nie jest konieczne. Korzystaj z bibliotek Microsoft jako przykładu, zobacz, które metody implementują w rzeczywistości ObjectDisposedException, gdy typ implementuje IDisposable.

Uwaga:GC.SuppressFinalize(this); nie jest bezwzględnie konieczne, ponieważ nie ma finalizator, ale pozostało tam od podklasą mogą wdrożyć finalizatora. Jeśli klasa została oznaczona jako sealed, można bezpiecznie usunąć GC.SupressFinalize.

+1

Jestem otwarty na dodatkowe wyjaśnienia, ale na pierwszy rzut oka nie zgadzam się z tą odpowiedzią. Wywołanie metody Dispose wskazuje, że kod wywołujący nie zamierza ponownie używać tego obiektu. Jeśli metoda zostanie wywołana po tym, jest to błąd, a zasada "fail-fast" nakazuje, aby wyjątek był zgłaszany natychmiast, niezależnie od tego, czy ta metoda rzeczywiście próbuje użyć zasobu, który został usunięty. – StriplingWarrior

+0

Również ustawienie 'this.stream' na wartość null w twoim pierwszym przykładzie jest dobrą praktyką, ponieważ celem wywoływania' Dispose' jest zapewnienie, że wszystkie zasoby obiektu są zwolnione. 'GC.SuppressFinalize' zasadniczo mówi GC, że już posprzątałeś po sobie, więc jeśli wywołasz tę metodę bez wyczyszczenia wszystkich referencji, zrywasz umowę. Co więcej, nie masz pojęcia, w jakich kontekstach OP wdraża IDisposable, więc nie jesteś w stanie ocenić, czy generowanie kodu jest odpowiednie. – StriplingWarrior

+0

Aby utworzyć kopię zapasową mojego pierwszego roszczenia, http://msdn.microsoft.com/en-us/library/system.objectdisposedexception.aspx definiuje ObjectDisposedException jako "wyjątek, który jest zgłaszany, gdy operacja jest wykonywana na unieszkodliwionym obiekcie."Myślę, że dość wyraźnie wskazuje, że wyjątek ma być wyrzucany za każdym razem, gdy operacja jest wykonywana na usuniętym obiekcie – StriplingWarrior

2

Zamiast korzystać z NotifyPropertyWeaver, sugeruję, aby rozwiązać swój problem w inny sposób. Ci twierdzą, że mają dwie kwestie:

  1. Ludzie zapomną wdrożyć wzór o
  2. szukasz uniknąć kosztów wdrożenia to wiele razy w dużej bazy kodu

W celu pokrycia koszt ponownego napisania bardzo podobnego kodu, sugeruję tworzenie i używanie fragmentów kodu w Visual Studio. Problem, który fragmenty próbują rozwiązać, jest dokładnie # 2 na powyższej liście. Instrukcje znajdziesz w tej instrukcji: MSDN article.

Problem z # 1 na powyższej liście jest taki, że wzorzec IDisposable nie zawsze jest konieczny na 100% twoich zajęć. To sprawia, że ​​statyczna analiza utrudnia "złapanie" tego, czy dana klasa powinna być jednorazowa. (edytuj: Po zastanowieniu się nad tym przez chwilę, to naprawdę nie byłoby trudne do sprawdzenia Jeśli twoja obecna klasa zawiera instancje, które są IDisposable, twoja klasa powinna być IDisposable)

Z tego powodu, polecam Ci użyj przeglądu kodu, aby złapać przypadki, w których programista powinien uczynić swoją klasę jednorazową. Możesz dodać go do listy kontrolnej przedmiotów, które programista powinien sam sprawdzić przed poproszeniem o sprawdzenie kodu.

+0

Twoje sugestie są dobre zarówno dla świeżo napisanego kodu. Ale mamy dużo starego kodu, który łączy się z niezarządzanymi bibliotekami. Nienawidzę tego, że ktoś będzie musiał siedzieć i pisać to samo w kółko, nawet przy kopiowaniu i wklejaniu, to nie jest to, co powinni robić deweloperzy. –

Powiązane problemy