IDisposable korrekt implementieren

145

In meinen Klassen implementiere ich IDisposable wie folgt:

public class User : IDisposable
{
    public int id { get; protected set; }
    public string name { get; protected set; }
    public string pass { get; protected set; }

    public User(int UserID)
    {
        id = UserID;
    }
    public User(string Username, string Password)
    {
        name = Username;
        pass = Password;
    }

    // Other functions go here...

    public void Dispose()
    {
        // Clear all property values that maybe have been set
        // when the class was instantiated
        id = 0;
        name = String.Empty;
        pass = String.Empty;
    }
}

In VS2012 heißt es in meiner Codeanalyse, IDisposable korrekt zu implementieren, aber ich bin mir nicht sicher, was ich hier falsch gemacht habe.
Der genaue Text lautet wie folgt:

CA1063 IDisposable korrekt implementieren Stellen Sie eine überschreibbare Implementierung von Dispose (bool) für 'User' bereit oder markieren Sie den Typ als versiegelt. Ein Aufruf von Dispose (false) sollte nur native Ressourcen bereinigen. Ein Aufruf von Dispose (true) sollte sowohl verwaltete als auch native Ressourcen bereinigen. stman User.cs 10

Als Referenz: CA1063: IDisposable korrekt implementieren

Ich habe diese Seite gelesen, aber ich fürchte, ich verstehe nicht wirklich, was hier zu tun ist.

Wenn jemand mehr Lamens erklären kann, was das Problem ist und / oder wie IDisposable implementiert werden sollte, wird das wirklich helfen!

