Thoughts on records, preserving invariants and verbosity

[castholm]

I've been acquainting myself with records over the past few days and while I was originally excited for the feature, hoping that I would be able to trim away thousands of lines of code simply by rewriting our immutable types using the record syntax, I quickly came to the realization that asserting and preserving invariants using the record syntax still requires almost as much boilerplate code as using the regular class syntax.

---

Let's say we want to define an immutable type representing some sort of calendar event with a name, a start date/time and a duration. We can do so using the positional record syntax.

public sealed record CalendarEvent(string Name, DateTime Start, DateTime Duration);

However, for various good reasons, there are a number of invariants we want to assert and preserve at all times, such as the name never being null, the start having a DateTimeKind of Utc and the duration not being negative.

To accomplish this, we need to define our own properties instead of using the auto-generated ones.

public sealed record CalendarEvent(string Name, DateTime Start, TimeSpan Duration)
{
    public string Name { get; }
        = Name ?? throw new ArgumentNullException(nameof(Name));

    public DateTime Start { get; }
        = Start.Kind == DateTimeKind.Utc
        ? Start
        : throw new ArgumentException("Start must be UTC.", nameof(Start));

    public TimeSpan Duration { get; }
        = Duration >= TimeSpan.Zero
        ? Duration
        : throw new ArgumentException("Duration must not be negative.", nameof(Duration));
}

A bit more verbose, but that should do, right?

var calendarEvent = new CalendarEvent(
    "Meeting",
    new DateTime(2020, 01, 01, 12, 00, 00, DateTimeKind.Utc),
    TimeSpan.FromMinutes(60));

var rescheduledCalendarEvent = calendarEvent with { Start = calendarEvent.Start.AddHours(1) };
//                                                  ~~~~~
// CS0200: Property or indexer 'CalendarEvent.Start' cannot be assigned to -- it is read only

Oh, right, we can't use with expressions if the property is not publicly settable. Let's fix that by adding init accessors.

public sealed record CalendarEvent(string Name, DateTime Start, TimeSpan Duration)
{
    public string Name { get; init; }
        = Name ?? throw new ArgumentNullException(nameof(Name));

    public DateTime Start { get; init; }
        = Start.Kind == DateTimeKind.Utc
        ? Start
        : throw new ArgumentException("Start must be UTC.", nameof(Start));

    public TimeSpan Duration { get; init; }
        = Duration >= TimeSpan.Zero
        ? Duration
        : throw new ArgumentException("Duration must not be negative.", nameof(Duration));
}

var calendarEvent = new CalendarEvent(
    "Meeting",
    new DateTime(2020, 01, 01, 12, 00, 00, DateTimeKind.Utc),
    TimeSpan.FromMinutes(60));

var rescheduledCalendarEvent = calendarEvent with { Start = calendarEvent.Start.AddHours(1) };

var calendarEventWithInvalidState = calendarEvent with { Duration = TimeSpan.FromMinutes(-60) };

Debug.Assert(calendarEventWithInvalidState.Duration >= TimeSpan.Zero);

Huh, the assertion on the last line failed. But if we think about it, that makes sense, since the validation that happens when instantiating the type is completely separate from code that uses the init accessors.

Here's where things start to get annoying.

To add validation to the init accessors, we need to do away with auto-properties and define them all manually using backing fields.

public sealed record CalendarEvent(string Name, DateTime Start, TimeSpan Duration)
{
    private readonly string _name;
    private readonly DateTime _start;
    private readonly TimeSpan _duration;

    public string Name
    {
        get => _name;
        init => _name = value ?? throw new ArgumentNullException(nameof(Name));
    }
        = Name;

    public DateTime Start
    {
        get => _start;
        init => _start
            = value.Kind == DateTimeKind.Utc
            ? Start
            : throw new ArgumentException("Start must be UTC.", nameof(Start));
    }
        = Start;

    public TimeSpan Duration
    {
        get => _duration;
        init => _duration
            = Duration >= TimeSpan.Zero
            ? Duration
            : throw new ArgumentException("Duration must not be negative.", nameof(Duration));
    }
        = Duration;
}

But this is actually not legal and won't compile, because only auto-properties can have initializers.

What if we instead change it to initialize the fields directly?

public sealed record CalendarEvent(string Name, DateTime Start, TimeSpan Duration)
{
    private readonly string _name = Name;
    private readonly DateTime _start = Start;
    private readonly TimeSpan _duration = Duration;

    public string Name
    {
        get => _name;
        init => _name = value ?? throw new ArgumentNullException(nameof(Name));
    }

