Skip to content

Instantly share code, notes, and snippets.

@BrianOstrander
Last active October 17, 2020 09:17
Show Gist options
  • Save BrianOstrander/ef793a36a00106db50cf1c57ce656151 to your computer and use it in GitHub Desktop.
Save BrianOstrander/ef793a36a00106db50cf1c57ce656151 to your computer and use it in GitHub Desktop.
How to lose your mind with the clashing design choices of C# 6.0 and Newtonsoft's deserialization

{ get; } = rekt

How to lose your mind with the clashing design choices of C# 6.0 and Newtonsoft's deserialization

Recently, I upgraded a personal Unity3D project to enjoy all the wonderful new C# features that have come out, such as the get only auto property. It looks like this:

public string SomeProperty { get; }

On first glance, it works a lot like C#'s readonly field, with the ability to set a default value, or set its value in a constructor.

public string SomeProperty { get; } = "a default value";

public SomeConstructor(string someConstructorValue)
{
    SomeProperty = someConstructorValue;
}

Why Use Get Only Auto Properties?

  • Interfaces -- Since readonly fields cannot be defined in interfaces, using the get only auto property is a straightforward alternative
  • Overloading -- It simplifies how parent classes handle default values for properties that may be overloaded
  • Aesthetics -- It looks and feels very C#, this is obviously up to personal preference, but I prefer controlling access to my classes data with properties instead of the readonly keyword

Upon reading about the get only auto property, I thought to myself,

That's amazing, I love it! I'll use it everywhere I would normally use the readonly property!

After two days of pain, debugging, and horror, all I can say is... don't be me.

What Went Wrong

I made dangerous assumptions about how the get only auto property worked. I assumed the following lines of code were more or less equivalent,

[JsonProperty] public readonly string SomeReadonlyField;
[JsonProperty] public string SomeGetOnlyProperty { get; }

public SomeConstructor(string someValue)
{
    SomeReadonlyField = someValue;
    SomeGetOnlyProperty = someValue;
}

Newtonsoft can easily serialize and deserialize the readonly property, however, it can only serialize the get only auto property, not deserialize it.

Hacking Newtonsoft

As far as I can tell, and trust me I tried almost everything, there is no 100% reliable way to hack Newtonsoft into deserializing a get only auto property. There are certain cases deserializing a get only auto property may work, but it's hard to remember and predict the exact behavior. This unpredictability stems from how Newtonsoft resolves your object: does it use a [JsonConstructor] decorated constructor? Are there default values assigned to the property? Is the property an object type or a primitive type? And so on...

Creating a new resolver for Newtonsoft won't help either, and the developers of Newtonsoft have stated there is currently no plan to change the behavior of the deserializer. I tried experimenting with auto-implemented property field-targeted attributes, but it seems like Newtonsoft cannot interact with these in a way that allows you to deserialize values on get only auto properties.

At the end of the day, I understand why Newtonsoft's developers have no plans to fix this, there is currently no way to properly support deserialization for get only auto properties.

The Scope of the Problem

At this point I realized I was truly, absolutely, unequivocally, screwed. There was no refactoring feature in any IDE that could save me now. I had made a terrible error and my code base was littered with get only auto properties that may or may not need to be replaced. Consider the full space of the problem with the code snippet below:

public interface ISomeInterface
{
    string SomeProperty { get; }                            // ✅
}

public class SomeClass
{
    public string SomeProperty { get; }                     // ❌

    public string SomeProperty { get; private set; }        // ✅

    [JsonIgnore] public string SomeProperty { get; }        // ✅

    [JsonProperty] public string SomeProperty { get; }      // ❌

    [JsonIgnore]                                            // ✅
    public string SomeProperty { get; }                     // 
    
    [JsonProperty]                                          // ❌
    public string SomeProperty { get; }                     // 

    public string SomeProperty                              // ❌
    {                                                       //
        get;                                                //
    }                                                       //

