2010-07-06 7 views
11

Sto lavorando su una classe che si occupa di un sacco di oggetti SQL - Connessione, Comando, DataAdapter, CommandBuilder, ecc Ci sono più istanze in cui abbiamo il codice come questo:funzione generica per gestire lo smaltimento di oggetti IDisposable

if(command != null) 
{ 
    command.Dispose(); 
} 

if(dataAdapter != null) 
{ 
    dataAdapter.Dispose(); 
} 

etc 

So che questo è abbastanza insufficiente in termini di duplicazione, ma ha iniziato a puzzare. Il motivo per cui penso che odori è perché in alcuni casi anche l'oggetto è impostato su null.

if(command != null) 
{ 
    command.Dispose(); 
    command = null; 
} 

Mi piacerebbe eliminare la duplicazione, se possibile. Sono arrivato con questo metodo generico per smaltire un oggetto e impostarlo su null.

private void DisposeObject<TDisposable>(ref TDisposable disposableObject) 
    where TDisposable : class, IDisposable 
{ 
    if(disposableObject != null) 
    { 
     disposableObject.Dispose(); 
     disposableObject = null; 
    } 
} 

Le mie domande sono ...

  1. È questo funzione generica una cattiva idea?
  2. È necessario impostare l'oggetto su null?

EDIT:

Sono consapevole della dichiarazione using, comunque non posso sempre usarlo perché ho alcune variabili membro che hanno bisogno di persistere più a lungo di una chiamata. Ad esempio la connessione e gli oggetti della transazione.

Grazie!

risposta

1

presumo questi sono campi e le variabili non locali, quindi, perché la parola chiave using non ha senso.

Is this generic function a bad idea?

Penso che sia una buona idea, e ho usato una funzione simile un paio di volte; +1 per renderlo generico.

Is it necessary to set the object to null?

Tecnicamente un oggetto dovrebbe consentire a più chiamate al suo metodo Dispose. (Ad esempio, ciò accade se un oggetto viene resuscitato durante la finalizzazione.) In pratica, dipende da te se ti fidi degli autori di queste classi o se vuoi codificare in modo difensivo. Personalmente, controllo null, quindi imposta i riferimenti a null successivamente.

Modifica: Se questo codice si trova all'interno del metodo Dispose del proprio oggetto, non riuscendo a impostare i riferimenti su null non si verificherà alcuna perdita di memoria. Invece, è utile come difesa contro il doppio smaltimento.

6

È necessario implementare IDisposable nella classe che possiede questi campi. Vedi my blog post sull'argomento. Se questo non funziona bene, la classe non segue i principi OOP e deve essere sottoposta a refactoring.

È not necessary impostare le variabili su null dopo averli eliminati.

+0

Non sono in grado di utilizzare l'istruzione 'using' per la maggior parte dei miei casi. Ho aggiornato la mia domanda. –

+0

@Jerod: ho aggiornato la mia risposta per corrispondere. –

7

È necessario considerare se è possibile utilizzare l'istruzione using.

using (SqlCommand command = ...) 
{ 
    // ... 
} 

Questo assicura che Dispose si chiama sull'oggetto di comando quando il controllo abbandona la portata del usando. Questo ha un certo numero di vantaggi rispetto alla scrittura del codice di pulizia come hai fatto:

  • È più conciso.
  • La variabile non verrà mai impostata su null - viene dichiarata e inizializzata in un'unica istruzione.
  • La variabile esce dall'ambito quando l'oggetto viene eliminato in modo da ridurre il rischio di tentare accidentalmente di accedere a una risorsa dismessa.
  • È un'eccezione sicura.
  • Se si annidano utilizzando le istruzioni, le risorse vengono naturalmente disposte nell'ordine corretto (l'ordine inverso in cui sono state create).

Is it necessary to set the object to null?

Di solito non è necessario impostare le variabili per null quando avete finito di usarli. L'importante è che chiami Dispose quando hai finito di usare la risorsa.Se si utilizza il modello di cui sopra, non è solo inutile per impostare la variabile su null - darà un errore di compilazione:

 
Cannot assign to 'c' because it is a 'using variable' 

