loading...

Avoiding mutable classes in C#

shimmer profile image Brian Berns ・4 min read

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:

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:

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

// output:
// 90, 60

Globetrotting

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

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

We can try it out like this:

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:

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:

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:

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:

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:

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:

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 on class invariants. 

Discussion

pic
Editor guide
Collapse
ferdeen profile image
Ferdeen Mughal

Named tuples would be great here :

private static (Degrees, Degrees) Normalize(Degrees latitude, Degrees longitude)

Nice article by the way!