FlorianHockmann commented on a change in pull request #1567:
URL: https://github.com/apache/tinkerpop/pull/1567#discussion_r807788224
##########
File path:
gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/TypeSerializerRegistry.cs
##########
@@ -214,11 +212,25 @@ public ITypeSerializer GetSerializerFor(Type valueType)
throw new InvalidOperationException($"No serializer found for type
${valueType}.");
}
- private static bool IsDictionaryType(Type type)
+ private bool IsDictionaryType(Type type, out Type keyType, out Type
valueType)
Review comment:
(nitpick) Why shouldn't this be `static` any more?
##########
File path:
gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/TypeSerializerRegistry.cs
##########
@@ -214,11 +212,25 @@ public ITypeSerializer GetSerializerFor(Type valueType)
throw new InvalidOperationException($"No serializer found for type
${valueType}.");
}
- private static bool IsDictionaryType(Type type)
+ private bool IsDictionaryType(Type type, out Type keyType, out Type
valueType)
{
- return type.IsConstructedGenericType &&
type.GetGenericTypeDefinition() == typeof(Dictionary<,>);
+ var maybeInterfaceType = type.GetInterfaces()
+ .FirstOrDefault(interfaceType =>
interfaceType.IsConstructedGenericType &&
interfaceType.GetGenericTypeDefinition() == typeof(IDictionary<,>));
Review comment:
`interfaceType` is also used again below in line 220. Please rename one
of the two variables.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
