Il mio codice pulisce correttamente il suo elenco ?
-
20-08-2019 - |
Domanda
Ho un componente di terze parti che esegue la manipolazione dei file PDF. Ogni volta che ho bisogno di eseguire operazioni, recupero i documenti PDF da un archivio di documenti (database, SharePoint, filesystem, ecc.). Per rendere le cose un po 'coerenti, passo i documenti PDF come a byte[]
.
Questo componente di terze parti si aspetta a MemoryStream[]
(MemoryStream
Array) come parametro per uno dei metodi principali che devo usare.
Sto cercando di avvolgere questa funzionalità nel mio componente in modo da poter usare questa funzionalità per un numero di aree all'interno della mia applicazione. Ho escogitato essenzialmente quanto segue:
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
}
Il codice chiamante per il mio "wrapper" sembra questo:
byte[] manipulatedResult = null;
using (PdfDocumentManipulator manipulator = new PdfDocumentManipulator())
{
manipulator.AddFileToManipulate(file1bytes);
manipulator.AddFileToManipulate(file2bytes);
manipulatedResult = manipulator.Manipulate();
}
Alcune domande su quanto sopra:
- È il
using
clausola nelAddFileToManipulate()
Metodo ridondante e non necessario? - Sto ripulendo le cose ok nel mio oggetto
Dispose()
metodo? - È un uso "accettabile" di
MemoryStream
? Non sto anticipando molti file in memoria contemporaneamente ... probabilmente 1-10 pagine totali PDF, ogni pagina di circa 200kb. App progettata per essere eseguita su server a supporto di un sito ASP.NET. - Qualche commento/suggerimenti?
Grazie per la recensione del codice :)
Soluzione
- La clausola usando il metodo AddFileTomanipulate () è ridondante e non necessaria?
Peggio ancora, è distruttivo. Fondamentalmente stai chiudendo il flusso di memoria prima che venga aggiunto. Vedi le altre risposte per i dettagli, ma in sostanza, smalti alla fine, ma non altra volta. Ogni utilizzo con un oggetto provoca che si verifica una disposizione alla fine del blocco, anche se l'oggetto viene "passato" ad altri oggetti tramite metodi.
- Sto ripulendo le cose ok nel metodo Disose () del mio oggetto?
Sì, ma stai rendendo la vita più difficile di quanto deve essere. Prova questo:
foreach (var stream in this.pdfDocumentStreams)
{
stream.Dispose();
}
this.pdfDocumentStreams.Clear();
Funziona altrettanto bene ed è molto più semplice. Lo smaltimento di un oggetto non lo elimina: gli dice solo per liberare le sue risorse interne e non gestite. Chiamare Disosa su un oggetto in questo modo va bene: l'oggetto rimane non raccolto, nella raccolta. Puoi farlo e quindi cancellare l'elenco in un colpo.
- È un uso "accettabile" di Memorystream? Non sto anticipando molti file in memoria contemporaneamente ... probabilmente 1-10 pagine totali PDF, ogni pagina di circa 200kb. App progettata per essere eseguita su server a supporto di un sito ASP.NET.
Questo dipende dalla tua situazione. Solo tu puoi determinare se il sovraccarico di avere questi file in memoria ti causerà problemi. Questo sarà un oggetto abbastanza pesante, però, quindi lo userei attentamente.
- Qualche commento/suggerimenti?
Implementa un finalizer. È una buona idea ogni volta che implementi idisposibile. Inoltre, dovresti rielaborare l'implementazione di disporre a quella standard o contrassegnare la tua classe come sigillata. Per i dettagli su come questo dovrebbe essere fatto, Vedi questo articolo. In particolare, dovresti avere un metodo dichiarato come protected virtual void Dispose(bool disposing)
Che il tuo metodo di smalto e il tuo finalizer chiamano entrambi.
Altri suggerimenti
AddfiletomaniPulate mi spaventa.
public void AddFileToManipulate(byte[] pdfDocument)
{
using (MemoryStream stream = new MemoryStream(pdfDocument))
{
pdfDocumentStreams.Add(stream);
}
}
Questo codice sta aggiungendo un flusso eliminato all'elenco PDFDocumentstream. Invece dovresti semplicemente aggiungere il flusso usando:
pdfDocumentStreams.Add(new MemoryStream(pdfDocument));
E smaltirlo nel metodo Disosa.
Inoltre, dovresti cercare l'implementazione di un finalizzatore per assicurarti che le cose vengano eliminate nel caso in cui qualcuno dimentichi di smaltire l'oggetto di alto livello.
Mi sembra che tu fraintenda cosa fa l'utilizzo.
È solo zucchero sintattico da sostituire
MemoryStream ms;
try
{
ms = new MemoryStream();
}
finally
{
ms.Dispose();
}
Il tuo utilizzo in addfiletomaniPulate è ridondante. Avrei impostato l'elenco dei memorystreams nel costruttore di PDFDocumentManipulator, quindi avrei avuto la chiamata del metodo di smalto di PDFDocumentManipulator su tutti i ricordi.
Nota a margine. Questo sembra davvero richiedere un metodo di estensione.
public static void DisposeAll<T>(this IEnumerable<T> enumerable)
where T : IDisposable {
foreach ( var cur in enumerable ) {
cur.Dispose();
}
}
Ora il tuo metodo di smalto diventa
public void Dispose() {
pdfDocumentStreams.Reverse().DisposeAll();
pdfDocumentStreams.Clear();
}
MODIFICARE
Non è necessario il framework 3.5 per avere metodi di estensione. Lavoreranno felicemente sul compilatore 3.0 down mirato a 2.0
http://blogs.msdn.com/jaredpar/archive/2007/11/16/extension-methods-without-3-5-framework.aspx