Unexpected inconsistency in records

Unexpected inconsistency in records The other day, I was trying to figure out a bug in my code, and it turned out to be a misunderstanding on my part as to how C# records work. It’s entirely possible that I’m the only one who expected them to work in the way that I did, but I figured it was worth writing about in case. As it happens, this is something I discovered when making a change to my 2029 UK general election site, but it isn’t actually related to the election, so I haven’t included it in the election site blog series. Recap: nondestructive mutation When records were introduced into C#, the “nondestructive mutation” with operator was introduced at the same time. The idea is that record types can be immutable, but you can easily and efficiently create a new instance which has the same data as an existing instance, but with some different property values. For example, suppose you were to have a record like this: public sealed record HighScoreEntry(string PlayerName, int Score, int Level); You could then have code of: HighScoreEntry entry = new("Jon", 5000, 50); var updatedEntry = entry with { Score = 6000, Level = 55 }; This doesn’t change the data in the first instance (so entry.Score would still be 5000). Recap derived data Records don’t allow you to specify constructor bodies for the primary constructor (something I meant to write about in my earlier post about records and collections, but you can initialize fields (and therefore auto-implemented properties) based on the values for the parameters in the primary constructor. So as a very simple (and highly contrived) example, you could create a record which determines whether or not a value is odd or even on initialization: public sealed record Number(int Value) { public bool Even { get; } = (Value & 1) == 0; } At first glance, this looks fine: var n2 = new Number(2); var n3 = new Number(3); Console.WriteLine(n2); // Output: Number { Value = 2, Even = True } Console.WriteLine(n3); // Output: Number { Value = 3, Even = False } So far, so good. Until this week, I’d thought that was all fine. Oops: mixing with and derived data The problem comes when mixing these two features. If we change the code above (while leaving the record itself the same) to create the second Number using the with operator instead of by calling the constructor, the output becomes incorrect: var n2 = new Number(2); var n3 = n2 with { Value = 3 }; Console.WriteLine(n2); // Output: Number { Value = 2, Even = True } Console.WriteLine(n3); // Output: Number { Value = 3, Even = True } “Value = 3, Even = True” is really not good. How does this happen? Well, for some reason I’d always assumed that the with operator called the constructor with the new values. That’s not actually what happens. The with operator above translates into code roughly like this: // This won't compile, but it's roughly what is generated. var n3 = n2.$(); n3.Value = 3; The $ method (at least in this case) calls a generated copy constructor ( Number(Number) ) which copies both Value and the backing field for Even . This is all documented – but currently without any warning about the possible inconsistency it can introduce. (I’ll be emailing Microsoft folks to see if we can get something in there.) Note that because Value is set after the cloning operation, we couldn’t write a copy constructor to do the right thing here anyway. (At least, not in any sort of straightforward way – I’ll mention a convoluted approach later.) In case anyone is thinking “why not just use a computed property?” obviously this works fine: public sealed record Number(int Value) { public bool Even => (Value & 1) == 0; } Any property that can easily be computed on demand like this is great – as well as not exhibiting the problem from this post, it’s more efficient in memory too. But that really wouldn’t work for a lot of the properties in the records I use in the election site, where often the record is constructed with collections which are then indexed by ID, or other relatively expensive computations are performed. What can we do? So far, I’ve thought of four ways forward, none of them pleasant. I’d be very interested to hear recommendations from others. Option 1: Shrug and get on with life Now I know about this, I can avoid using the with operator for anything but “simple” records. If there are no computed properties or fields, the with operator is still really useful. There’s a risk that I might use the with operator on a record type which is initially “simple” and then later introduce a computed member, of course. Hmm. Option 2: Write a Roslyn analyzer to detect the problem In theory, at least for any records being used within the same solution in which they’re declared (which is everything for my election site) it should be feasible to write a Roslyn analyzer which: Analyzes every member initializer in every declared record to see which parameters are used Analyzes every with operator usage to see which parameters are being set operator usage to see which parameters are being set Records an error if there’s any intersection between the two That’s quite appealing and potentially useful to others. It does have the disadvantage of having to implement the Roslyn analyzer though. It’s been a long time since I’ve written an analyzer, but my guess is that it’s still a fairly involved process. If I actually find the time, this is probably what I’ll do – but I’m hoping that someone comments that either the analyzer already exists, or explains why it isn’t needed anyway. Option 3: Figure out a way of using with safely I’ve been trying to work out how to potentially use Lazy to defer computing any properties until they’re first used, which would come after the with operator set new values for properties. I’ve come up with the pattern below – which I think works, but is ever so messy. Adopting this pattern wouldn’t require every new parameter in the parent record to be reflected in the nested type – only for parameters used in computed properties. public sealed record Number(int Value) { private readonly Lazy computed = new(() => new(Value), LazyThreadSafetyMode.ExecutionAndPublication); public bool Even => computed.Value.Even; private Number(Number other) { Value = other.Value; // Defer creating the ComputedMembers instance until computed = new(() => new(this), LazyThreadSafetyMode.ExecutionAndPublication); } // This is a struct (or could be a class) rather than a record, // to avoid creating a field for Value. We only need the computed properties. // (We don't even really need to use a primary // constructor, and in some cases it might be best not to.) private struct ComputedMembers(int Value) { internal ComputedMembers(Number parent) : this(parent.Value) { } public bool Even { get; } = (Value & 1) == 0; } } This is: Painful to remember to do A lot of extra code to start with (although after it’s been set up, adding a new computed member isn’t too bad) Inefficient in terms of memory, due to adding a Lazy instance The inefficiency is likely to be irrelevant in “large” records, but it makes it painful to use computed properties in “small” records with only a couple of parameters, particularly if those are just numbers etc. Option 4: Request a change to the language I bring this up only for completeness. I place a lot of trust in the C# design team: they’re smart folks who think things through very carefully. I would be shocked to discover that I’m the first person to raise this “problem”. I think it’s much more likely that the pros and cons of this behaviour have been discussed at length, and alternatives discussed and prototyped, before landing on the current behaviour as the least-worst option. Now maybe the Roslyn compiler could start raising warnings (option 2) so that I don’t have to write an analyzer – and maybe there are alternatives that could be added to C# for later versions (ideally giving more flexibility for initialization within records in general, e.g. a specially named member that is invoked when the instance is “ready” and which can still write to read-only properties)… but I’m probably not going to start creating a proposal for that without explicit encouragement to do so. Conclusion It’s very rare that I discover a footgun in C#, but this really feels like one to me. Maybe it’s only because I’ve used computed properties so extensively in my election site – maybe records really aren’t designed to be used like this, and half of my record types should really be classes instead. I don’t want to stop using records, and I’m definitely not encouraging anyone else to do so either. I don’t want to stop using the with operator, and again I’m not encouraging anyone else to do so. I hope this post will serve as a bit of a wake-up call to anyone who is using with in an unsound way though. Oh, and of course if I do write a Roslyn analyzer capable of detecting this, I’ll edit this post to link to it.