Breaking encapsulation with collections

Encapsulation is one of the most important features of object orientation, but often easy to break in practice. One common mistake to make in this respect happens when creating a class that holds some sort of collection or array.

For example, let’s assume that we want to make an immutable object (meaning that its state should never change troughout its lifecycle):

public class Request
{
    private readonly string[] _acceptedTypes;
    public Request(params string [] acceptedTypes) {...}
    public string[] AcceptedTypes { get {...} }
    public bool Accepts(string type) {...}
}

We have a class that take a list of strings as parameter to the constructor. The intent is that the list of strings should never change. Typical use of the class would be something like this:

Request request = new Request("application/json", "application/x-json");
if (request.Accepts(somestring))
{
    ...
}

Here’s a naîve implementation of the class:

public class Request
{
    private readonly string[] _acceptedTypes;
    public Request(params string[] acceptedTypes)  
    {  
        _acceptedTypes = acceptedTypes;  
    }

    public string[] AcceptedTypes { get { return _acceptedTypes; } }

    public bool Accepts(string type)  
    {  
        foreach (string acceptableType in _acceptedTypes)  
        {
            if (acceptableType.Equals(type))  
            {  
                return true;  
            }  
        }  
        return false;  
    }  
}

Our intent of creating an immutable object is here manifested in the readonly keyword used when defining the member variable _acceptedTypes, and the fact that there is only a set accessor for the AcceptedTypes property. But alas, there are several issues that break our intent.

First, let’s have a look at the AcceptedTypes accessor. It allows us to write code such as this:

request = new Request("application/json", "application/x-json" );
bool accepts1 = request2.Accepts("application/javascript");  
request.AcceptedTypes[1] = "application/javascript";  
bool accepts2 = request.Accepts("application/javascript");

After running this code, we find that accepts1 != accepts2. We have been able to manipulate the state of the object (in other words, it is mutable). The problem is that the AcceptedTypes exposes a reference to the array of types. Alternatively, it could only expose an IEnumerable that can be used to iterate over the types:

public IEnumerable<string> AcceptedTypes
{
    get
    {
        foreach (string acceptableType in _acceptedTypes)
        {
            yield return acceptableType;
        }
    }
}

Then, it will not be possible to manipulate the state of the object by calling request.AcceptedTypes[i].
Still, there is one problem with the current implementation. We still can write code such as this:

string[] types = new string[] { "application/json", "application/x-json" };
Request request = new Request(types);
bool accepts1 = request.Accepts("application/javascript");
types[1] = "application/javascript";
bool accepts2 = request.Accepts("application/javascript");

After running this code, accepts1 != accepts2. The problem is that we pass a referene to the constructor, and the object instance stores it in its local variable. We are free to change the object that our reference points to, indirectly changing the state of the object. In order to fix this, we change the constructor code for Request:

public Request(params string[] acceptedTypes)
{
    _acceptedTypes = new string[acceptedTypes.Length];
    Array.Copy(acceptedTypes, _acceptedTypes, acceptedTypes.Length);
}