7

I have been trying to wrap my head around this FXCop violation "DoNotDeclareReadOnlyMutableReferenceTypes"

MSDN: http://msdn.microsoft.com/en-us/library/ms182302%28VS.80%29.aspx

Code from MSDN which would cause this violation:

namespace SecurityLibrary
{
    public class MutableReferenceTypes
    {
        static protected readonly StringBuilder SomeStringBuilder;

        static MutableReferenceTypes()
        {
            SomeStringBuilder = new StringBuilder();
        }
    }
}

From Jon's answer here and here , I understand that the field holding the reference to the object (in this case SomeStringBuilder) is readonly and not the object itself (which is created by new StringBuilder() )

So taking this example, how would I change the object itself, once the field has a reference to it ? I like Eric Lippert's example of how the readonly array can be changed, and would like to see something similar for any other mutable reference type

Community
  • 1
  • 1
ram
  • 11,468
  • 16
  • 63
  • 89

5 Answers5

6

readonly means you can't change the reference post-construction.

The official FXCop stance is that it recommends that only types that can't be modified should be declared readonly. Therefore something like a string is okay because the value of the object can't be changed. However the value of StringBuilder can changed but making it readonly only prevents you from assigning the field to a different StringBuilder instance or null after the constructor runs.

I disagree with FXCop on this rule. As long as one understands that this is simply an enforcement that the reference may not change post-construction then there is no confusion.

Note that value-types are made immutable by the readonly keyword but reference types are not.

namespace SecurityLibrary
{
    public class MutableReferenceTypes
    {
        static protected readonly StringBuilder SomeStringBuilder;

        static MutableReferenceTypes()
        {
            // allowed
            SomeStringBuilder = new StringBuilder();
        }

        void Foo()
        {
            // not allowed
            SomeStringBuilder = new StringBuilder();
        }

        void Bar()
        {
            // allowed but FXCop doesn't like this
            SomeStringBuilder.AppendLine("Bar");
        }
    }
}
Brandon Cuff
  • 1,438
  • 9
  • 24
  • +1 for the code. Linking your code in MSDN. Would have marked yours as answer had you answered earlier. good one anyways !! – ram Feb 16 '10 at 18:17
  • Old answer, but still very relevant. Code Analysis (as it is now) still has some silly rules (such as this one) which assume coders are idiots. Anyone who even half way understands the language knows what `readonly` means. First thing I do when setting up a new project is disable a ton of CA rules such as CA2104. – 0b101010 Jul 08 '16 at 09:37
3

As the MutableReferenceTypes class is presented in the question, you can't really mutate it from any outside caller since the SomeStringBuilder field is private.

However, the class itself could mutate the field. It doesn't currently, but it could in a later iteration.

Here's an example method:

public static void Mutate()
{
    SomeStringBuilder.AppendLine("Foo");
}

Calling the Mutate method will mutate the class because SomeStringBuilder will now have changed.

Immutability is not only about the current incarnation of your code, but also about protecting yourself from future mistakes. Not that all classes need to be immutable, but it's safest to stay consistent if you elect to create an immutable type.

Mark Seemann
  • 225,310
  • 48
  • 427
  • 736
  • 2
    Just a tiny mistake: The field is **protected**, not private, hence it is *absolutely* mutable from the outside. I guess *this* is what FXCop is objecting to. – Konrad Rudolph Feb 16 '10 at 18:57
  • @Konrad Rudolph: Good catch! I just looked at the first keyword and noticed that it wasn't an access modifier, and therefore must have defaulted to the default (sic). I didn't notice that the keywords had been switched around. – Mark Seemann Feb 16 '10 at 19:29
0

.Net has a list of immutable reference types that are allowed here, StringBuilder isn't one of them.

The complaint is that what you're constructing isn't immutable, though the static constructor is called once and the class is initialized once, that's all that stays the same, the rest is mutable. One thread may call .Append(), then another...you see how the string builder itself mutates and isn't really readonly, because it constantly changes states/mutates.

Declaring it readonly is really a misnomer since the object referenced there itself is constantly changing.

Nick Craver
  • 623,446
  • 136
  • 1,297
  • 1,155
  • 1
    How else should one distinguish between a reference-type field which will always point to the same object, and one which may point to different objects during the lifetime of the containing object? Someone who doesn't understand what reference types are may be confused, but such a person is apt to be confused about many things unless or until such time as he learns to understand reference types. – supercat Nov 03 '12 at 03:50
0

You cannot change the reference, but any call on the (mutable) object changes its state.

Therefore, since the SomeStringBuilder (in this example) is mutable itself, its content may change, which may be misleading for users of the class because it is therefore not really "read-only".

Basically, readonlydoes not in any way guarantee that the object foes not change, it merely does say that the reference does not change.

Lucero
  • 59,176
  • 9
  • 122
  • 152
  • 1
    The fact that a reference does not change may be vital to other parts of the code (e.g. some other object may grab a reference to the StringBuilder and expect to be able to grab its value); while one should certainly be aware that the referenced object may be mutated, that does not imply that there's anything useless about declaring the reference itself immutable. BTW, even delegates aren't always immutable; a delegate formed from a structure mutator will box the structure, and the boxed structure will be mutable. – supercat Jan 20 '11 at 15:52
0

You would not change the value of the object. That's the point of the rule. Any field declared as readonly should infact be that, readonly. Having a readonly mutable reference is an oxymoron. If you Can change the value of what the field "points" to then it's not really readonly any more. There's really no functional difference between assigning the value of all members of some object A to some object B represented by a field or simply assigning A to that field (when they are of the same type) however only one of Them is valid when the field is declared readonly but since you Can effectively chage the value of what's being represented by the field it is as already stated not really readonly

Rune FS
  • 21,497
  • 7
  • 62
  • 96