2009-03-27 6 views
4

J'ai un code comme ceci:C# CA2104 - code automatisé analyse n'aime statiques types readonly mutables

public abstract class Base 
{ 
    // is going to be used in deriving classes 
    // let's assume foo is threadsafe 
    protected static readonly Foo StaticFoo = new Foo(); 
} 

Visual Studio code de 2008 Analyse Déroule ce message:

CA2104 : Microsoft.Security : Remove the read-only designation from 'Base.StaticFoo' or change the field to one that is an immutable reference type. If the reference type 'Foo' is, in fact, immutable, exclude this message.

Ma conception est-elle entachée d'un défaut d'ordre, ou puis-je ajouter un [SuppressMessage] dans la source?

Répondre

8

Le problème est qu'il donne une fausse impression - il donne l'impression que la valeur ne peut pas changer, alors qu'en réalité c'est seulement la valeur du champ qui ne peut pas changer, plutôt que l'objet. Je pense que l'analyse de code est trop prudente ici - il y a beaucoup d'endroits où vous pourriez vouloir un type mutable stocké dans un champ readonly, particulièrement s'il est privé, initialisé dans un initialiseur statique et ensuite jamais muté dans votre code. Bien que le champ soit protégé, les classes dérivées risquent davantage de l'utiliser abusivement. Personnellement, j'aime que tous mes champs soient privés. D'un autre côté, je ne connais pas votre code - il se peut que vous ayez pesé cela par rapport aux possibilités et décidé que c'est la bonne chose à faire.

Si Foo est vraiment thread-safe, il y a moins à craindre, à part l'impression générale d'immuabilité donnée par le fait que le champ est en lecture seule. Je désactiverais l'avertissement pour cette ligne, et ajouterais un commentaire pour souligner que l'objet est mutable même si le champ est en lecture seule.

+0

Quelqu'un peut-il voir quelque chose d'utile dans cette réponse qui n'est pas dans les deux plus âgés? Sinon, je vais supprimer. –

+0

Oui, votre réponse a été utile, s'il vous plaît ne pas l'enlever. – mafu

+0

En fait j'utilise souvent readonly (mais pas statique) sur les champs privés que je modifie. La seule chose que je veux m'assurer, c'est que l'instance est toujours la même et je ne l'échangerai pas accidentellement par la suite. Donc, je pense que readonly avec des types mutables est toujours utile. – mmmmmmmm

1

L'avertissement vous indique que le modificateur readonly ne s'applique qu'à la référence elle-même. Si Foo est un type de référence, l'état de votre instance Foo peut toujours être modifié, sauf si vous vous assurez que Foo est immuable. Sur un autre nœud, des statistiques de ce genre peuvent vous donner toutes sortes de problèmes quand il s'agit de tester le code unitaire, donc si cela est important pour vous, vous voudrez peut-être chercher d'autres façons de réaliser ce que vous voulez. réessayer de faire.

3

Le type Foo est-il immuable? Si ce n'est pas le cas, avez-vous l'intention d'utiliser uniquement cet objet Foo, mais d'en modifier les propriétés? Si tel est le cas, l'avertissement indique simplement que le mot-clé readonly est trompeur. Il n'y a pas d'erreur de compilation - la référence à l'objet est en lecture seule, et c'est ce que vous avez déclaré au compilateur. Cependant, ce que vous avez déclaré aux autres développeurs, c'est que StaticFoo est en lecture seule, ce qui implique qu'il ne changera jamais. Donc, vous avez le choix, comme il est dit. Pour éliminer cet avertissement, supprimez le clavier de lecture en lecture seule ou ajoutez un attribut SuppressMessage. Alternativement, regardez la conception de votre code. Est-ce que l'implémentation de Foo en tant que pattern singleton serait plus appropriée par exemple?