Una cosa da notare è che using funziona solo se un oggetto viene acquisito e smaltiti in lo stesso metodo di chiamata. Non puoi usare questo modello se le tue risorse hanno bisogno di rimanere in vita per la durata di più di una chiamata di metodo. In questo caso potresti voler implementare la tua classe IDisposable e assicurarti che le risorse vengano ripulite quando viene chiamato il tuo metodo Dispose. In questo caso avrai bisogno del codice come hai scritto tu. L'impostazione delle variabili su null non è errata in questo caso, ma non è importante in quanto il garbage collector pulirà comunque correttamente la memoria. L'importante è assicurarsi che tutte le risorse che possiedi siano disposte quando viene chiamato il tuo metodo di smaltimento, e lo stai facendo.

Un paio di dettagli di implementazione:

  • È necessario assicurarsi che se il Dispose viene chiamato due volte che non si un'eccezione. La tua funzione di utilità gestisce correttamente questo caso.
  • È necessario assicurarsi che i metodi pertinenti sul proprio oggetto generino un ObjectDisposedException se l'oggetto è già stato eliminato.
+1

Questo naturalmente presuppone che la durata della risorsa sia solo una chiamata al metodo --- il che è improbabile in questo caso, come indicato dalla necessità di verificare prima i valori vuoti. –

+0

@James, hai assolutamente ragione. @ Marco, ho aggiornato la mia domanda. –

0

Altri hanno raccomandato il costrutto using, che consiglio anche. Tuttavia, vorrei sottolineare che, anche se hai bisogno di un metodo di utilità, non è assolutamente necessario renderlo generico nel modo in cui hai fatto. Basta dichiarare il vostro metodo di prendere un IDisposable:

private static void DisposeObject(ref IDisposable disposableObject) 
{ 
    if(disposableObject != null) 
    { 
     disposableObject.Dispose(); 
     disposableObject = null; 
    } 
} 
+0

Ho provato questo, ma per qualche motivo dovevo lanciare tutte le mie chiamate al metodo. Sono d'accordo che questo dovrebbe funzionare! Dovrò ricontrollare. –

+0

@Jerod: l'imballaggio non funzionerà, poiché si modifica il valore di 'disposable' invece di' mTransaction'. @JS: Dal momento che stiamo * modificando * disposableObject, il compilatore deve conoscere il tipo esatto che è. –

+2

