2012-01-19 9 views
31

ho qualche codice che attiverà CA1063 avvertimento analisi del codice:incendi analisi del codice CA1063 quando derivanti da IDisposable e forniscono implementazione in classe di base

CA1063: Microsoft.Design: Rimuovere IDisposable dall'elenco delle interfacce implementate da 'Funzionalità' e sostituisce invece l'implementazione Dispose della classe base.

Tuttavia, non sono sicuro di cosa devo fare per correggere questo avviso.

In breve, ho un'interfaccia IFunctionality che deriva da IDisposable. La classe Functionality implementa IFunctionality ma deriva dalla classe Reusable per poter riutilizzare il codice som. La classe Reusable deriva anche da IDisposable.

public class Reusable : IDisposable { 

    ~Reusable() { 
    Dispose(false); 
    } 

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

    protected virtual void Dispose(Boolean disposing) { 
    // ... 
    } 

    public void DoSomething() { 
    // ... 
    } 

} 

public interface IFunctionality : IDisposable { 

    void DoSomething(); 

    void DoSomethingElse(); 

} 

public class Functionality : Reusable, IFunctionality { 

    public void DoSomethingElse() { 
    // ... 
    } 

#if WORK_AROUND_CA1063 
    // Removes CA1063 
    protected override void Dispose(Boolean disposing) { 
    base.Dispose(disposing); 
    } 
#endif 

} 

posso liberarmi di avvertimento sovrascrivendo Dispose su Functionality e chiamando la classe di base Dispose anche se farlo non dovrebbe cambiare la semantica del codice.

Quindi c'è qualcosa di IDisposable in questo contesto che ho trascurato o è solo CA1063 che si mischia per questo particolare costrutto?

So che posso sopprimere CA1063 ma la regola è abbastanza ampia e non voglio perdere nessun altro problema nell'implementazione di IDisposable riportato da questa regola.

+0

vecchia questione, ma i problemi con CA1063 sono molte volte superare contrassegnando la classe come 'sealed'. Se la classe non è 'sealed' penso che la classe deve' virual' o/e che il metodo 'Dispose()' deve essere 'protected' – mortb

risposta

34

Questo è un falso positivo a causa di un bug secondario nella regola stessa. Quando si tenta di capire se una classe reimplementa IDisposable (dopo aver scoperto che esiste un'implementazione di una classe base che può essere sovrascritta), controlla solo se le interfacce della classe includono IDisposable. Sfortunatamente, l'elenco delle interfacce visualizzato nei metadati dell'assieme include l'elenco "esploso" di interfacce, comprese le interfacce ereditate tramite interfacce che la classe implementa esplicitamente nel codice C# originale.Ciò significa che FxCop sta vedendo una dichiarazione che appare come il seguente per la classe Funzionalità:

public class Functionality : Reusable, IFunctionality, IDisposable 
{ 
    ... 
} 

Data questa rappresentazione dei metadati, la regola ImplementIDisposableCorrectly dovrebbe essere un po 'più intelligente su come si tenta di determinare se la classe è in realtà reimplementare IDisposable (ad esempio, cercando un'implementazione esplicita di Dispose() se la classe base ha un Dispose overridable (bool)). Tuttavia, dato che la regola non lo fa, il tuo approccio migliore è quello di eliminare i falsi positivi.

BTW, suggerirei seriamente di considerare l'utilizzo di SuppressMessageAttribute per sopprimere i falsi positivi anziché l'attuale metodo di compilazione condizionale. es .:

[SuppressMessage("Microsoft.Design", "CA1063:ImplementIDisposableCorrectly", 
    Justification = "False positive. IDisposable is inherited via IFunctionality. See http://stackoverflow.com/questions/8925925/code-analysis-ca1063-fires-when-deriving-from-idisposable-and-providing-implemen for details.")] 
public class Functionality : Reusable, IFunctionality 
{ 
    ... 
} 

Inoltre, si potrebbe prendere in considerazione seriamente getting rid of the finalizer ...

+0

+1 per il suggerimento SupressMessage –

+0

Il collegamento per "eliminare il finalizzatore" ha ceduto il collegamento a rot. Non posso dire se in origine avesse indicato http://joeduffyblog.com/2005/12/27/never-write-a-finalizer-again-well-almost-never/ o http://joeduffyblog.com/2008/02/17/idisposable-finalization-and-concorrency /, o forse qualche altro articolo che non ho ancora trovato. Potresti modificare la risposta per correggere il link? –

+0

Fatto. (La tua prima ipotesi era giusta.) –

1

Questo è correlato all'uso di IDisposable anziché all'interfaccia stessa. Stai semplicemente implementando il modello consigliato per il suo utilizzo fornendo e sovrascrivendo un metodo protetto Dispose(bool) - questo non fa parte dell'interfaccia stessa.

Se il metodo di sovrascrittura in realtà non aggiunge nulla, non c'è alcun problema ad ometterlo. Attenzione CA1063 è lì per evidenziare il problema, ma se sai che non ci sono effettivamente risorse da smaltire nel tuo caso, puoi aggirare il problema fornendo l'implementazione vuota che hai o escludendo questa particolare regola per questo particolare file.

È possibile eseguire questa operazione con l'attributo [SuppressMessage].

4

La "soluzione" è lo schema corretto qui, per una classe derivata che implementa nuovamente IDisposable.

Ma penso che dovresti riconsiderare il design di IFunctionality : IDisposable. Essere monouso è davvero una preoccupazione di IFunctionality? Penso che questa decisione appartenga alla classe di attuazione.

+0

' IDisposable' è una parte importante del contratto di 'IFunctionality' che è usato dal resto del sistema. Tuttavia, 'Reusable' sta fornendo l'implementazione di' Dispose'. Da qui il design. –

+0

"IDisposable è una parte importante del contratto di IFunctionality" - sembra che tu stia mischiando l'implementazione in interfacce. Ma il campione è troppo astratto per dirlo. –

+0

È importante che gli utenti di 'IFunctionality' eliminino l'istanza fornita durante l'arresto. Quindi fondamentalmente 'IFunctionality' ha un metodo per arrestare l'istanza e ho deciso di usare' IDisposable.Dispose' per questo scopo. Qualsiasi feedback su modi alternativi per progettare questo è molto apprezzato, ma suppongo che dovrebbe quindi andare in un'altra domanda. –