2
\$\begingroup\$

I have the input JSON

{"QtyEqualsQtyWorked":0,"ReasonCode":"IN"}

and my class is

public class LoadForMovementOpt
{
    public bool QtyEqualsQtyWorked { get; set; }
    public string[] ReasonCode { get; set; }
}

The property ReasonCode has to be array for some reasons I cannot argue with, but in json it can be both string and string[]. So I wrote a custom converter that actually works, but maybe it can improved:

class LoadForMovementConverter : JsonConverter
{
    public override bool CanConvert(Type objectType) { return true; }
    public override bool CanWrite
    {
        get
        {
            return false;
        }
    }
    public override bool CanRead
    {
        get
        {
            return true;
        }
    }

    public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) { }

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
    {
        JToken token = JToken.Load(reader);
        var reasonToken = token["ReasonCode"];

        if(reasonToken == null)
        {
            return token.ToObject(objectType);
        }

        if(reasonToken.Type == JTokenType.Array)
        {
            return token.ToObject(objectType);
        }
        else
        {
            var QtyEqualsQtyWorkedToken = token["QtyEqualsQtyWorked"];
            return 
            new LoadForMovementOpt 
            {
                QtyEqualsQtyWorked = 
                    QtyEqualsQtyWorkedToken
                        .ToObject<bool>(), 
                ReasonCode = 
                    new string[] 
                    {
                        reasonToken.ToObject<string>() 
                    }
            };
        }
    }
}

Is there any way to improve it?

\$\endgroup\$
  • 1
    \$\begingroup\$ @t3chb0t Yeah sorry, I edited the question. This was a purely test code so it's messy \$\endgroup\$ – Phate01 Feb 7 '18 at 12:54
  • \$\begingroup\$ ok, now I have a technical question, does this mean reasonToken.Type == JTokenType.Array that you can also have a json that contains an array, this is, both cases can occur? \$\endgroup\$ – t3chb0t Feb 7 '18 at 12:56
  • \$\begingroup\$ Clearly I forgot to say, I'm sorry. Editing.. \$\endgroup\$ – Phate01 Feb 7 '18 at 14:01
  • \$\begingroup\$ I'm not sure if that is good idea to convert all object when you need custom serializationg for one property. See stackoverflow.com/questions/18521970/… for details \$\endgroup\$ – Viktor Lova Feb 7 '18 at 19:11
1
\$\begingroup\$

Do you have access the JsonReader and JToken classes? If so, consider the following APIs. There is a lot of logic tied into the current ReadJson method which could be (IMO) better separated out of there. You would of course have to create/implement/override the appropriate base classes and methods: Notice how a lot of the clutter is separated and moved out to a factory with the following APIs? And the respective JTokens will know exactly what to do when you send the ToObject method to it - there is no need to check for the type, or to check if it's an array because the factory will take care of that logic. The only negative is that it might be a little annoying if you have to roll your own JsonReader implementation.

// A so-so API that is marginally better
public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
{
    JToken token = JToken.LoadMovementConverter(reader);
    token.ToObject();        
}


    // even more ideal API:
public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
{
    JToken token = reader.GetMovementConverterJToken();
    token.ToObject();        
}

    // create new JToken types:
public class JTokenNullReason : JToken
{   
    public override object ToObject()
    {
        throw new NotImplementedException();
    }
}

public class JTokenArray : JToken
{   
    public override object ToObject()
    {
        throw new NotImplementedException();
    }
}

public class JTokenEqualsQtyWorked : JToken
{   
    public override object ToObject()
    {
        throw new NotImplementedException();
    }
}

And then the abstract class something like this:

public abstract class JToken 
{
    public abstract object ToObject();

    public static JToken Load(JsonReader reader )
    {       
            // not ideal because an abstract ReasonCode class instance method
            // could return the required JToken via a factory method.
            // you could certainly refactor this code, or, even better: 
            // ask the reader itself to get you the correct JToken.

          string reasonCode = reader.GetReasonCode();
          switch (reasonCode)
          {
              case null:
                  return new JTokenNullReason();
                  break;
              case "Array":
                  return new JTokenArray();  
                  break;
              default:
                  return new JTokenEqualsQtyWorked();
                  break;
          }
    }
}
\$\endgroup\$
  • \$\begingroup\$ I'm afraid implementing your onlyJsonConverter is the only way. JToken etc are provided by the framework and not intended to be re-implemented... at least I've never seen any example. \$\endgroup\$ – t3chb0t Feb 13 '18 at 7:47
  • \$\begingroup\$ This helped me a lot \$\endgroup\$ – Phate01 Jun 29 '18 at 12:23
1
\$\begingroup\$

Had a look at it, and just wanted to point out that in my opinion using a JsonConverter on a property would be a good solution, as you already mentioned.

[JsonConverter(typeof(StringValueToArrayConverter))]
public string[] ReasonCode { get; set; }

The implementation of this Converter would inherit from Newtonsoft.Json.JsonConverter and implement the interface.

ReadJson method of this converter would be called on deserialization of that property, and could be as simple as:

return new string[] { reader.Value.ToString() };

but of cause, can be as complex as you need it to be.

There is also a WriteJson method for serialization and a CanConvert method for validation.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.