@Jerod - i parametri del metodo non possono essere covarianti in questo caso a causa della parola chiave ref ', altrimenti per esempio potresti passare in un 'SqlConnection', quindi impostare l'oggetto su un'istanza di' FileStream' all'interno del metodo (che violerebbe la sicurezza del tipo dell'argomento) –

1

Presumo che si stia creando la risorsa in un modo, lo si smaltisce in un altro e lo si utilizza in uno o più altri, rendendo inutile l'istruzione using.

In tal caso, il metodo è perfetto.

Per quanto riguarda la seconda parte della domanda ("È necessario impostarla su null?"), La risposta semplice è "No, ma non fa male nulla".

La maggior parte degli oggetti contiene una risorsa, ovvero la memoria, che la raccolta di dati inutili tratta della liberazione, quindi non dobbiamo preoccuparcene. Alcuni hanno anche qualche altra risorsa: un handle di file, una connessione al database, ecc. Per la seconda categoria, dobbiamo implementare IDisposable, per liberare quell'altra risorsa.

Una volta chiamato il metodo Dispose, entrambe le categorie sono uguali: conservano la memoria. In questo caso, possiamo semplicemente lasciare che la variabile vada fuori campo, lasciando cadere il riferimento alla memoria e consentire a GC di liberarlo alla fine - Oppure possiamo forzare il problema, impostando la variabile su null e facendo cadere esplicitamente il riferimento alla memoria. Dobbiamo ancora aspettare che il GC esca per far sì che la memoria venga effettivamente liberata, e più che probabile che la variabile andrà fuori campo comunque, pochi istanti dopo per impostarla su null, quindi nella stragrande maggioranza dei casi, avrà nessun effetto, ma in alcuni rari casi, permetterà alla memoria di essere liberata pochi secondi prima.

Tuttavia, se il caso specifico, in cui si sta verificando per nulla per vedere se si dovrebbe chiamare Dispose a tutti, probabilmente dovrebbe impostato a null, se c'è una possibilità che si potrebbe chiamare Dispose() due volte.

0

Non è mai necessario impostare le variabili su null. L'intero punto di IDisposable.Dispose consiste nel riportare l'oggetto in uno stato in cui è possibile aggrapparsi alla memoria in modo innocuo finché il GC non lo finalizza, quindi basta "disporre e dimenticare".

Sono curioso del motivo per cui pensi di non poter usare l'istruzione using. Creare un oggetto in un metodo e smaltirlo con un metodo diverso è uno codice postale nel mio libro, perché presto perdi traccia di ciò che è aperto dove. Meglio refactoring del codice come questo:

using(var xxx = whatever()) { 
    LotsOfProcessing(xxx); 
    EvenMoreProcessing(xxx); 
    NowUseItAgain(xxx); 
} 

Sono sicuro che c'è un nome di modello standard per questo, ma ho appena chiamarlo "distruggere tutto ciò che si crea, ma nient'altro".

+0

Sono d'accordo che questo non sia privo di odori di codice. Questo codice proviene da una classe di librerie SQL utilizzata in diversi prodotti. Il motivo principale per cui l'istruzione 'using' non funziona è perché una delle funzionalità necessarie è il supporto delle transazioni. L'utente della libreria ci dice di avviare la transazione, quindi possono eseguire più manipolazioni di dati, quindi ci dicono di eseguire la transazione. Penso che sia un caso d'uso molto ragionevole, in quanto crea una separazione tra il codice client e il codice SQL specifico. –

+0

@Jerod, non è possibile creare la connessione, iniziare la transazione, chiamare tutti i metodi della libreria, eseguire il commit della transazione e disporre la connessione, tutto in un unico metodo? Quello che descrivi è esattamente come ho progettato le mie classi di librerie SQL, ma posso ancora usare le istruzioni 'using'. –

+0

che è possibile tranne che l'utente avrebbe bisogno di conoscere i dettagli intricati sulla nostra biblioteca. Sfortunatamente, non sono libero di cambiare la nostra API pubblica a questo punto e devo lavorare con quello che ho ottenuto. –

3

Se gli oggetti hanno un sacco di pulizia da fare, potrebbero voler tenere traccia di ciò che deve essere eliminato in un elenco separato di prodotti usa e getta, e gestirli tutti in una volta. Quindi a smantellamento non ha bisogno di ricordare tutto ciò che ha bisogno di essere smaltito (né ha bisogno di verificare la presenza di null, appare solo nella lista).

Questo probabilmente non crea, ma per scopi esplicativi, potresti includere un RecycleBin nella tua classe. Quindi la classe deve solo sbarazzarsi del cestino.

public class RecycleBin : IDisposable 
{ 
    private List<IDisposable> _toDispose = new List<IDisposable>(); 

    public void RememberToDispose(IDisposable disposable) 
    { 
     _toDispose.Add(disposable); 
    } 

    public void Dispose() 
    { 
     foreach(var d in _toDispose) 
      d.Dispose(); 

     _toDispose.Clear(); 
    } 
} 
+0

+1 Questo non funziona per la mia situazione attuale perché non dispongo tutto in una volta. Anche se questo suggerimento mi piace e potrebbe doverlo usare in futuro. –

1

Dato che IDisposable non include alcun modo standard per determinare se un oggetto è stato eliminato, mi piace impostare attrattive nullo quando disporne. Per essere sicuri, smaltire un oggetto che è già stato smaltito è innocuo, ma è bello poter esaminare un oggetto in una finestra di controllo e dire immediatamente quali campi sono stati eliminati. È anche bello poter eseguire il test del codice per assicurarsi che gli oggetti che dovrebbero essere stati eliminati siano (supponendo che il codice aderisca alla convenzione di annullare le variabili quando vengono eliminati e in nessun altro momento).

Problemi correlati