views:

221

answers:

5

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

A: 

.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
A: 

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
+2  A: 

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
nice. Fantastic. Perfect.
ram
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
@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
A: 

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
+2  A: 

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 for the code. Linking your code in MSDN. Would have marked yours as answer had you answered earlier. good one anyways !!
ram