.net Immutable objects

Question

I want to enforce on my code base immutable rule with following test

[TestFixture]
public class TestEntityIf
{
    [Test]
    public void IsImmutable()
    {
        var setterCount =
            (from s in typeof (Entity).GetProperties(BindingFlags.Public | BindingFlags.Instance)
             where s.CanWrite
             select s)
                .Count();

        Assert.That(setterCount == 0, Is.True, "Immutable rule is broken");
    }
}

It passes for:

public class Entity
{
    private int ID1;
    public int ID
    {
        get { return ID1; }
    }
}

but doesn't for this:

public class Entity
{
    public int ID { get; private set; }
}

And here goes the question "WTF?"

Answer 1

Surely it's your private set in the second one.

In the first, an instance of the class Entity can have its ID1 property written to by an external class.

In the latter, the setter is private to the class itself and so can only be called from within Entity (i.e. it's own constructor/initializer)

Take the private out of your setter in the second and it should pass

Answer 2

I think the reason is that CanWrite says that it returns true if there's a setter. A private setter is also a setter.

What surprises me a bit is that the first passes, as it has a public setter, so unless I'm still too low on cafeine, the settercount is 1 so the assert should fail. They both should fail, as CanWrite simply returns true for both. (and the linq query simply retrieves public properties, including the ID, as it is public)

(edit) I now see you changed the code of the first class, so it doesn't have a setter anymore.

So the thing is your assumption that CanWrite looks at accessors of the setter method, that's not the case. You should do:

var setterCount =
            (from s in typeof (Entity).GetProperties(BindingFlags.Public | BindingFlags.Instance)
             where s.GetSetMethod().IsPublic
             select s)
                .Count();

Answer 3

The problem is, that the property is public - because of the public getter - and it is writeable - because of the private setter. You will have to refine your test.

Further I want to add, that you cannot guarantee immutability this way because you could modify private data inside methods. To guarantee immutability you have to check that all fields are declared readonly and there are no auto-implemented properties.

public static Boolean IsImmutable(this Type type)
{
    const BindingFlags flags = BindingFlags.Instance |
                               BindingFlags.NonPublic |
                               BindingFlags.Public;

    return type.GetFields(flags).All(f => f.IsInitOnly);
}

public static Boolean IsImmutable(this Object @object)
{
    return (@object == null) ? true : @object.GetType().IsImmutable();
}

With this extension method you can easily test types

typeof(MyType).IsImmutable()

and instances

myInstance.IsImmutable()

for their immutability.

Notes

• Looking at instance fields allows you to have writable properties, but ensures that there are now fields that could be modified.
• Auto-implemented properties will fail the immutability test as exspected because of the private, anonymous backing field.
• You can still modify readonly fields using reflection.
• Maybe one should check that the types of all fields are immutable, too, because this objects belong to the state.
• This cannot be done with a simple FiledInfo.FieldType.IsImmutable() for all fields because of possible cycles and because the base types are mutable.

Answer 4

You probably need to call

propertyInfo.GetSetMethod().IsPublic

because the set-method and the get-method do not have the same access modifier, you can't rely on the PropertyInfo itself.

    var setterCount =
        (from s in typeof (Entity).GetProperties(
           BindingFlags.Public 
           | BindingFlags.NonPublic 
           | BindingFlags.Instance)
         where 
           s.GetSetMethod() != null       // public setter available
         select s)

Answer 5

If I understood you correctly, you want Entity to be immutable. If so, then the test should be changed to

var setterCount = (from s in typeof(string).GetProperties(BindingFlags.Public | BindingFlags.Instance).Select(p => p.GetSetMethod())
    where s != null && s.IsPublic
    select s).Count();

Assert.That(setterCount == 0, Is.True, "Immutable rule is broken");

Answer 6

I suggest a change in the property condition to:

s.CanWrite && s.GetSetMethod().IsPublic

Answer 7

Have you tried to debug it to see what properties whose CanWrite property is true are being returned by your LINQ query? Apparently, it is picking up things that you don't expect. With that knowledge, you can make your LINQ query more selective to work the way you want it to. Also, I agree with @Daniel Bruckner that your class is not completely mutable unless fields are also readonly. The caller may call a method or use a getter that may internally modify private fields that're exposed publicly as readonly via getters and this would break the immutable rule, take the client by surprise, and cause problems. The operations on mutable object should not have side effects.

Answer 8

A small modification to answers posted elsewhere. The following will return non-zero if there is at least one property with a protected or public setter. Note the check for GetSetMethod returning null (no setter) and the test for IsPrivate (i.e. not public or protected) rather than IsPublic (public only).

    var setterCount =
           (from s in typeof(Entity).GetProperties(
              BindingFlags.Public
              | BindingFlags.NonPublic
              | BindingFlags.Instance)
            where
              s.GetSetMethod(true) != null // setter available
              && (!s.GetSetMethod(true).IsPrivate)
            select s).Count();

Nevertheless, as pointed out in Daniel Brückner's answer, the fact that a class has no publicly-visible property setters is a necessary, but not a sufficient condition for the class to be considered immutable.