timcarterwex commented on code in PR #2017:
URL: https://github.com/apache/avro/pull/2017#discussion_r1061102401
##########
lang/csharp/src/apache/main/Reflect/ClassCache.cs:
##########
@@ -300,5 +295,22 @@ public void LoadClassCache(Type objType, Schema s)
break;
}
}
+
+ /// <summary>
+ /// Check if schema has to be cached
+ /// </summary>
+ /// <param name="schema"></param>
+ /// <returns>false - not applicable schema type or has been cached
before</returns>
+ protected virtual bool ToBeCached(Schema schema)
Review Comment:
this change seems to move the concept of caching to all schema types instead
of just RecordSchema, would it be a simpler change to replace the existing
check with a check of the _nameClassMap instead to correct the bug
##########
lang/csharp/src/apache/main/Reflect/ClassCache.cs:
##########
@@ -300,5 +295,22 @@ public void LoadClassCache(Type objType, Schema s)
break;
}
}
+
+ /// <summary>
+ /// Check if schema has to be cached
+ /// </summary>
+ /// <param name="schema"></param>
+ /// <returns>false - not applicable schema type or has been cached
before</returns>
+ protected virtual bool ToBeCached(Schema schema)
Review Comment:
this change seems to move the concept of caching to all schema types instead
of just `RecordSchema`, would it be a simpler change to replace the existing
check with a check of the `_nameClassMap` instead to correct the bug
--
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]
