JamesNK / Newtonsoft.Json

Json.NET is a popular high-performance JSON framework for .NET
https://www.newtonsoft.com/json
MIT License
10.81k stars 3.26k forks source link

Deserialization of record types with default values #2765

Open ArthurGrot opened 1 year ago

ArthurGrot commented 1 year ago

Hi I tried deserializing JSON to record types with default values, but the default values are not set by the serializer. With System.Text.Json this is working.

internal class Program
{
    private static void Main(string[] args)
    {
        var newtonsoftJson = Newtonsoft.Json.JsonConvert.DeserializeObject<DefaultRecordTest>("{\"StringOne\":\"test\"}");
        Console.WriteLine(newtonsoftJson);
        // DefaultRecordTest { StringOne = test, StringTwo =  }
        // => StringTwo doesn't get default value

        var csJson = System.Text.Json.JsonSerializer.Deserialize<DefaultRecordTest>("{\"StringOne\":\"test\"}");
        Console.WriteLine(csJson);
        // DefaultRecordTest { StringOne = test, StringTwo = Am I null? }
        // => StringTwo gets default value
    }

    record DefaultRecordTest(string StringOne, string StringTwo = "Am I null?");
}

I'm expecting to get the default value for the record property if it is not provided by the serialized JSON.

elgonzo commented 1 year ago

Ah yes. This is Newtonsoft.Json not taking into account the default value for an optional parameter of the constructor used during deserialization. It's not specifically related to records. And it is a long-standing bug/limitation (see here an issue report from 2016: https://github.com/JamesNK/Newtonsoft.Json/issues/820), that unfortunately so far neither has received much attention nor a fix :(

There is a, uh... "workaround" that involves declaring multiple constructors for the record so as to avoid optional parameters with values that are not the default value of the respective parameter type:

record DefaultRecordTest(string StringOne, string StringTwo)
{
    [Newtonsoft.Json.JsonConstructor]
    public DefaultRecordTest(string StringOne) : this(StringOne, "Am I null?") { }
}

That said, it's something which in my eyes is a rather ugly and janky workaround with respect to records as it muddles the clarity of a record definition. Which is why i personally would try to avoid this, e.g., by choosing to use System.Text.Json instead of Newtonsoft.Json, if you can.