Private readonly Interface - is it redundant?

I am using IoC and DI for my project.

However I was wondering if it is good pr开发者_运维知识库actise to have the following:

private readonly IMyService myservice;

as the field inside the class that is a consumer of the service. The field is set in the constructor.

I'm sure I've seen this somewhere and I've picked up on it. However I also see:

private IMyService myservice;

and it seems to suffice. Is there any purpose to have a readonly field for the injected service interface? What are the advantages?

The fact that it's an interface is irrelevant. Applying the readonly modifier on a field prevents you (or someone else) to change its value after the object is constructed. It can only be assigned in the constructor.

I consider use of the readonly keyword a central part of proper implementation of Constructor Injection.

public class MyClass
{
    private readonly IMyService myservice;

    public MyClass(IMyService myservice)
    {
        if (myservice == null)
        {
            throw new ArgumentNullException("myservice");
        }
        this.myservice = myservice;
    }
}

Neither the readonly keyword nor the Guard Clause are technically required to implement Constructor Injection. However, they both help strengthen the invariants of the class. This is what encapsulation is all about.

A readonly field means it can only be written in the ctor. Once that's completed, the reference can't be changed or destroyed. It's very useful for initializing state and enforcing immutability.

The advantage of having readonly on the field is it's a clear declaration that the field will not change during the lifetime of the containing instance. In many scenarios this makes it easier to reason about the behavior of a given method. For example

void Method() {
  var marker = myservice.StartOperation();
  try {
    SomeOtherMethod();
  } finally {
    myservice.StopOperation(marker);
  }
}

Assume that StartOperation and StopOperation are methods which must be called in pairs on a given IMyService instance. When myservice is a readonly field you can look only at this function and have a high degree of confidence in meeting this contract.

However if it's not readonly you must be immediately suspcious of SomeOtherMethod and every other method transitively called from that function. If any of them could reset the myservice field suddenly you would be in violation of the contract and end up with some very subtle bugs.

Here is the documentation for the readonly keyword.

When applied to a field in a class, readonly indicates to the reader "this field will not change for the lifetime of this instance." That is highly useful information for dependencies, which are not intended to change after receiving them in the constructor.

A misguided attempt to alter a dependency results in a compile-time error, reminding you or whomever else is modifying the class that injected dependencies should not change. This is a much easier situation to detect and fix than omitting the readonly keyword and later having to track down a bug because of a reassignment.

In short, yes, it is good practice to declare something readonly if you do not change it after the object is constructed, as it will keep all future authors from making that mistake.