Suis-je mise en œuvre de IDisposable correctement?

Question

Cette classe utilise un StreamWriter et, par conséquent, met en œuvre IDisposable.

public class Foo : IDisposable
{
    private StreamWriter _Writer;

    public Foo (String path)
    {
        // here happens something along the lines of:
        FileStream fileWrite = File.Open (path, FileMode.OpenOrCreate, FileAccess.Write, FileShare.ReadWrite);
        _Writer = new StreamWriter (fileWrite, new ASCIIEncoding ());
    }

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

    ~Foo()
    {
        Dispose (false);
    }

    protected virtual void Dispose (bool disposing)
    {
        if (_Disposed) {
            return;
        }
        if (disposing) {
            _Writer.Dispose ();
        }
        _Writer = null;
        _Disposed = true;
    }
    private bool _Disposed;
}

}

Est-il un problème avec l'implémentation actuelle? I. e., dois-je libérer le sous-jacent FileStream manuellement? Est - Dispose(bool) écrit correctement?

Answer 1

Vous n'avez pas besoin d'utiliser cette grande version de IDisposable mise en œuvre si votre classe n'est pas directement l'utilisation des ressources non managées.

Un simple

 public virtual void Dispose()
 {

     _Writer.Dispose();
 }

suffira.

Si votre consommateur ne parvient pas à Éliminer de votre objet, il sera GC feriez normalement, sans un appel à Disposer de l'objet tenu par _Writer sera également GC avais et il aura une finaliser donc, il obtient toujours de nettoyer ses ressources non gérées correctement.

Modifier

Ayant fait quelques recherches sur les liens fournis par Matt et les autres, je suis venu à la conclusion que ma réponse ici, se dresse. Voici pourquoi:-

L'hypothèse de base du jetable, de la mise en œuvre du "motif" (j'entends par là que le virtuel protégé dispose(bool), SuppressFinalize etc. marlarky) sur un héritées de la classe est une sous-classe peut tenir sur une ressource non managée.

Cependant, dans le monde réel, la grande majorité d'entre nous .NET développeurs ne jamais aller n'importe où à proximité d'une ressource non managée. Si vous aviez à quantifier le "peut" au-dessus de ce probabilty figure voulez-vous venir avec vous en quelque sorte .NET de codage?

Supposons que j'ai un type de Personne (qui pour l'amour de l'argument a un type jetable dans un de ses champs, et donc devrait être jetables lui-même). Maintenant, j'ai héritiers Client, un Employé, etc. Est-il vraiment raisonnable pour moi de l'encombrement de la classe Personne avec ce "Modèle" juste au cas où quelqu'un hérite de Personne et veut garder une ressource non managée?

Parfois, nous les développeurs peuvent plus de compliquer les choses dans une tentative de code pour toutes les circonstances possibles sans l'aide d'un peu de bon sens concernant la probabilité relative de telles circonstances.

Si jamais nous voulions utiliser une ressource non managée directement le sensible, modèle serait envelopper une telle chose dans sa propre classe où l'intégralité du "jetable modèle" serait raisonnable. D'où le très grand nombre de "normal" code nous de ne pas avoir à vous soucier de tout ce qui marinage sur. Si nous avons besoin de IDisposable nous pouvons utiliser le modèle simple ci-dessus, pouvant être héritées ou pas.

Ouf, content d'obtenir ce hors de ma poitrine. ;)

Answer 2

Vous n'avez pas besoin d'un Finalize (destructeur) méthode, car vous n'avez pas d'objets managés. Toutefois, vous devez garder à l'appel d' GC.SuppressFinalize dans le cas d'une classe héritant de Foo a des objets non gérés, et donc d'un outil de finalisation.

Si vous faites de la classe scellée, alors vous savez que les objets managés n'allez jamais à entrer dans l'équation, donc il n'est pas nécessaire d'ajouter de l' protected virtual Dispose(bool) de surcharge, ou l' GC.SuppressFinalize.

Edit:

@AnthonyWJones de l'objection à cela est que si vous savez que le sous-classes seront pas référence à des objets managés, l'ensemble de l' Dispose(bool) et GC.SuppressFinalize est inutile. Mais si c'est le cas, vous devriez vraiment rendre les classes internal plutôt que d' public, et l' Dispose() méthode de virtual. Si vous savez ce que vous faites, alors sentez-vous libre de ne pas suivre Microsoft suggéré par le modèle, mais vous devez connaître et comprendre les règles avant de vous les briser!

Answer 3

La pratique recommandée consiste à utiliser un finaliseur seulement lorsque vous avez des ressources non managées (tels que les handles de fichiers natifs, les pointeurs de mémoire, etc).

J'ai deux petites suggestions si,

Il n'est pas nécessaire d'avoir la "m_Disposed" de la variable à tester si vous avez déjà appelé Dispose sur vos ressources gérées. Vous pourriez utiliser un code similaire comme,

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

Ouvrir des fichiers pour seulement aussi longtemps que nécessaire. Donc dans votre exemple, vous pouvez tester l'existence du fichier dans le constructeur à l'aide de File.Exists et puis quand vous avez besoin de lire/écrire le fichier, puis vous l'ouvrir et l'utiliser.

Aussi, si vous vous contentez de vous weant pour écrire du texte dans un fichier, avoir un regard sur File.WriteAllText ou File.OpenText ou même File.AppendText qui est destiné à des fichiers texte spécifiquement ASCIIEncoding.

Autres que que, oui, vous êtes la mise en œuvre de l' .NET Dispose correctement.

Answer 4

J'ai beaucoup de classes comme celle - ci, et ma recommandation est de faire de la classe scellée chaque fois que possible. IDisposable + héritage peuvent travailler, mais 99% du temps, vous n'avez pas besoin, et il est facile de faire des erreurs avec.

Sauf si vous écrivez une API publique (dans ce cas, il est bon de permettre à des personnes de mettre en œuvre votre code toutefois ils le souhaitent à - dire d'utiliser IDisposable), vous n'avez pas besoin de le soutenir.

juste à faire:

public sealed class Foo : IDisposable
{
    private StreamWriter _Writer;

    public Foo(string path)
    {
            FileStream fileWrite = File.Open (path, FileMode.OpenOrCreate, FileAccess.Write, FileShare.ReadWrite);
            try { 
                _Writer = new StreamWriter (fileWrite, new ASCIIEncoding());
            } catch {
                fileWrite.Dispose();
                throw;
            }
    }

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

Remarque le try...catch; techniquement, l' StreamWriter constructeur pourrait lancer une exception, dans ce cas il ne prend jamais la propriété de l' FileStream et vous devez vous débarrasser vous-même.

Si vous êtes vraiment en utilisant de nombreux IDisposables, envisager de mettre ce code en C++/CLI: c'est tout ce que le code réutilisable pour vous (de manière transparente des déterministe destruction tech pour les deux natifs et les objets gérés).

Wikipedia a une bonne IDisposable de l'échantillon pour le C++ (vraiment, si vous avez beaucoup de IDisposable, de C++ est en fait beaucoup plus simple que le C#): Wikipédia: C++/CLI Finaliseurs et variables automatiques.

Par exemple, la mise en œuvre d'un "coffre-fort" contenant jetable en C++/CLI ressemble...

public ref class MyDisposableContainer
{
	auto_handle<IDisposable> kidObj;
	auto_handle<IDisposable> kidObj2;
public:

	MyDisposableContainer(IDisposable^ a,IDisposable^ b) 
            : kidObj(a), kidObj2(b)
	{
		Console::WriteLine("look ma, no destructor!");
	}
};

Ce code permettra d'éliminer correctement les kidObj et kidObj2 même sans l'ajout d'une coutume IDisposable mise en œuvre, et il est robuste aux exceptions soit Disposer de mise en œuvre (pas que celles-ci doivent se produire, mais quand même), et simple à maintenir dans le visage de beaucoup plus d' IDisposable des membres ou des ressources autochtones.

Non pas que je suis un grand fan de C++/CLI, mais fortement axée sur les ressources de code, on a C# battre facilement, et il est absolument génial interop à la fois avec code natif et géré, bref, parfait colle le code ;-). J'ai tendance à écrire de 90% de mon code en C#, mais l'utilisation de C++/CLI pour tous les besoins d'interopérabilité (esp. si vous voulez l'appeler toute fonction win32 - MarshalAs et d'autres interop attributs de tous sur la place sont horrible et complètement incompréhensible).

Answer 5

Vous devez vérifier que _Writer n'est pas null avant de tenter de le disposer. (Il ne semble pas probable qu'il en sera toujours null, mais juste au cas où!)

protected virtual void Dispose(bool disposing)
{
    if (!_Disposed)
    {
        if (disposing && (_Writer != null))
        {
            _Writer.Dispose();
        }
        _Writer = null;
        _Disposed = true;
    }
}