6
\$\begingroup\$

I am writing a GenericDeserializer for Apache Kafka. My class implements IDeserializer<T> from Confluent.Kafka.Net package. I need to supply a Deserialize method which has this signature, T Deserialize(ReadOnlySpan<byte> data, bool isNull, SerializationContext context). But I also need to use Deserializers class of Confluent because it implements some low level details such as decoding a big endian message from network as primitive types, as byte array, as UTF8 string etc. How can I simplify this method. One example of simplification may be removing the use of casts which I introduced to make the compiler happy.

My logic in this method is like this, use every supported type in already implemented Deserializers class. For other types that are not deserialized with the help of this class, use Json Serialization. Here is my code:

public class GenericDeserializer<T> : IDeserializer<T>
{
    public T Deserialize(ReadOnlySpan<byte> data, bool isNull, SerializationContext context)
    {
        var type = typeof(T);

        if (type == typeof(double))
        {
            var retVal = Deserializers.Double.Deserialize(data, isNull, context);

            return (T) (object) retVal;
        }
        if (type == typeof(float))
        {
            var retVal = Deserializers.Single.Deserialize(data, isNull, context);

            return (T) (object) retVal; 
        }
        if (type == typeof(int))
        {
            var retVal = Deserializers.Int32.Deserialize(data, isNull, context);

            return (T) (object) retVal; 
        }
        if (type == typeof(long))
        {
            var retVal = Deserializers.Int64.Deserialize(data, isNull, context);

            return (T) (object) retVal; 
        }
        if (type == typeof(Null))
        {
            var retVal = Deserializers.Null.Deserialize(data, isNull, context);

            return (T) (object) retVal; 
        }
        if (type == typeof(string))
        {
            var retVal = Deserializers.Utf8.Deserialize(data, isNull, context);

            return (T) (object) retVal; 
        }
        if (type == typeof(byte[]))
        {
            var retVal = Deserializers.ByteArray.Deserialize(data, isNull, context);

            return (T) (object) retVal; 
        }
        if (isNull)
        {
            return default;
        }
        
        return JsonSerializer.Deserialize<T>(data, new JsonSerializerOptions()
        {
            PropertyNameCaseInsensitive = true
        });
    }
}

Here is the source of Deserializers class from Confluent.Kafka https://github.com/confluentinc/confluent-kafka-dotnet/blob/master/src/Confluent.Kafka/Deserializers.cs

\$\endgroup\$
5
  • \$\begingroup\$ See docs.microsoft.com/en-us/dotnet/api/… \$\endgroup\$ – Rick Davin Oct 15 '20 at 20:48
  • \$\begingroup\$ @RickDavin We can handle simple primitive types with GetTypeCode. But how do we handle the types such as byte[] or Null (a type in Confluent library). \$\endgroup\$ – ndogac Oct 16 '20 at 13:45
  • \$\begingroup\$ I don't think it's a particularly good idea to deliver defaultof a struct if the method has been told that you should deserialize to null. You should instead be deserializing to double?, long?, etc., and not using default. \$\endgroup\$ – Reinderien Oct 16 '20 at 15:03
  • \$\begingroup\$ @Reinderien We cannot make a Nullable<T> type if T is not a ValueType, we have to introduce a struct constraint but that breaks generic method. \$\endgroup\$ – ndogac Oct 16 '20 at 15:37
  • 1
    \$\begingroup\$ Right; I'm not saying to return Nullable<T> when T is a struct; I'm saying that you shouldn't accept T as a struct at all. This can be enforced on generics. \$\endgroup\$ – Reinderien Oct 16 '20 at 15:38
3
\$\begingroup\$

You can define a mapping between Types and the Deserializers. You can do this for example like this:

public class GenericDeserializer<T> : IDeserializer<T>
{
    private readonly ImmutableDictionary<Type, object> _deserializers =
        new Dictionary<Type, object>
    {
        { typeof(double), Deserializers.Double },
        { typeof(float), Deserializers.Single },
        { typeof(int), Deserializers.Int32 },
        { typeof(long), Deserializers.Int64 },
        { typeof(Null), Deserializers.Null },
        { typeof(string), Deserializers.Utf8 },
        { typeof(byte[]), Deserializers.ByteArray },
    }.ToImmutableDictionary();
}

You can't use the IDeserializer<T> in the Dictionary's Value type parameter that's why it is an object.

Then all you need to do is to make a lookup call and try to cast the value to IDeserializer<T>

if (_deserializers.ContainsKey(typeof(T)))
{
    var deserializer = _deserializers[typeof(T)] as IDeserializer<T>;
    ...
}

If the type was found then you can make branching based on the isNull value:

if (_deserializers.ContainsKey(typeof(T)))
{
    var deserializer = _deserializers[typeof(T)] as IDeserializer<T>;
    var retVal = deserializer.Deserialize(data, isNull, context);
    return !isNull ? retVal : default;
}

