Why does resharper suggest readonly fields

Why is ReSharper suggesting readonly field for 'settings' in my example below?

If I understand correctly, you should use readonly modifier if you change this field only in constructor, but in my example I also change it in another method in the same class.

What am I missing?

public partial class OptionsForm : Form
{
    private Settings setting开发者_高级运维s;

    public OptionsForm(Settings s)
    {
        settings = s;
    }

    private void SaveData()
    {
        settings.ProjectName = TextBoxProject.Text;
    }
}

When a reference type is declared as readonly, the pointer is immutable, but not the object it points to. This means that:

• a reference type data member can be initialized in order to point to an instance of a class, but once this is done it's not possible to make it point to another instance of a class outside of constructors
• the readonly modifier has no effect on the object the readonly data member points to.

Read a detailed article on this

Mark a C# class data member as readonly when it’s read only

Remember the primary reason for coding standards and design patterns is to make to easier for people to understand your code.

By marking a field as “readonly” you are telling the reader of the class that they don’t need to consider how the value of the field is changed.

However as the object the readonly field points to can have it’s state change, marking a field as readonly can be misleading at times. So think about weather it helps the reader (e.g a person) of your code understand your design or not.

If the values in the object that the field points to changes within the object lifetime, then I don’t think a field should be marked read-only. (E.g the object pointed to should be behave as if it is immutable by the time the contractor on your class is called)

(However there are some exceptions, e.g it is OK to use a read-only field to point to a logger even thought the logger does change the state of the log file.)

ReSharper suggests making "settings" readonly:

readonly private Settings settings;

public OptionsForm(Settings s)
{
    settings = s;
}

because, upon scanning your code it concludes that your "settings" field only occurs in the constructor for this same class.

If howerver you were to provide a partial class or some other code in this class that modified "settings", it would no longer suggest it was readonly.

Once marked readonly the compiler will flag as warnings various misuses of the field, such as:

The left-hand side of an assignment must be an l-value

which is the same error you get when you attempt to assign a value to a constant.

also the use of ref and out parameter modifiers are limited.

By following ReSharpers suggestion you will be warned by the compiler if you attempt to misuse a field that you really intend not to change after initialization.

You aren't changing settings outside of the constructor, the object is the same one in SaveData. The object properties might be changing but not the object reference, so it seems to make sense from a Resharper perspective.

The SaveData() method does not change the settings variable, it changes one its properties. The contents of settings (what it refers to) is only set in the constructor.

Actually, you're right and Resharper is wrong. A field should only be marked readonly if it is immutable in its entirety. In your example if you make it readonly and enable Microsoft's Code Analysis it will warn you that Settings has mutable properties.

This seems to be a bit strange, I can't imagine that Eric Lippert et al, didn't consider the obvious fact that making a reference immutable doesn't make instance pointed to by the reference immutable, though the mentioned Code Analysis rule does support the view of those above (http://msdn.microsoft.com/en-us/library/ms182302(v=VS.100).aspx).

It still doesn't really make any sense. If mutable instances should not be readonly, then, in my view, it should be a compile-time error, seems rather pointless otherwise.

I can see the use of making a reference immutable, rather than the instance it points to.