Avoiding mutable classes in C#
Tags: csharp, dotnet, functional
Published: 2020-02-17 03:45:22

# GeoPoint

Consider a class that represents a point on the surface of the earth, with the following invariants:[^1]

* Latitude is always between -90° and 90°.
* Longitude is always between -180° and 180°.

Let's implement this class, focusing on construction first:

```csharp
using Degrees = System.Double;

class GeoPoint
{
    public GeoPoint(Degrees latitude, Degrees longitude)
    {
        _latitude = latitude;
        _longitude = longitude;
        Normalize();
    }
    private Degrees _latitude;
    private Degrees _longitude;

    private void Normalize()
    {
        _latitude = Math.Max(Math.Min(_latitude, 90), -90);
        while (_longitude <= -180) _longitude += 360;   // inefficient, but clear
        while (_longitude > 180) _longitude -= 360;     // inefficient, but clear

        Debug.Assert(_latitude >= -90 && _latitude <= 90);
        Debug.Assert(_longitude > -180 && _longitude <= 180);
    }

    public override string ToString()
        => string.Format($"{_latitude}, {_longitude}");
}
```

The main thing to notice here is the `Normalize` method, which enforces the class invariants. If we try to create a geopoint with non-standard coordinates, normalization will bring them in bounds:

```csharp
var pt = new GeoPoint(100, -300);
Console.WriteLine(pt);

// output:
// 90, 60
```

# Globetrotting

Next, let's add a way to move a geopoint:

```csharp
public void Move(Degrees latitudeChange, Degrees longitudeChange)
{
    _latitude += latitudeChange;
    _longitude += longitudeChange;
    Normalize();
}
```

We can try it out like this:

```csharp
var pt = new GeoPoint(0, 0);
for (var i = 0; i < 10; ++i)
{
    pt.Move(5, 30);
    Console.WriteLine(pt);
}

// output:
// 5, 30
// 10, 60
// 15, 90
// 20, 120
// 25, 150
// 30, 180
// 35, -150
// 40, -120
// 45, -90
// 50, -60
```

What do you think of this class so far? Is it ready for use, or does it need more work?

# Immutability

One issue that jumps out at me is that `Move` mutates the object's fields. This is a potential problem because two different threads attempting to move the same point simultaneously could corrupt it - one thread could read the object at the same time the other is in the middle of updating it. That's a bug waiting to happen. We could protect against this with careful use of `lock` statements, but that's tricky and could even become a performance bottleneck.

Fortunately, there is a better way: immutability. The first thing we have to do is make our class's fields read-only:

```csharp
private readonly Degrees _latitude;
private readonly Degrees _longitude;
```

With this change in place, we can be certain that the class is thread-safe, because race conditions are no longer possible. I'd go as far as to say that any time you declare a variable in C#, it's good practice to at least consider making it `readonly`. You can save yourself a lot of grief that way.

We now have to revise the class's implementation, since both `Move` and `Normalize` are mutating. Can we turn them into pure functions instead? To start with, let's change `Normalize` into a static function that takes a pair of arbitrary coordinates as input and returns normalized coordinates as output:

```csharp
private static (Degrees, Degrees) Normalize(Degrees latitude, Degrees longitude)
{
    latitude = Math.Max(Math.Min(latitude, 90), -90);
    while (longitude <= -180) longitude += 360;   // inefficient, but clear
    while (longitude > 180) longitude -= 360;     // inefficient, but clear

    Debug.Assert(latitude >= -90 && latitude <= 90);
    Debug.Assert(longitude > -180 && longitude <= 180);

    return (latitude, longitude);
}
```

Even though `Normalize` continues to mutate variables internally, there are no side-effects visible to the caller, so it is now a pure function. (Exercise for the reader: Rewrite the function body to eliminate internal mutation as well.)

Initialization in the constructor can then be simplified to a single line:

```csharp
public GeoPoint(Degrees latitude, Degrees longitude)
{
    (_latitude, _longitude) = Normalize(latitude, longitude);
}
```

Next, we have to change the semantics of moving a geopoint. Instead of modifying the point's coordinates in place, we create and return a separate point that represents the new location:

```csharp
public GeoPoint Move(Degrees latitudeChange, Degrees longitudeChange)
    => new GeoPoint(_latitude + latitudeChange, _longitude + longitudeChange);
```

Both geopoints remain valid, as "before" and "after" snapshots. This can actually be a very helpful way to maintain history.

Our test code now looks like this:

```csharp
var pt = new GeoPoint(0, 0);
for (var i = 0; i < 10; ++i)
{
    pt = pt.Move(5, 30);
    Console.WriteLine(pt);
}
```

The output is the same as before.

# Wrap up

By making the class immutable, we've not just eliminated a possible bug - we've also made the class easier to reason about, because we no longer have to worry about how the state of an object might change over time. Instead, a geopoint now represents a single fixed value, much like a .NET string or an integer does. Immutability and value semantics are key principles of functional programming.

But wait. This actually raises a second problem, which is that the class doesn't quite behave the way we expect when we compare two values:

```csharp
var a = new GeoPoint(0, 0);
var b = new GeoPoint(0, 0);
Console.WriteLine(Object.Equals(a, b));

// output:
// False
```

Fixing this isn't too hard, but can be a bit subtle. Maybe someone will take a shot at it in the comments!

[^1]: This example is adapted from the [Wikipedia page](https://en.wikipedia.org/wiki/Class_invariant) on class invariants.