[GitHub] [avro] KalleOlaviNiemitalo commented on a diff in pull request #1728: AVRO-3540: Support dictionaries keyed by types other than string

[GitBox <gi...@apache.org>]

KalleOlaviNiemitalo commented on code in PR #1728:
URL: https://github.com/apache/avro/pull/1728#discussion_r903310104


##########
lang/csharp/src/apache/main/Reflect/ClassCache.cs:
##########
@@ -252,7 +259,18 @@ public void LoadClassCache(Type objType, Schema s)
                         throw new AvroException($"Cant map non-generic type {objType.Name} to map {ms.Name}");
                     }
 
-                    if (!typeof(string).IsAssignableFrom(objType.GenericTypeArguments[0]))
+                    var genericTypeDef = objType.GetGenericTypeDefinition();
+                    if (!objType.IsInstanceOfType(typeof(IDictionary))

Review Comment:
   [Type.IsInstanceOfType(object)](https://docs.microsoft.com/dotnet/api/system.type.isinstanceoftype?view=netstandard-2.0) is not correct here. This checks whether `typeof(IDictionary)`, which is an instance of Type, is an instance of _objType_. Which would be true if _objType_ is `typeof(Type)` or  `typeof(MemberInfo)` or `typeof(object)`, but false if _objType_ is `typeof(IDictionary)`. To check whether instances of _objType_ implement IDictionary, you can use `typeof(IDictionary).IsAssignableFrom(objType)`.



-- 
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: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org