Implementando IDisposable correctamente

145

En mis clases implemento IDisposable de la siguiente manera:

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;
    }
}

En VS2012, mi análisis de código dice implementar IDisposable correctamente, pero no estoy seguro de qué he hecho mal aquí.
El texto exacto es el siguiente:

CA1063 ID de implemento desechable correctamente Proporcione una implementación reemplazable de Dispose (bool) en 'Usuario' o marque el tipo como sellado. Una llamada a deshacerse (falso) solo debe limpiar los recursos nativos. Una llamada a la eliminación (verdadero) debe limpiar los recursos administrados y nativos. stman User.cs 10

Para referencia: CA1063: ID de implemento desechable correctamente

He leído esta página, pero me temo que realmente no entiendo lo que hay que hacer aquí.

Si alguien puede explicar en términos más generales cuál es el problema y / o cómo se debe implementar IDisposable, ¡eso realmente ayudará!

Ortund
fuente
1
¿Eso es todo el código dentro Dispose?
Claudio Redi
42
Debe implementar su método Dispose () para llamar al método Dispose () en cualquiera de los miembros de su clase. Ninguno de esos miembros tiene uno. Por lo tanto, no debe implementar IDisposable. Restablecer los valores de propiedad no tiene sentido.
Hans Passant
13
Sólo es necesario poner en práctica IDispoablesi tiene recursos no administrados de desechar (esto incluye los recursos no administrados que se envuelven ( SqlConnection, FileStream, etc.). No es y no debe poner en práctica IDisposablesi sólo han conseguido recursos como aquí. Esto es, la OMI, . un gran problema con el análisis de código es muy bueno en la comprobación tonta pequeñas reglas, pero no bueno en la comprobación de errores conceptuales.
Jason
51
Es bastante molesto para mí que algunas personas prefieran rechazar y ver esta pregunta cerrada que intentar ayudar a una persona que claramente ha entendido mal un concepto. Qué lástima.
Ortund
2
Así que no votes a favor, no votes a favor, deja la publicación en cero y cierra la pregunta con un puntero útil.
tjmoore 01 de

Respuestas:

113

Esta sería la implementación correcta, aunque no veo nada que deba disponer en el código que publicó. Solo necesita implementar IDisposablecuando:

  1. Tienes recursos no gestionados
  2. Estás aferrado a referencias de cosas que son desechables.

Nada en el código que publicó debe ser eliminado.

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
fuente
2
Cuando comencé a escribir en C # me dijeron que es mejor utilizarlo using(){ }siempre que sea posible, pero para hacerlo, debe implementar IDisposable, por lo que, en general, prefiero acceder a una clase a través de usos, especialmente. si solo necesito la clase en una o dos funciones
Ortund
62
@Ortund Usted entendió mal. Es mejor usar un usingbloque cuando la clase implementa IDisposable . Si no necesita que una clase sea desechable, no la implemente. No sirve para nada.
Daniel Mann
55
@DanielMann Sin embargo, la semántica de un usingbloque tiende a ser atractiva más allá de la IDisposableinterfaz. Me imagino que ha habido más de unos pocos abusos IDisposablesolo con fines de alcance.
Thomas
1
Solo como nota al margen si desea liberar recursos no administrados, debe incluir Finalizer llamando a Dispose (falso), que permitirá que GC llame a Finalizer cuando realice la recolección de basura (en caso de que todavía no se haya llamado a Dispose) y libere adecuadamente sin administrar recursos
mariozski
44
Sin un finalizador en su implementación, llamar no GC.SuppressFinalize(this);tiene sentido. Como señaló @mariozski, un finalizador ayudaría a garantizar que Disposese llame en absoluto si la clase no se usa dentro de un usingbloque.
Haymo Kutschbach
57

En primer lugar, no es necesario para "limpiar" strings y ints - que será atendido de forma automática por el recolector de basura. Lo único que debe limpiarse Disposeson los recursos no administrados o los recursos administrados que se implementan IDisposable.

Sin embargo, suponiendo que esto sea solo un ejercicio de aprendizaje, la forma recomendada de implementar IDisposablees agregar una "captura de seguridad" para garantizar que los recursos no se eliminen dos veces:

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
fuente
3
+1, tener una bandera para asegurarse de que el código de limpieza se ejecute solo una vez es mucho mejor que establecer las propiedades en nulo o lo que sea (especialmente porque eso interfiere con la readonlysemántica)
Thomas
+1 por usar el código de usuario (aunque se limpiará automáticamente) para dejar en claro lo que sucede allí. Además, por no ser un marinero salado y golpearlo por cometer un pequeño error mientras aprende como muchos otros aquí.
Murphybro2
42

El siguiente ejemplo muestra las mejores prácticas generales para implementar la IDisposableinterfaz. Referencia

Tenga en cuenta que necesita un destructor (finalizador) solo si tiene recursos no administrados en su clase. Y si agrega un destructor, debe suprimir Finalización en Dispose , de lo contrario hará que sus objetos residan en la memoria durante dos ciclos de basura (Nota: lea cómo funciona la Finalización ). A continuación se detallan todos los ejemplos anteriores.

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
fuente
14

IDisposableexiste para proporcionar un medio para que usted limpie los recursos no administrados que el recolector de basura no limpiará automáticamente.

Todos los recursos que está "limpiando" son recursos administrados y, como tal, su Disposemétodo no está logrando nada. Tu clase no debería implementarse IDisposableen absoluto. El recolector de basura se encargará de todos esos campos por sí solo.

Servy
fuente
1
De acuerdo con esto: existe la noción de deshacerse de todo cuando en realidad no es necesario. ¡Deseche debe usarse solo si tiene recursos no administrados para limpiar!
Chandramouleswaran Ravichandra
44
No es estrictamente cierto, el método Dispose también le permite disponer de "recursos gestionados que implementan IDisposable"
Matt Wilko
@MattWilko Eso lo convierte en una forma indirecta de deshacerse de los recursos no administrados, ya que permite que otro recurso disponga de un recurso no administrado. Aquí ni siquiera hay una referencia indirecta a un recurso no administrado a través de otro recurso administrado.
Servicio
@MattWilko Dispose llamará automáticamente a cualquier recurso administrado que implementó IDesposable
panky sharma
@pankysharma No, no lo hará. Necesita ser llamado manualmente . De eso se trata. No puede suponer que se llamará automáticamente, solo sabe que las personas deben llamarlo manualmente, pero las personas cometen errores y olvidan.
Servicio el
14

Debe usar el patrón desechable de esta manera:

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
fuente
1
¿no sería más inteligente tener una llamada a GC.SuppressFinalize (this) también en el destructor? De lo contrario, el objeto en sí sería reclamado en el próximo GC
Sudhanshu Mishra el
2
@dotnetguy: se llama al destructor de objetos cuando se ejecuta gc. Entonces llamar dos veces no es posible. Ver aquí: msdn.microsoft.com/en-us/library/ms244737.aspx
schoetbi
1
Entonces, ¿estamos llamando a cualquier parte del código repetitivo un "patrón"?
Chel
44
@rdhs No, no lo somos. MSDN afirma que ES un patrón "Dispose Pattern" aquí: msdn.microsoft.com/en-us/library/b1yfkh5e(v=vs.110).aspx, así que, antes de votar, ¿quizás Google un poco?
Belogix
2
Ni Microsoft ni su publicación indican claramente por qué el patrón debería verse así. En general, ni siquiera es repetitivo, es superfluo, reemplazado por SafeHandle(y subtipos). En el caso de los recursos gestionados, la implementación de la eliminación adecuada se vuelve mucho más simple; puede recortar el código a una implementación simple del void Dispose()método.
BatteryBackupUnit
10

No necesita hacer su Userclase IDisposableya que la clase no adquiere ningún recurso no administrado (archivo, conexión de base de datos, etc.). Por lo general, marcamos las clases como IDisposablesi tuvieran al menos un IDisposablecampo o propiedad. Al implementar IDisposable, mejor ponlo según el esquema típico de Microsoft:

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
fuente
Ese suele ser el caso. Pero, por otro lado, la construcción de uso abre la posibilidad de escribir algo similar a los punteros inteligentes de C ++, es decir, un objeto que restaurará el estado anterior sin importar cómo se sale de un bloque de uso. La única forma que he encontrado de hacer esto es hacer que un objeto de este tipo implemente IDisposable. Parece que puedo ignorar la advertencia del compilador en un caso de uso tan marginal.
Papa Pitufo
3

Idisposable se implementa cuando se desea una recolección de basura determinista (confirmada).

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
        }
    }

Al crear y usar la clase Users, use el bloque "using" para evitar llamar explícitamente al método dispose:

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

El final del bloque de uso creado El objeto Usuarios se eliminará mediante la invocación implícita del método de eliminación.

S.Roshanth
fuente
2

Veo muchos ejemplos del patrón Microsoft Dispose, que es realmente un antipatrón. Como muchos han señalado, el código en la pregunta no requiere IDisposable en absoluto. Pero si va a implementarlo, no use el patrón de Microsoft. Una mejor respuesta sería seguir las sugerencias de este artículo:

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

La única otra cosa que probablemente sería útil es suprimir esa advertencia de análisis de código ... https://docs.microsoft.com/en-us/visualstudio/code-quality/in-source-suppression-overview?view=vs- 2017

MikeJ
fuente