    public string SomeProperty                              // ✅
    {                                                       //
        get;                                                //
        private set;                                        //
    }                                                       //
}

public abstract class SomeAbstractClass
{
    public abstract string SomeProperty { get; }            // ✅
}

Lines with a ❌ require modifications in order for Newtonsoft to properly deserialize them. A simple find and replace would not work, for the following reasons:

  • Interface or Class -- Writing { get; } in an interface requires no changes, while using it in a class makes it a get only auto property
  • Specifies a Setter -- Assuming you modify your Newtonsoft deserializer to check for private setters, you can ignore lines containing public, protected, or private setters
  • Attributes on Previous Lines -- A check must be done to see if a previous line contains [JsonIgnore]
  • Multiline Properties -- If you are in the habit of breaking your properties across multiple lines, you will have to know a lot more about the context of your property to infer if it requires modifications
  • Abstract -- Again, we can ignore any { get; } properties in abstract classes like we did with interfaces
  • Shared Files -- We can't use the name of the file we're in, or checking to see if the first class we find is abstract, to determine if a file can be ignored. Interfaces, abstract classes, and inheriting classes may share a single file

This list is by no means exhaustive, in fact, I expanded it as I wrote it out and thought of more pitfalls.

The Fix

Despite my best efforts to avoid it, my only remaining option was to write a script to analyze and modify my codebase. The scope of the problem was simply too large to fix by hand, refactoring tools wouldn't cut it, and hacking Newtonsoft's deserializer was futile.

I had to modify this:

public class SomeClass
{
    public string SomeProperty { get; }
}

into this:

public class SomeClass
{
    [JsonProperty] public string SomeProperty { get; private set; }
}

while ignoring these:

public interface ISomeInterface
{
    string SomeProperty { get; }
}

public abstract class SomeAbstractClass
{
    public abstract string SomeProperty { get; }
}

Additionally, any classes that didn't previously include usingNewtonsoft.Json; at the top of the file would now need to include it when decorating properties with the [JsonProperty] attribute.

Implementing the Fix

The steps I took and the code I used to make these fixes is not a one size fits all solution, my script made many assumptions about my coding style and what classes I knew required modifications.

  1. Normalize Your Code -- Rather than handle every possible coding style my script will run across, I normalized the format of any properties requiring serialization in my codebase. This meant making sure attributes, getters, and setters where on the same line, so my script wouldn't have to worry about attributes on previous lines or a setter on a next line. Due to the very consistent way I code -- humble brag -- and my desire to inline anything that I can -- fight me about it -- I only had to make a handful of changes to my codebase for this. It reduced readability, but that can be fixed later as I come across these code snippets again.
  2. Print a Delta -- Making a script to modify my code makes me very uneasy, nauseous even. To ensure it did what I wanted, I did a couple dry runs where I printed a list of changes the script planned to make, without actually modifying the code. While I couldn't read the whole audit log, I could scroll through and identify obvious issues and uncover new pitfalls I hadn't thought of before.
  3. Press and Pray -- Assuming your code is on source control -- it's 2020 after all -- it's time to run your script and hope it works! Since I spent a lot of time auditing the output of my script in the previous step, it worked perfectly when I finally took the training wheels off, successfully modifying 209 lines of code across 43 files.

Conclusion

This write up is not a dismissal of the get only auto property, or an ultimatum that I'll never use it again. In fact, I still really like it, I will just be much more careful when using it in the future. When stated here, the problem seems so obvious, and the fact that get only auto properties don't play nicely with Newtonsoft seems inevitable, but for a lot of C# developers I'm willing to bet they'll make the same mistake I did. Never have I so thoroughly misunderstood a feature of a language or library to the point where I required a script to modify my code -- and I hope this will serve as a warning to those who adopt new and exciting syntax without considering any drawbacks it may have.

For those curious about the script I used to modify my codebase, it is available here. It's not pretty, and references utility libraries not included, but feel free to steal whatever you need from it should you be unfortunate enough to fall into this quagmire.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment