On mutable shared state

Problems with mutable shared state are often discussed in a concurrent programming context. However, those who are lucky enough to avoid concurrent programming can still fall in trouble because of mutable shared state. First pitfall is a global variable. Everyone knows global variables are bad. Although in software design there is no absolute truth or principle, this statement is almost always true. But why are they bad? The full list of reasons can be looked up here, but in a few words – it is because it hinders code understandability and can cause nasty bugs. Understanding of function which uses global variable is never easy because you can not reason about it only by its signature. You can never predict its behavior because you can never be sure what state will global variable have during function’s execution. For the same reason, it is easy to make a mistake during coding.

So, if you don’t use global variables and don’t do concurrent programming, are you safe then? Not really. First, sometimes you can’t avoid global variables. For example, if you program in JavaScript. The problem with JavaScript is that it requires global variables. JavaScript does not have a linker. All compilation units are loaded into a common global object. Second, in case you are doing ordinary object-oriented programming, you can fall in trouble with your objects’ state. Take a close look at this design:

public class Student
{
    public string Name { get; set; }
    public int Age { get; set; }
}

public class AcademicGroup
{
    private readonly List<Student> _students;

    public AcademicGroup(List<Student> students)
    {
        if (HaveUniqueNames(students))
            _students = students;
        else
            throw new ArgumentException(
                "Two or more students with the same names are not allowed!");
    }

    public bool IsValid()
    {
        return HaveUniqueNames(_students);
    }

    private bool HaveUniqueNames(List<Student> students)
    {
        return students
            .Select(s => s.Name)
            .Distinct()
            .Count() == students.Count;
    }
}

AcademicGroup is perfectly encapsulated little class which holds a list of students and for this class to be valid all those students must have unique names. To ensure this, we have validation check in the constructor. Internal data is private (and even read-only!), hence it seems like no one can destroy successfully constructed object of this class. However, look at this usage of the class:

class Program
{
    static void Main()
    {
        var students = new List<Student>
            {
                new Student { Name = "Arnold Schwarzenegger", Age = 18},
                new Student { Name = "Britney Spears", Age = 20},
                new Student { Name = "Barak Obama", Age = 19}
            };

        var academicGroup = new AcademicGroup(students);
        Console.WriteLine(academicGroup.IsValid()); //True
        var anotherBritney = new Student { Name = "Britney Spears", Age = 30 };
        students.Add(anotherBritney);
        Console.WriteLine(academicGroup.IsValid()); //False

        students.Remove(anotherBritney);
        academicGroup = new AcademicGroup(students);
        Console.WriteLine(academicGroup.IsValid()); //True
        students.FirstOrDefault(s => s.Name == "Britney Spears").Name = "Barak Obama";
        Console.WriteLine(academicGroup.IsValid()); //False
    }
}

Nice, is not it? Especially nice to ask students about encapsulation using an example like this one. The internal state of the academic group has been broken after its construction. This line, students.Add(anotherBritney), could have happened anywhere in the system, so that you would search for the reason of having your object in the incorrect state for many hours. And maybe even on the customer side. The same issue arises when we use singleton pattern to simulate global variable.

The problem can be formulated as a combination of 3 conditions: 1) there is shared state between two modules (between Program and AcademicGroup there is the list of students); 2) there is a change of the shared state by one module without notifying another module which have references to this shared state (after creating object of AcademicGroup, Program changes list of students and does not notify about this action newly created object); 3) one of the modules which shares the state has assumptions about this state i.e. correctness of this module depends on the shared state (AcademicGroup assumes list of students to have unique references).

To avoid this problem, one must do one of the following 1) Make shared state immutable; 2) Notify all others about the change of the shared state which they depend on; 3) All modules should avoid making any assumptions about the shared state.

In my example, I can’t avoid making assumptions about incoming to AcademicGroup data. Therefore, I can try to do notification by adding another method to the AcademicGroup:

public class AcademicGroup
{
...
    public void NotifyAboutStudentsChange()
    {
        if (!IsValid())
            throw new
                InvalidConstraintException(
                   "Two or more students with the same names are not allowed!");
    }
....
}

And then notifying all objects of AcademicGroup like this:

class Program
{
    static void Main()
    {
        var students = new List<Student>
            {
                new Student { Name = "Arnold Schwarzenegger", Age = 18},
                new Student { Name = "Britney Spears", Age = 20},
                new Student { Name = "Barak Obama", Age = 19}
            };

        var academicGroup = new AcademicGroup(students);
        Console.WriteLine(academicGroup.IsValid()); //True
        var anotherBritney = new Student { Name = "Britney Spears", Age = 30 };
        students.Add(anotherBritney);
        academicGroup.NotifyAboutStudentsChange();  //Trows
        Console.WriteLine(academicGroup.IsValid()); //This line never executes

        students.Remove(anotherBritney);
        academicGroup = new AcademicGroup(students);
        Console.WriteLine(academicGroup.IsValid()); //True
        students.FirstOrDefault(s => s.Name == "Britney Spears").Name = "Barak Obama";
        academicGroup.NotifyAboutStudentsChange(); //Trows
        Console.WriteLine(academicGroup.IsValid()); //This line never executes
    }
}

Nasty mess, is not it? But without it, we have a design flaw which makes our code hard to understand and buggy. But don’t get me wrong, I am not suggesting creating stuff like this in your designs. No, although it is a solution if you can’t avoid assumptions about shared state and you can’t make shared state immutable. To my mind the best solution here is going functional way, even though in some cases it may be less efficient. Here is what well-designed code would look like (thanks to heaven we now have immutable collections in .NET 4.5):

class Program
{
    static void Main()
    {
        var students = new List<Student>
            {
                new Student(name: "Arnold Schwarzenegger", age: 18),
                new Student(name: "Britney Spears", age: 20),
                new Student(name: "Barak Obama", age: 19)
            };

        var academicGroup = new AcademicGroup(students);
        Console.WriteLine(academicGroup.IsValid()); //True
        var anotherBritney = new Student(name: "Britney Spears", age: 30);
        students.Add(anotherBritney);
        Console.WriteLine(academicGroup.IsValid()); //True

        students.Remove(anotherBritney);
        academicGroup = new AcademicGroup(students);
        Console.WriteLine(academicGroup.IsValid()); //True
        
        //This does not even compile anymore: 
        //students.FirstOrDefault(s => s.Name == "Britney Spears").Name = "Barak Obama";
        
        var student = students.FirstOrDefault(s => s.Name == "Britney Spears");
        students[students.IndexOf(student)] =
            new Student(name: "Barak Obama", age: students[students.IndexOf(student)].Age);
        Console.WriteLine(academicGroup.IsValid()); //True
    }
}
public class Student
{
    //This can be done only in C# 6
    //In C# 5 you have to create readonly backing fields
    public string Name { get; }
    public int Age { get; }

    public Student(string name, int age)
    {
        Name = name;
        Age = age;
    }
}
public class AcademicGroup
{
    private readonly ImmutableList<Student> _students;

    public AcademicGroup(IEnumerable<Student> students)
    {
        _students = students.ToImmutableList();
        if (!HaveUniqueNames(_students))
            throw new ArgumentException(
                "Two or more students with the same names are not allowed!");
    }

    public bool IsValid()
    {
        return HaveUniqueNames(_students);
    }

    private bool HaveUniqueNames(IReadOnlyCollection<Student> students)
    {
        return students
            .Select(s => s.Name)
            .Distinct()
            .Count() == students.Count;
    }
}

To conclude, I would say I start more often thinking that I am not a fan of classical object-oriented programming anymore. Some functional elements, like immutable state, seem to be a really powerful tool to simplify designs and make us less likely to write a bug.

Facebooktwittergoogle_pluslinkedinmailFacebooktwittergoogle_pluslinkedinmail

Leave a Reply

Your email address will not be published.