Why does resharper suggest readonly fields

Question

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 change it also in another method in the same class. What am I missing?

public partial class OptionsForm : Form
{
 private Settings settings;

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

 private void SaveData()
 {
  settings.ProjectName = TextBoxProject.Text;
  // serialize settings to settings.xml
 }
}

Answer 1

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.

Answer 2

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.

Answer 3

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

Answer 4

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.)

Answer 5

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.