Ortund
quelle
1
Ist das der ganze Code drin Dispose?
Claudio Redi
42
Sie sollten Ihre Dispose () -Methode implementieren, um die Dispose () -Methode für eines der Mitglieder Ihrer Klasse aufzurufen. Keines dieser Mitglieder hat eines. Sie sollten IDisposable daher nicht implementieren. Das Zurücksetzen der Eigenschaftswerte ist sinnlos.
Hans Passant
13
Sie müssen nur implementieren , IDispoablewenn Sie nicht verwalteten Ressourcen zu entsorgen (dies schließt nicht verwalteten Ressourcen , die gewickelt werden ( SqlConnection, FileStreamusw.). Sie können und sollten nicht implementieren , IDisposablewenn Sie nur Ressourcen wie hier geschafft haben. Das ist, IMO, Ein großes Problem bei der Code-Analyse. Es ist sehr gut darin, dumme kleine Regeln zu überprüfen, aber nicht gut darin, konzeptionelle Fehler zu überprüfen.
Jason
51
Es ist ziemlich ärgerlich für mich, dass einige Leute diese Frage lieber ablehnen und als geschlossen ansehen würden, als zu versuchen, einer Person zu helfen, die ein Konzept eindeutig missverstanden hat. Schade.
Ortund
2
Stimmen Sie also nicht ab, stimmen Sie nicht ab, lassen Sie den Beitrag bei Null und schließen Sie die Frage mit einem hilfreichen Zeiger.
tjmoore

Antworten:

113

Dies wäre die richtige Implementierung, obwohl ich in dem von Ihnen geposteten Code nichts sehe, was Sie entsorgen müssen. Sie müssen nur implementieren, IDisposablewenn:

  1. Sie haben nicht verwaltete Ressourcen
  2. Sie halten an Referenzen von Dingen fest, die selbst verfügbar sind.

Nichts in dem von Ihnen geposteten Code muss entsorgt werden.

public class User : IDisposable
{
    public int id { get; protected set; }
    public string name { get; protected set; }
    public string pass { get; protected set; }

    public User(int userID)
    {
        id = userID;
    }
    public User(string Username, string Password)
    {
        name = Username;
        pass = Password;
    }

    // Other functions go here...

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

    protected virtual void Dispose(bool disposing)
    {
        if (disposing) 
        {
            // free managed resources
        }
        // free native resources if there are any.
    }
}
Daniel Mann
quelle
2
Als ich anfing, in C # zu schreiben, wurde mir gesagt, dass es am besten ist, using(){ }wenn immer möglich zu verwenden, aber um dies zu tun, müssen Sie IDisposable implementieren. Im Allgemeinen bevorzuge ich den Zugriff auf eine Klasse über usings, insb. wenn ich die Klasse nur in ein oder zwei Funktionen
brauche
62
@Ortund Du hast es falsch verstanden. Verwenden Sie am besten einen usingBlock, wenn die Klasse IDisposable implementiert . Wenn Sie keine Klasse benötigen, um verfügbar zu sein, implementieren Sie sie nicht. Es hat keinen Zweck.
Daniel Mann
5
@DanielMann Die Semantik eines usingBlocks ist jedoch in der Regel über die IDisposableSchnittstelle hinaus ansprechend . Ich stelle mir vor, dass es mehr als ein paar Missbräuche IDisposablenur zu Scoping-Zwecken gegeben hat.
Thomas
1
Nur als Randnotiz, wenn Sie nicht verwaltete Ressourcen freigeben möchten, sollten Sie Finalizer einschließen, der Dispose (false) aufruft. Dadurch kann GC Finalizer aufrufen, wenn die Speicherbereinigung durchgeführt wird (falls Dispose noch nicht aufgerufen wurde), und nicht verwaltete Ressourcen ordnungsgemäß freigeben Ressourcen.
Mariozski
4
Ohne einen Finalizer in Ihrer Implementierung ist das Aufrufen GC.SuppressFinalize(this);sinnlos. Wie @mariozski betonte, würde ein Finalizer dazu beitragen, dass dieser überhaupt Disposeaufgerufen wird, wenn die Klasse nicht in einem usingBlock verwendet wird.
Haymo Kutschbach
57

Zunächst müssen Sie strings und ints nicht "bereinigen" - sie werden automatisch vom Garbage Collector erledigt. Das einzige, was bereinigt werden muss, Disposesind nicht verwaltete Ressourcen oder verwaltete Ressourcen, die implementiert werden IDisposable.

Unter der Annahme, dass dies nur eine Lernübung ist, wird empfohlen , IDisposableeinen "Sicherheitsverschluss" hinzuzufügen, um sicherzustellen, dass Ressourcen nicht zweimal entsorgt werden:

public void Dispose()
{
    Dispose(true);

    // Use SupressFinalize in case a subclass 
    // of this type implements a finalizer.
    GC.SuppressFinalize(this);   
}
protected virtual void Dispose(bool disposing)
{
    if (!_disposed)
    {
        if (disposing) 
        {
            // Clear all property values that maybe have been set
            // when the class was instantiated
            id = 0;
            name = String.Empty;
            pass = String.Empty;
        }

        // Indicate that the instance has been disposed.
        _disposed = true;   
    }
}
D Stanley
quelle
3
+1, ein Flag zu haben, um sicherzustellen, dass der Bereinigungscode nur einmal ausgeführt wird, ist viel, viel besser als das Setzen von Eigenschaften auf null oder was auch immer (zumal dies die readonlySemantik stört )
Thomas
+1 für die Verwendung des Benutzercodes (obwohl dieser automatisch bereinigt wird), um zu verdeutlichen, was dort vor sich geht. Auch, weil er kein salziger Seemann ist und ihn dafür hämmert, dass er einen kleinen Fehler gemacht hat, während er wie viele andere hier gelernt hat.
Murphybro2
42

Das folgende Beispiel zeigt die allgemeine Best Practice zum Implementieren der IDisposableSchnittstelle. Referenz

Denken Sie daran, dass Sie einen Destruktor (Finalizer) nur benötigen, wenn Sie nicht verwaltete Ressourcen in Ihrer Klasse haben. Wenn Sie einen Destruktor hinzufügen, sollten Sie die Finalisierung in der Entsorgung unterdrücken , da sich Ihre Objekte sonst zwei Speicherzyklen im Speicher befinden (Hinweis: Lesen Sie, wie die Finalisierung funktioniert ). Das folgende Beispiel erläutert alles oben.

public class DisposeExample
{
    // A base class that implements IDisposable. 
    // By implementing IDisposable, you are announcing that 
    // instances of this type allocate scarce resources. 
    public class MyResource: IDisposable
    {
        // Pointer to an external unmanaged resource. 
        private IntPtr handle;
        // Other managed resource this class uses. 
        private Component component = new Component();
        // Track whether Dispose has been called. 
        private bool disposed = false;

        // The class constructor. 
        public MyResource(IntPtr handle)
        {
            this.handle = handle;
        }

        // Implement IDisposable. 
        // Do not make this method virtual. 
        // A derived class should not be able to override this method. 
        public void Dispose()
        {
            Dispose(true);
            // This object will be cleaned up by the Dispose method. 
            // Therefore, you should call GC.SupressFinalize to 
            // take this object off the finalization queue 
            // and prevent finalization code for this object 
            // from executing a second time.
            GC.SuppressFinalize(this);
        }

        // Dispose(bool disposing) executes in two distinct scenarios. 
        // If disposing equals true, the method has been called directly 
        // or indirectly by a user's code. Managed and unmanaged resources 
        // can be disposed. 
        // If disposing equals false, the method has been called by the 
        // runtime from inside the finalizer and you should not reference 
        // other objects. Only unmanaged resources can be disposed. 
        protected virtual void Dispose(bool disposing)
        {
            // Check to see if Dispose has already been called. 
            if(!this.disposed)
            {
                // If disposing equals true, dispose all managed 
                // and unmanaged resources. 
                if(disposing)
                {
                    // Dispose managed resources.
                    component.Dispose();
                }

                // Call the appropriate methods to clean up 
                // unmanaged resources here. 
                // If disposing is false, 
                // only the following code is executed.
                CloseHandle(handle);
                handle = IntPtr.Zero;

                // Note disposing has been done.
                disposed = true;

            }
        }

        // Use interop to call the method necessary 
        // to clean up the unmanaged resource.
        [System.Runtime.InteropServices.DllImport("Kernel32")]
        private extern static Boolean CloseHandle(IntPtr handle);

        // Use C# destructor syntax for finalization code. 
        // This destructor will run only if the Dispose method 
        // does not get called. 
        // It gives your base class the opportunity to finalize. 
        // Do not provide destructors in types derived from this class.
        ~MyResource()
        {
            // Do not re-create Dispose clean-up code here. 
            // Calling Dispose(false) is optimal in terms of 
            // readability and maintainability.
            Dispose(false);
        }
    }
    public static void Main()
    {
        // Insert code here to create 
        // and use the MyResource object.
    }
}
CharithJ
quelle
14

IDisposableEs gibt eine Möglichkeit, nicht verwaltete Ressourcen zu bereinigen, die vom Garbage Collector nicht automatisch bereinigt werden.

Alle Ressourcen, die Sie "bereinigen", sind verwaltete Ressourcen, und als solche erreicht Ihre DisposeMethode nichts. Ihre Klasse sollte überhaupt nicht implementieren IDisposable. Der Garbage Collector kümmert sich selbstständig um all diese Felder.

Servieren
quelle
1
Stimmen Sie dem zu - es gibt den Gedanken, alles zu entsorgen, wenn es tatsächlich nicht benötigt wird. Entsorgen sollte nur verwendet werden, wenn Sie nicht verwaltete Ressourcen zum Aufräumen haben !!
Chandramouleswaran Ravichandra
4
Nicht unbedingt wahr, die Dispose-Methode ermöglicht es Ihnen auch, "verwaltete Ressourcen, die IDisposable implementieren" zu entsorgen
Matt Wilko
@MattWilko Dies macht es zu einer indirekten Möglichkeit, nicht verwaltete Ressourcen zu entsorgen, da eine andere Ressource eine nicht verwaltete Ressource entsorgen kann. Hier gibt es nicht einmal einen indirekten Verweis auf eine nicht verwaltete Ressource über eine andere verwaltete Ressource.
Servy
@MattWilko Dispose wird automatisch jede verwaltete Ressource aufgerufen, die IDesposable
Panky Sharma
@pankysharma Nein, das wird es nicht. Es muss manuell aufgerufen werden. Das ist der springende Punkt. Sie können nicht sie davon ausgehen , automatisch aufgerufen wird, wissen Sie nur Menschen soll es manuell aufrufen, aber die Leute machen Fehler machen und vergessen.
Servy
14

Sie müssen das Einwegmuster wie folgt verwenden:

private bool _disposed = false;

protected virtual void Dispose(bool disposing)
{
    if (!_disposed)
    {
        if (disposing)
        {
            // Dispose any managed objects
            // ...
        }

        // Now disposed of any unmanaged objects
        // ...

        _disposed = true;
    }
}

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

// Destructor
~YourClassName()
{
    Dispose(false);
}
Belogix
quelle
1
Wäre es nicht klüger, GC.SuppressFinalize (this) auch im Destruktor aufzurufen? Andernfalls würde das Objekt selbst im nächsten GC
Sudhanshu Mishra
2
@dotnetguy: Der Objektdestruktor wird aufgerufen, wenn der gc ausgeführt wird. Ein zweimaliger Anruf ist also nicht möglich. Siehe hier: msdn.microsoft.com/en-us/library/ms244737.aspx
schoetbi
1
Nennen wir jetzt irgendeinen Teil des Boilerplate-Codes ein "Muster"?
Chel
4
@rdhs Nein, sind wir nicht. MSDN heißt es IST ein Muster „Entsorgen Muster“ hier - msdn.microsoft.com/en-us/library/b1yfkh5e(v=vs.110).aspx so vor nach unten stimmrechts vielleicht Google ein wenig?
Belogix
2
Weder Microsoft noch Ihr Beitrag geben eindeutig an, warum das Muster so aussehen sollte. Im Allgemeinen ist es nicht einmal Boilerplate, es ist nur überflüssig - ersetzt durch SafeHandle(und Untertypen). Bei verwalteten Ressourcen wird die ordnungsgemäße Entsorgung viel einfacher. Sie können den Code auf eine einfache Implementierung der void Dispose()Methode reduzieren.
BatteryBackupUnit
10

Sie müssen Ihre UserKlasse nicht ausführen , IDisposableda die Klasse keine nicht verwalteten Ressourcen (Datei, Datenbankverbindung usw.) erhält . Normalerweise markieren wir Klassen so, als IDisposablehätten sie mindestens ein IDisposableFeld oder / und eine Eigenschaft. Sagen Sie es bei der Implementierung IDisposablebesser nach dem typischen Microsoft-Schema:

public class User: IDisposable {
  ...
  protected virtual void Dispose(Boolean disposing) {
    if (disposing) {
      // There's no need to set zero empty values to fields 
      // id = 0;
      // name = String.Empty;
      // pass = String.Empty;

      //TODO: free your true resources here (usually IDisposable fields)
    }
  }

  public void Dispose() {
    Dispose(true);

    GC.SuppressFinalize(this);
  } 
}
Dmitry Bychenko
quelle
Das ist normalerweise der Fall. Andererseits eröffnet das using-Konstrukt die Möglichkeit, etwas zu schreiben, das C ++ - Smart-Zeigern ähnelt, nämlich ein Objekt, das den vorherigen Status wiederherstellt, unabhängig davon, wie ein using-Block verlassen wird. Die einzige Möglichkeit, dies zu tun, besteht darin, ein solches Objekt IDisposable implementieren zu lassen. Es scheint, dass ich die Warnung des Compilers in einem so marginalen Anwendungsfall ignorieren kann.
Papa Schlumpf
3

Idisposable wird immer dann implementiert, wenn Sie eine deterministische (bestätigte) Speicherbereinigung wünschen.

class Users : IDisposable
    {
        ~Users()
        {
            Dispose(false);
        }

        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
            // This method will remove current object from garbage collector's queue 
            // and stop calling finilize method twice 
        }    

        public void Dispose(bool disposer)
        {
            if (disposer)
            {
                // dispose the managed objects
            }
            // dispose the unmanaged objects
        }
    }

Verwenden Sie beim Erstellen und Verwenden der Users-Klasse den Block "using", um zu vermeiden, dass die dispose-Methode explizit aufgerufen wird:

using (Users _user = new Users())
            {
                // do user related work
            }

Das Ende des mit dem Benutzerobjekt erstellten Verwendungsblocks wird durch implizites Aufrufen der Entsorgungsmethode entsorgt.

S.Roshanth
quelle
2

Ich sehe viele Beispiele für das Microsoft Dispose-Muster, das wirklich ein Anti-Muster ist. Wie viele darauf hingewiesen haben, erfordert der Code in der Frage überhaupt keine IDisposable. Wenn Sie es jedoch implementieren möchten, verwenden Sie bitte nicht das Microsoft-Muster. Eine bessere Antwort wäre, den Vorschlägen in diesem Artikel zu folgen:

https://www.codeproject.com/Articles/29534/IDisposable-What-Your-Mother-Never-Told-You-About

Das einzige andere, was wahrscheinlich hilfreich wäre, ist die Unterdrückung dieser Warnung zur Codeanalyse ... https://docs.microsoft.com/en-us/visualstudio/code-quality/in-source-suppression-overview?view=vs- 2017

MikeJ
quelle