If it is not found then you can use the JsonSerializer as your fallback.

return JsonSerializer.Deserialize<T>(data, new JsonSerializerOptions()
{
    PropertyNameCaseInsensitive = true
});

The final code would look like this:

public class GenericDeserializer<T> : IDeserializer<T>
{
    private readonly ImmutableDictionary<Type, object> _deserializers =
        new Dictionary<Type, object>
    {
        { typeof(double), Deserializers.Double },
        { typeof(float), Deserializers.Single },
        { typeof(int), Deserializers.Int32 },
        { typeof(long), Deserializers.Int64 },
        { typeof(Null), Deserializers.Null },
        { typeof(string), Deserializers.Utf8 },
        { typeof(byte[]), Deserializers.ByteArray },
    }.ToImmutableDictionary();

    public T Deserialize(ReadOnlySpan<byte> data, bool isNull, SerializationContext context)
    {
        if (_deserializers.ContainsKey(typeof(T)))
        {
            var deserializer = _deserializers[typeof(T)] as IDeserializer<T>;
            var retVal = deserializer.Deserialize(data, isNull, context);
            return !isNull ? retVal : default;
        }

        return JsonSerializer.Deserialize<T>(data, new JsonSerializerOptions()
        {
            PropertyNameCaseInsensitive = true
        });
    }
}
\$\endgroup\$
4
  • 1
    \$\begingroup\$ I like this solution, but if typeof(T) does not exist in dictionary and it will throw KeyNotFoundException, am I wrong? \$\endgroup\$ – ndogac Oct 16 '20 at 14:54
  • \$\begingroup\$ @ndogac Yepp, you are right, I've just amended my answer to overcome on this issue. \$\endgroup\$ – Peter Csala Oct 16 '20 at 14:56
  • 1
    \$\begingroup\$ TryGetValue is better than ContainsKey because you can lookup through dictionary once instead of twice. Also JsonSerializerOptions can be static/single instance/lazy. Also the logic is different from the initial code. @ndogac, fyi \$\endgroup\$ – aepot Oct 16 '20 at 21:03
  • 1
    \$\begingroup\$ @aepot Yes, it can be further optimized. I just wanted to share the core idea to avoid code duplication (calling the Deserialize method), you can use mapping. \$\endgroup\$ – Peter Csala Oct 17 '20 at 6:35
0
\$\begingroup\$

You can get rid of the branching in the Deserialize method and instead move that logic to a place that is executed only once - the construction place. An instance of your GenericDeserializer<T> will always execute the same branch for any input, because the instance is already tied to a specific output type T. For the types like int, and double where you use their implementation just return their implementation, use your implementation for the rest.

class JsonDeserializer<T> : IDeserializer<T>
{
    public T Deserialize(ReadOnlySpan<byte> data, bool isNull, SerializationContext context)
    {
        if (isNull)
        {
            return default;
        }
        
        return JsonSerializer.Deserialize<T>(data, new JsonSerializerOptions()
        {
            PropertyNameCaseInsensitive = true
        });
    }
}


IDeserializer<T> CreateDeserializer<T>()
{
    var type = typeof(T);

    if (type == typeof(double))
    {
        return (IDeserializer<T>) Deserializers.Double;
    }

    if (type === typeof(long))
    {
        return (IDeserializer<T>) Deserializers.Int64;
    }

    // ...

    return new JsonDeserializer<T>();
}
\$\endgroup\$
6
  • \$\begingroup\$ This way, we do not get rid of branching, but move it into CreateDeserializer<T> method. There is also redundancy in writing deserializers for every supported type, we do not actually need these classes because we can return Deserializers.Double directly. \$\endgroup\$ – ndogac Oct 16 '20 at 11:43
  • \$\begingroup\$ @ndogac aha, i didnt know they already implement the interface, And it didn't make sense to me why wouldnt you return them directly if they did. And that's exactly what i suggest you do then... \$\endgroup\$ – slepic Oct 16 '20 at 13:36
  • \$\begingroup\$ And sry, i didnt imply you use the CreateDeserializer<T> as Is. It was just an example of a calling code. I meant you get rid of the branching everytime the Deserialize method Is called. \$\endgroup\$ – slepic Oct 16 '20 at 13:39
  • \$\begingroup\$ Unfortunately, we cannot return Deserializers.Double for example, if the return type is IDeserializer<T> and we are returning IDeserializer<double>, the cast will be required again and we have the same readability problem. \$\endgroup\$ – ndogac Oct 16 '20 at 13:42
  • \$\begingroup\$ In the updated code example, I get DoubleDeserializer is not assignable to IDeserializer<T> error. \$\endgroup\$ – ndogac Oct 16 '20 at 13:48

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.