NRE DiscriminatedUnionConverter

[birojnayak]

Source/destination types

string type (invalid input)

Source/destination JSON

"Hello Admin"

Expected behavior

Valid exception message if input is invalid

Actual behavior

Got NRE due to FSharpUtils.Instance was null. Probably EnsureInitialized is not invoked ??

Steps to reproduce

public class RandoopTest5379
{
    public static void Main()
    {
        try
        {
            Newtonsoft.Json.Converters.DiscriminatedUnionConverter discriminatedUnionConverter =
                new Newtonsoft.Json.Converters.DiscriminatedUnionConverter();
            Newtonsoft.Json.Linq.JTokenWriter jTokenWriter = new Newtonsoft.Json.Linq.JTokenWriter();
            String tempStr = "Hello Admin";
            Newtonsoft.Json.JsonSerializer jsonSerializer = Newtonsoft.Json.JsonSerializer.CreateDefault();
            discriminatedUnionConverter.WriteJson(jTokenWriter, tempStr, jsonSerializer);
        }
        catch (Exception ex)
        {
            System.Console.WriteLine(ex.ToString());
        }
    }
}

[elgonzo]

That's not a bug in the DiscriminatedUnionConverter but in your own code. Your code is not checking whether a JsonConverter instance (such as discriminatedUnionConverter, for example) is suitable for converting a given instance such as a string.

Note that all JsonConverter's have a CanConvert method that indicates whether the JsonConverter can be used for converting instances of a given type.

So, basically, a more correct code with respect to your example could have been:

if (discriminatedUnionConverter.CanConvert(tempStr.GetType()))
{
    discriminatedUnionConverter.WriteJson(jTokenWriter, tempStr, jsonSerializer);
}
else
{
   ... write the given instance to the jTokenWriter in a different way not using discriminatedUnionConverter ...
}

That said, it's debatable whether the NRE response needs to be replaced with a different exception message. JsonConverters are supposed to be used by the serializer (which then handles JsonConverters correctly). The JsonConverter APIs are not meant to be called directly by user code, and therefore typically don't feature the safeguards one would expect from an API that is supposed to be called from user code. (But then again, the JsonConverter APIs are public instead of protected internal, so contrary to my opinion it is perhaps one of the intended use case to be called directly from user code...? Only the author of the library can clarify...)

[birojnayak]

@elgonzo you are correct... but as you said either it shouldn't be public or if it is, it should handle error conditions/edge cases for better code quality(just my thought). That's the reason I have created.