Mon code nettoie-t-il correctement sa liste <MemoryStream>?

Question

J'ai un composant tiers qui manipule les fichiers PDF. Chaque fois que j'ai besoin d'effectuer des opérations, je récupère les documents PDF à partir d'un magasin de documents (base de données, SharePoint, système de fichiers, etc.). Pour rendre les choses un peu cohérentes, je passe les documents PDF autour de byte[].

Ce composant tiers attend un MemoryStream[] (tableau MemoryStream) en tant que paramètre de l'une des principales méthodes que j'ai besoin d'utiliser.

Je tente d'intégrer cette fonctionnalité dans mon propre composant afin que je puisse utiliser cette fonctionnalité pour un certain nombre de domaines dans mon application. Je suis venu avec essentiellement les suivantes:

public class PdfDocumentManipulator : IDisposable 
{ 
    List<MemoryStream> pdfDocumentStreams = new List<MemoryStream>(); 

    public void AddFileToManipulate(byte[] pdfDocument) 
    { 
     using (MemoryStream stream = new MemoryStream(pdfDocument)) 
     { 
     pdfDocumentStreams.Add(stream); 
     } 
    } 

    public byte[] ManipulatePdfDocuments() 
    { 
     byte[] outputBytes = null; 

     using (MemoryStream outputStream = new MemoryStream()) 
     { 
      ThirdPartyComponent component = new ThirdPartyComponent(); 
      component.Manipuate(this.pdfDocumentStreams.ToArray(), outputStream); 

      //move to begining 
      outputStream.Seek(0, SeekOrigin.Begin); 

      //convert the memory stream to a byte array 
      outputBytes = outputStream.ToArray(); 
     } 

     return outputBytes; 
    } 

    #region IDisposable Members 
    public void Dispose() 
    { 
     for (int i = this.pdfDocumentStreams.Count - 1; i >= 0; i--) 
     { 
      MemoryStream stream = this.pdfDocumentStreams[i]; 
      this.pdfDocumentStreams.RemoveAt(i); 
      stream.Dispose(); 
     } 
    } 
    #endregion 
} 

Le code d'appel à mon "wrapper" ressemble à ceci:

byte[] manipulatedResult = null; 
    using (PdfDocumentManipulator manipulator = new PdfDocumentManipulator()) 
    { 
     manipulator.AddFileToManipulate(file1bytes); 
     manipulator.AddFileToManipulate(file2bytes); 
     manipulatedResult = manipulator.Manipulate(); 
    } 

Quelques questions sur ce qui précède:

1. Est-ce le using clause dans la méthode AddFileToManipulate() redondant et inutile? Est-ce que je nettoie les choses correctement dans la méthode Dispose() de mon objet? Est-ce une utilisation «acceptable» de MemoryStream? Je ne prévois pas beaucoup de fichiers dans la mémoire à la fois ... Probablement 1-10 pages au total PDF, chaque page environ 200KB. Application conçue pour fonctionner sur un serveur prenant en charge un site ASP.NET.
2. Des commentaires/suggestions?

Merci pour la révision du code :)

Answer 1

1. La clause relative à l'aide de la méthode de AddFileToManipulate() redondant et inutile?

Pire, c'est destructeur. Vous fermez votre flux de mémoire avant de l'ajouter. Reportez-vous aux autres réponses pour les détails, mais jetez-les à la fin, mais pas à n'importe quel moment. Chaque utilisation avec un objet provoque un Dispose à se produire à la fin du bloc, même si l'objet est "passé" à d'autres objets via des méthodes. Est-ce que je nettoie les choses correctement dans la méthode Dispose() de mon objet? Oui, mais vous rendez la vie plus difficile qu'elle ne devrait l'être. Essayez ceci:

 
foreach (var stream in this.pdfDocumentStreams) 
{ 
    stream.Dispose(); 
} 
this.pdfDocumentStreams.Clear(); 

Cela fonctionne tout aussi bien, et est beaucoup plus simple. La mise au rebut d'un objet ne le supprime pas - il lui dit simplement de libérer ses ressources internes non gérées. Appeler disposer sur un objet de cette façon est bien - l'objet reste non collecté, dans la collection. Vous pouvez le faire et effacer la liste en une fois.

3. Est-ce une utilisation "acceptable" de MemoryStream? Je ne prévois pas beaucoup de fichiers dans la mémoire à la fois ... Probablement 1-10 pages au total PDF, chaque page environ 200KB. Application conçue pour fonctionner sur un serveur prenant en charge un site ASP.NET.

Cela dépend de votre situation. Vous seul pouvez déterminer si la surcharge d'avoir ces fichiers en mémoire va vous causer des problèmes. Cela va être un objet assez lourd, donc, je l'utiliserais avec soin.

4. Des commentaires/suggestions?

Implémenter un finaliseur. C'est une bonne idée à chaque fois que vous implémentez IDisposable. En outre, vous devez retravailler votre implémentation Dispose à la version standard, ou marquer votre classe comme étant scellée. Pour plus de détails sur la façon dont cela devrait être fait, see this article. En particulier, vous devriez avoir une méthode déclarée comme protected virtual void Dispose(bool disposing) que votre méthode Dispose et votre finaliseur à la fois appel.

Answer 2

Il me semble que vous ne comprenez pas ce que l'aide fait.

Il est juste du sucre syntaxique pour remplacer

MemoryStream ms; 
try 
{ 
ms = new MemoryStream(); 
} 
finally 
{ 
ms.Dispose(); 
} 

Votre utilisation dans AddFileToManipulate est redondant. Je mettrais en place la liste des flux de mémoire dans le constructeur de PdfDocumentManipulator, puis ferais disposer de l'appel de méthode de disposition de PdfDocumentManipulator sur tous les flux de mémoire.

Answer 3

Note latérale. Cela semble vraiment appeler une méthode d'extension.

public static void DisposeAll<T>(this IEnumerable<T> enumerable) 
    where T : IDisposable { 
    foreach (var cur in enumerable) { 
    cur.Dispose(); 
    } 
} 

Maintenant votre méthode Dispose devient

public void Dispose() { 
    pdfDocumentStreams.Reverse().DisposeAll(); 
    pdfDocumentStreams.Clear(); 
} 

EDIT

Vous n'avez pas besoin du cadre 3.5 afin d'avoir des méthodes d'extension. Ils seront heureux de travailler sur le compilateur 3.0 vers le bas ciblé à 2,0

http://blogs.msdn.com/jaredpar/archive/2007/11/16/extension-methods-without-3-5-framework.aspx

Answer 4

AddFileToManipulate me fait peur.

public void AddFileToManipulate(byte[] pdfDocument) 
    { 
     using (MemoryStream stream = new MemoryStream(pdfDocument)) 
     { 
     pdfDocumentStreams.Add(stream); 
     } 
    } 

Ce code ajoute un flux disposé à votre liste pdfDocumentStream.Au lieu de cela, vous devez simplement ajouter le flux en utilisant:

pdfDocumentStreams.Add(new MemoryStream(pdfDocument)); 

Et de le jeter dans la méthode Dispose.

Vous devriez également envisager de mettre en œuvre un finaliseur pour s'assurer que les éléments sont éliminés au cas où quelqu'un oublierait de jeter l'objet de niveau supérieur.