    public DateTime Start
    {
        get => _start;
        init => _start
            = value.Kind == DateTimeKind.Utc
            ? value
            : throw new ArgumentException("Start must be UTC.", nameof(Start));
    }

    public TimeSpan Duration
    {
        get => _duration;
        init => _duration
            = value >= TimeSpan.Zero
            ? value
            : throw new ArgumentException("Duration must not be negative.", nameof(Duration));
    }
}

Now we have the opposite problem of when we didn't validate the init accessor -- the (synthesized) constructor performs no validation!

Unless I'm missing something, this leaves us with three choices:

Solution 1: Duplicate validation

public sealed record CalendarEvent(string Name, DateTime Start, TimeSpan Duration)
{
    private readonly string _name = Name ?? throw new ArgumentNullException(nameof(Name));

    private readonly DateTime _start
        = Start.Kind == DateTimeKind.Utc
        ? Start
        : throw new ArgumentException("Start must be UTC.", nameof(Start));

    private readonly TimeSpan _duration
        = Duration >= TimeSpan.Zero
        ? Duration
        : throw new ArgumentException("Duration must not be negative.", nameof(Duration));

    public string Name
    {
        get => _name;
        init => _name = value ?? throw new ArgumentNullException(nameof(Name));
    }

    public DateTime Start
    {
        get => _start;
        init => _start
            = value.Kind == DateTimeKind.Utc
            ? value
            : throw new ArgumentException("Start must be UTC.", nameof(Start));
    }

    public TimeSpan Duration
    {
        get => _duration;
        init => _duration
            = value >= TimeSpan.Zero
            ? value
            : throw new ArgumentException("Duration must not be negative.", nameof(Duration));
    }
}

Sure, you could reduce some code duplication by moving the actual validation behind some AssertUtcDateTimeKind helper, but at the end of the day you will still have two separate code paths that you need to be careful to keep in sync with each other.

Solution 2: Don't use the positional syntax

public sealed record CalendarEvent
{
    private readonly string _name;
    private readonly DateTime _start;
    private readonly TimeSpan _duration;

    public CalendarEvent(string name, DateTime start, TimeSpan duration)
    {
        Name = name;
        Start = start;
        Duration = duration;
    }

    public string Name
    {
        get => _name;
        init => _name = value ?? throw new ArgumentNullException(nameof(Name));
    }

    public DateTime Start
    {
        get => _start;
        init => _start
            = value.Kind == DateTimeKind.Utc
            ? value
            : throw new ArgumentException("Start must be UTC.", nameof(Start));
    }

    public TimeSpan Duration
    {
        get => _duration;
        init => _duration
            = value >= TimeSpan.Zero
            ? value
            : throw new ArgumentException("Duration must not be negative.", nameof(Duration));
    }
}

(This currently emits warning CS8618 because the nullable analyzer doesn't track that _name is set via Name, but that's a separate issue and the warning can safely be suppressed.)

This has less code duplication, but results in more boilerplate, especially if we also want to use Deconstruct, which we will now have to implement ourselves.

Solution 3: Don't define init accessors

This leaves us with our second iteration of the class and is the least verbose of the three. Depending on your use case, this can be perfectly fine, but on the other hand, it feels a bit sad to miss out on a potentially useful new language feature.

If you only have some properties that need to be validated, this can also lead to an inconsistent design where all of the synthesized properties have init accessors but not the ones you needed to manually override.

---

I'm not sure if developer convenience and reducing verbosity was an explicit goal of records or if it was designed for value semantics first and foremost, so maybe I was just expecting too much, but all of these inconveniences is definitely going to affect how much use records are going to see in our code bases.

For completely stateless and validation-less data types with value semantics, the record syntax is a great addition to the language that does away with a lot of boilerplate code. But as soon as you introduce any sort of invariants, you're back to writing almost as much property boilerplate code as you did in previous versions of the language.

One way to remedy this would be to allow initializing not just auto-properties but also manually implemented properties. Couple this with semi-auto-properties/the field keyword (#140) and you don't even need to define backing fields!

I'm curious to hear how others feel about records and validation now that they are out in the wild and you've had some chances to use them for real.

[castholm]

#3707 also mentions "final initializers" which I believe could become a convenient and concise way of ensuring the object is in a valid state after any form of initialization.

public sealed record CalendarEvent(string Name, DateTime Start, DateTime Duration)
{
    init
    {
        if (Name is null)
            throw new ArgumentNullException(nameof(Name));
        if (Start.Kind != DateTimeKind.Utc)
            throw new ArgumentException("Start must be UTC.", nameof(Start));
        if (Duration < TimeSpan.Zero)
            throw new ArgumentException("Duration must not be negative.", nameof(Duration));
    }
}